On Thu, 9 Apr 2020 09:58:27 GMT, Jesper Skov 
<github.com+2720909+js...@openjdk.org> wrote:

> Make the two ways of associating a ToggleButton with a ToggleGroup interact 
> correctly.
> 
> This fixes https://bugs.openjdk.java.net/browse/JDK-8198402

The fix looks correct to me. Have you run all tests to ensure no regressions?

I left a couple inline comments.

modules/javafx.controls/src/main/java/javafx/scene/control/ToggleButton.java 
line 196:

> 195:     private ObjectProperty<ToggleGroup> toggleGroup;
> 196:     @Override
> 197:     public final void setToggleGroup(ToggleGroup value) {

This is unrelated to the fix. The changes in this file should be reverted.

modules/javafx.controls/src/test/java/test/javafx/scene/control/ToggleButtonTest.java
 line 270:

> 269:             } catch (InterruptedException e) {
> 270:                System.err.println("InterruptedException occurred during 
> Thread.sleep()");
> 271:             }

An exception here should cause the test to fail. You can use `Assert.fail` or 
else re-throw the exception as an
`AssertionFailedError`.

modules/javafx.controls/src/main/java/javafx/scene/control/ToggleGroup.java 
line 74:

> 73:             while (c.next()) {
> 74:                 List<Toggle> addedToggles = c.getAddedSubList();
> 75:

This can be declared as final.

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

PR: https://git.openjdk.java.net/jfx/pull/167

Reply via email to