On Wed, 10 Apr 2024 11:47:28 GMT, drmarmac <d...@openjdk.org> wrote:

>> This PR should fix the issue and cover all relevant cases with new tests.
>> 
>> Note: This involves a small behavior change, as can be seen in 
>> dblSpinner_testWrapAround_decrement_twoSteps() in SpinnerTest.java:749. With 
>> this change the wraparound behavior is similar to that of the IntegerSpinner.
>
> drmarmac has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Use direction-dependent modulo arithmetic in DoubleSpinnerValueFactory 
> wrap-around logic

Marked as reviewed by angorya (Reviewer).

It looks like the issue 
[JDK-8193286](https://bugs.openjdk.org/browse/JDK-8193286) is also fixed by 
this change, so it probably should be added to this PR.

Also, this PR changes the way {List|Integer|Double}ValueFactory behaves when 
wrapping is enabled, so even though we determined that no CSR is required, I 
feel we need to document the new behavior in the ticket.

So here my suggestions:
1. add JDK-8193286 to this PR
2. describe the new behavior in JDK-8242553 description

otherwise, lgtm.

-------------

PR Review: https://git.openjdk.org/jfx/pull/1431#pullrequestreview-1995142070
PR Comment: https://git.openjdk.org/jfx/pull/1431#issuecomment-2050402556

Reply via email to