On Sun, 24 Mar 2024 15:11:16 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.
I am a bit uneasy with the proposed solution. Let me explain: The first thing that seems weird to me is the mere existence of increment(int)/decrement(int). Well, it is an existing API, and probably there are some use cases that I cannot immediately see. The real issue is when we have wrapAround enabled and either a large amountToStepBy or the argument for increase/decrease exceeding the (max - min) value. This simply makes no sense, and the modulo arithmetic produces, in my opinion, an unexpected user experience. What I mean is that some combinations of Spinner settings would result in apparently "random" jumps (from the user perspective). What I think should be done is - when this scenario is detected - the adjustment should default to maybe 1 (or (max - min)/10 in the case of the DoubleSpinnerValueFactory). The goal is to fall back gracefully to something a human can understand - and a modulo division is not one of those. What do you think? ------------- PR Comment: https://git.openjdk.org/jfx/pull/1431#issuecomment-2021398115
