RFR: 8242553: IntegerSpinner and DoubleSpinner do not wrap around values correctly in some cases

[Kevin Rushforth]

On Tue, 26 Mar 2024 20:25:19 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

> 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).

Why? This is entirely under app control. Is it useful to pass an increment that will produce a step larger than the range? Maybe not, but restricting it doesn't see like the right solution, at least not without a good reason to do so. The "obvious" thing is to do modulo arithmetic, which is what this PR aims to do. By "obvious" thing to do, it is the mathematically correct thing to do. It makes a single step of 10 equivalent to 10 steps of 1 regardless of the  min / max.

> 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.

And I would argue that either of those choices is more arbitrary and confusing than the proposed solution, and not appreciably better than what is done today.

I see two sensible solutions:

1. Use modulo arithmetic to compute the value (what this PR proposed)
2. Don't process the increment at all (i.e., treat it as an out-of-range request and do nothing)

Choice 1 seems best unless there is a down-side that I haven't considered.

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

PR Comment: https://git.openjdk.org/jfx/pull/1431#issuecomment-2021492726