On Wed, 2 Apr 2025 17:34:34 GMT, Rajat Mahajan <rmaha...@openjdk.org> wrote:
> Details: > Refactored code as requested in the Bug description. > > Tested and verified the test passes. Overall, it looks good to me. Except for the minor comments that I left, I can suggest: - removing `System.out.println` before throwing an error — it adds no value, the error message from the exception is sufficient; - removing "as expected" from the error message which is implied because it's an error. test/jdk/javax/swing/JRadioButton/8033699/bug8033699.java line 36: > 34: import java.awt.event.ActionListener; > 35: import java.awt.event.KeyEvent; > 36: import javax.swing.BorderFactory; I'd rather preserve the blank line between `java.*` and `javax.*` packages. However, the list of imports isn't too long in either block. test/jdk/javax/swing/JRadioButton/8033699/bug8033699.java line 74: > 72: focusManager = > KeyboardFocusManager.getCurrentKeyboardFocusManager(); > 73: }); > 74: Storing the `focusManager` once before calling `testLaF` for the first time is enough. test/jdk/javax/swing/JRadioButton/8033699/bug8033699.java line 181: > 179: mainFrame.getContentPane() > 180: .setLayout(new BoxLayout(mainFrame.getContentPane(), > 181: BoxLayout.Y_AXIS)); Alternatively, you can create another vertical box, put all the components into it instead of `mainFrame`, and add this vertical box into the frame. test/jdk/javax/swing/JRadioButton/8033699/bug8033699.java line 301: > 299: private static boolean actRB1 = false; > 300: private static boolean actRB2 = false; > 301: private static boolean actRB3 = false; These three boolean variables need to be declared volatile, the value is modified on the EDT but their value is read on the main thread. test/jdk/javax/swing/JRadioButton/8033699/bug8033699.java line 320: > 318: > 319: hitKey(KeyEvent.VK_DOWN); > 320: hitKey(KeyEvent.VK_DOWN); `addActionListener`, as well as `removeActionListener`, should rather be called on EDT. ------------- Changes requested by aivanov (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/24384#pullrequestreview-2776316752 PR Review Comment: https://git.openjdk.org/jdk/pull/24384#discussion_r2049286794 PR Review Comment: https://git.openjdk.org/jdk/pull/24384#discussion_r2049291058 PR Review Comment: https://git.openjdk.org/jdk/pull/24384#discussion_r2049300044 PR Review Comment: https://git.openjdk.org/jdk/pull/24384#discussion_r2049307571 PR Review Comment: https://git.openjdk.org/jdk/pull/24384#discussion_r2049310017
