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

[Kevin Rushforth]

On Wed, 27 Mar 2024 19:19:36 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

> Now, when wrapping occurs, should max/min be considered as valid steps?

For integer values (including list indices), yes, since each integer (or list index) is a discrete value that can have the value min, max, or any value in between. And a step of one when you are at "max" should wrap around to "min"

For double... that's a good question. I need to think about it a bit. Often when doing wrap-around for floating-point values (think rotation transforms) we treat max and min the same -- although usually that means treating the range as exclusive of max, and that isn't what is current done for double values of spinner.

> Example, try both integer and double spinners with {min=0, max=10, step=7} starting with value=0. try incrementing.
> 
> integer: 0, 7, 3, 10
> double: 0, 7, 10, 0, 7, 10

The integer is working as I would expect (and the next value should be "6").
The double is not -- it looks like it is clamping rather than wrapping unless the value is already at the max.

> And another question: should the new behavior (whatever everyone agrees to eventually) be documented? Where? This PR? JBS? Javadoc? A markdown file in doc-files/behavior/?

If we want to specify the functionality, it needs to be documented in the javadoc-generated API docs. That's the specification. Given that this is a clarification of what we mean by "wrapping" and not a fundamental change, we could either do that as part of this enhancement request or a separate one.

> Does it need a CSR?

The current spec is sufficiently vague on what the wrapping algorithm is that as long as we aren't changing anything fundamental, I don't think we do need a CSR. If we end up changing the spec to document the wrapping algorithm (either as part of this PR or separately), then the change in spec would need a CSR.

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

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