<Swing Dev> Review Request: JDK-8009477 :PageUp and PageDown keyboard buttons don't move slider indicator to next minor tick

[Alexandr Scherbatiy]

The fix looks good to me.

Thanks,
Alexandr.

On 9/6/2016 9:56 AM, Prem Balakrishnan wrote:
>
> Hi Alexandr,
>
> Thank you for the feedback.
>
> I have updated patch as per review comments.
>
> http://cr.openjdk.java.net/~pkbalakr/8009477/webrev.01/ 
> <http://cr.openjdk.java.net/%7Epkbalakr/8009477/webrev.01/>
>
> Regards,
>
> Prem
>
> *From:*Alexandr Scherbatiy
> *Sent:* Monday, September 05, 2016 3:50 PM
> *To:* Prem Balakrishnan; Sergey Bylokhov; Rajeev Chamyal; 
> swing-dev at openjdk.java.net
> *Subject:* Re: Review Request: JDK-8009477 :PageUp and PageDown 
> keyboard buttons don't move slider indicator to next minor tick
>
> On 9/2/2016 1:00 PM, Prem Balakrishnan wrote:
>
> Hi Alexandr,
>
> Yes true, Setting getSnapToTicks() true will set block increment to 
> minor/majorTickSpacing.
>
> But  enabling getSanpToTicks will Disable Unit Increment (scrollByUnit 
> using Up/Down Arrow keys )
>
>
>    It works in the following way before the fix:
>    Use minor, major, or (max-min)/10 if  snapToTicks is enabled. Use 
> (max-min)/10 if snapToTicks is disabled.
>
>   After the fix it works: Use minor or (max-min)/10 if snapToTicks is 
> disabled. Why major tick spacing is not taken into account in this 
> case in the same way as for the enabled snapToTicks? It seems that 
> both minor and major tick spacing should be checked together.
>
>   Thanks,
>   Alexandr.
>
>
> Regards,
> Prem
>
> *From:*Alexandr Scherbatiy
> *Sent:* Friday, September 02, 2016 3:14 PM
> *To:* Prem Balakrishnan; Sergey Bylokhov; Rajeev Chamyal; 
> swing-dev at openjdk.java.net <mailto:swing-dev at openjdk.java.net>
> *Subject:* Re: Review Request: JDK-8009477 :PageUp and PageDown 
> keyboard buttons don't move slider indicator to next minor tick
>
> On 9/2/2016 8:28 AM, Prem Balakrishnan wrote:
>
>
> Hi Alexandr,
>
> Thankyou for the Review.
>
> >  For the case there only majorTickSpacing is set should it be used for a 
> block increment?
>
> When setSnapToTicks(Boolean value) is set to true,
>
> All the keys Left/Right, Up/Down, PageUp/PageDown snaps to nearest 
> tick(i.e., If minor Tick not set, it considers major Tick as the 
> nearest tick).
>
> Based on my understanding, we need to set the  minimum blockincrement 
> size for PageUp/PageDown, which should be MinorTickSpacing if available.
>
> Where as if minorTickSpacing not Set and only MajorTickSpacing is set 
> then blockincrement=[ (slider.getMaximum() - slider.getMinimum()) / 
> 10] should be better.
>
>   I see that the minorTickSpacing and majorTickSpacing are already 
> used for the block increment in the BasicSliderUI.getTickSpacing() 
> method when slider.getSnapToTicks() is enabled. May be a user just 
> needs to set snapToTicks to true for a slider to get the desired block 
> increment?
>
>   Thanks,
>   Alexandr.
>
>
>
> Regards,
>
> Prem
>
> *From:*Alexandr Scherbatiy
> *Sent:* Thursday, September 01, 2016 8:46 PM
> *To:* Prem Balakrishnan; Sergey Bylokhov; Rajeev Chamyal; 
> swing-dev at openjdk.java.net <mailto:swing-dev at openjdk.java.net>
> *Subject:* Re: Review Request: JDK-8009477 :PageUp and PageDown 
> keyboard buttons don't move slider indicator to next minor tick
>
> On 8/31/2016 3:36 PM, Prem Balakrishnan wrote:
>
>
>
> Hi*,*
>
> Please review fix for JDK 9,
>
> *Bug:*https://bugs.openjdk.java.net/browse/JDK-8009477
>
> *Webrev:*http://cr.openjdk.java.net/~pkbalakr/8009477/webrev.00/ 
> <http://cr.openjdk.java.net/%7Epkbalakr/8009477/webrev.00/>
>
> *Cause:*PageUp/PageDown key is used to scroll by block,
>
> MinorTickSpacing set by the user is not consider as the Block size.
>
> *Fix:*   MinorTickSpacing(non-zero value) set by the user is set as 
> Block size.
>
>   For the case there only majorTickSpacing is set should it be used 
> for a block increment?
>
>   Thanks,
>   Alexandr.
>
>
>
> Regards,
>
> Prem
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20160906/446b9893/attachment.html>