On Mon, 6 May 2024 08:24:52 GMT, drmarmac <d...@openjdk.org> wrote:
> This PR updates the javadoc for the SpinnerFactory wrap-around behavior
> introduced in #1431.
Looks like a good start. I left a few minor suggestions.
@andy-goryachev-oracle Do you also want to take a look?
modules/javafx.controls/src/main/java/javafx/scene/control/SpinnerValueFactory.java
line 118:
> 116: * @param steps The number of decrements that should be performed on
> the value.
> 117: * If the number is negative, the call is equivalent to
> calling
> 118: * {@link #increment(int)} with the absolute number of
> steps.
Suggestion:
... with the absolute value of {@code steps}.
modules/javafx.controls/src/main/java/javafx/scene/control/SpinnerValueFactory.java
line 129:
> 127: * @param steps The number of increments that should be performed on
> the value.
> 128: * If the number is negative, the call is equivalent to
> calling
> 129: * {@link #decrement(int)} with the absolute number of
> steps.
Suggestion:
... with the absolute value of {@code steps}.
modules/javafx.controls/src/main/java/javafx/scene/control/SpinnerValueFactory.java
line 419:
> 417: * If {@link #wrapAround} is {@code true}, the {@code
> IntegerSpinnerValueFactory} will step from
> 418: * the minimum value to the maximum value (and vice versa). As a
> consequence of that, the number
> 419: * of steps required to wrap around to the same value is N+1, where
> N is the number of steps between
I recommend code case for `N` and `N+1`.
modules/javafx.controls/src/main/java/javafx/scene/control/SpinnerValueFactory.java
line 420:
> 418: * the minimum value to the maximum value (and vice versa). As a
> consequence of that, the number
> 419: * of steps required to wrap around to the same value is N+1, where
> N is the number of steps between
> 420: * {@link #min} (inclusive) and {@link #max} (inclusive).
Suggestion: It might be helpful to show the equation:
val = (val + delta) % (max - min + 1)
modules/javafx.controls/src/main/java/javafx/scene/control/SpinnerValueFactory.java
line 621:
> 619: * If {@link #wrapAround} is {@code true}, the {@code
> DoubleSpinnerValueFactory} will step through
> 620: * from the maximum value to the minimum value seamlessly; that is,
> any step up from the maximum value
> 621: * is equal to the same step up from the minimum value (and vice
> versa).
Suggestion: It might be helpful to show the equation:
val = (val + delta) % (max - min)
-------------
PR Review: https://git.openjdk.org/jfx/pull/1450#pullrequestreview-2041073507
PR Comment: https://git.openjdk.org/jfx/pull/1450#issuecomment-2096331641
PR Review Comment: https://git.openjdk.org/jfx/pull/1450#discussion_r1591187179
PR Review Comment: https://git.openjdk.org/jfx/pull/1450#discussion_r1591187802
PR Review Comment: https://git.openjdk.org/jfx/pull/1450#discussion_r1591189203
PR Review Comment: https://git.openjdk.org/jfx/pull/1450#discussion_r1591190823
PR Review Comment: https://git.openjdk.org/jfx/pull/1450#discussion_r1591191777
