On Fri, 5 Aug 2022 19:37:57 GMT, Harshitha Onkar <hon...@openjdk.org> wrote:
>> test/jdk/java/awt/regtesthelpers/PassFailJFrame.java line 297: >> >>> 295: .getDefaultScreenDevice().getDefaultConfiguration(); >>> 296: Insets screenInsets = >>> Toolkit.getDefaultToolkit().getScreenInsets(gc); >>> 297: >> >> I suggest separating the logic to calculate coordinates into a separate >> function, or save them into a local variable. >> >> Then call `sync` and `sleep`. Or this sequence could be extracted into its >> own helper method to avoid code duplication, it's repeated at least twice. >> >> Shall this method call `window.setVisible(true)` automatically? > > @aivanov-jdk As suggested, separating the common code into a helper method > would make the code look cleaner. > > Earlier I did think about having the setVisible for both the testWindow and > instruction frame within the positionWindow but usually at the test-level, > the user would be in the habit of creating the testWindow and setting its > visibility to true when creating test UI. Hence left it to be set at > test-level. I can change it, if this approach is better. > > Either way we need to make minor changes to existing manual tests - > **remove** (in case we add testWindow.setVisible(true) to PassFailJFrame) or > **reposition** (in case we retain it at test-level) setVisible() call. Toolkit.sync + Thread.sleep extracted to a separate helper method. ------------- PR: https://git.openjdk.org/jdk/pull/9525
