On Fri, 10 Mar 2023 11:38:56 GMT, Karthik P K <k...@openjdk.org> wrote:

> When converter was changed, `setText` method of spinner was not invoked to 
> update the spinner text using new converter.
> 
> Added a listener to converter property of `SpinnerValueFactory` to call 
> `setText` so that spinner text is updated using new converter.
> 
> Added unit test to validate the fix. Along with spinner of Double type, added 
> a spinner of Integer type as well so that the fix is validated using 2 types 
> of spinner.

Changes requested by aghaisas (Reviewer).

modules/javafx.controls/src/main/java/javafx/scene/control/Spinner.java line 
545:

> 543:                         value.bind(newFactory.valueProperty());
> 544:                         // Update the spinner editor when converter is 
> changed.
> 545:                         newFactory.converterProperty().addListener((o, 
> oldValue, newValue) -> {

The addition of ChangeListener is fine, but, this newly added ChangeListener 
should be removed from old valueFactory when `valueFactory` changes.
Please see the pattern used in `TimelineController` for rateListener.

modules/javafx.controls/src/test/java/test/javafx/scene/control/SpinnerTest.java
 line 1529:

> 1527:     }
> 1528: 
> 1529:     @Test public void testSpinnerEditorUpdateOnConverterChange() {

It is better to split this test case into two - one for each type of Spinner.

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

PR: https://git.openjdk.org/jfx/pull/1057

Reply via email to