On Thu, 11 Aug 2022 22:27:39 GMT, Harshitha Onkar <hon...@openjdk.org> wrote:
>> Additional position setting (TOP_LEFT_CORNER) and a method to obtain bounds >> of test instruction frame are added to PassFailJFrame to handle positioning >> of multiple test frames. >> >> In scenarios where multiple test windows might be present, the test windows >> might overlap the instruction frame. In order to fix this TOP_LEFT_CORNER >> position option is added that positions the test instruction frame at top >> left corner with main test window below it. >> >> Additionally `getInstructionFrameBounds()` is added to obtain the position >> and dimensions of test instruction frame. > > Harshitha Onkar has updated the pull request incrementally with one > additional commit since the last revision: > > review changes A few comments which are mostly (or all?) nit picking about the javadoc of the method. test/jdk/java/awt/regtesthelpers/PassFailJFrame.java line 265: > 263: /** > 264: * Position the instruction frame with testWindow (testcase created > 265: * window) by the specified position. If testWindow is null, only The pre-existing docs don't read very well to me and some of it doesn't make sense, so in this update we should try to fix that. First sentence would be better as "Positions the instruction frame relative to the test window as specified by the {@code position} parameter. test/jdk/java/awt/regtesthelpers/PassFailJFrame.java line 269: > 267: * parameter. > 268: * Note: This method should be invoked from the method that creates > 269: * testWindow. At test-level, the testWindow must be made visible I think we should remove that Note sentence - surely the calling method is irrelevant. It is sufficient to explain that "This method should be called before making the test window visible". But why is that ? test/jdk/java/awt/regtesthelpers/PassFailJFrame.java line 272: > 270: * only after calling this method. > 271: * > 272: * @param testWindow test window that the test is created. It can be > null. "It can be null" -> "May be {@code null}" test/jdk/java/awt/regtesthelpers/PassFailJFrame.java line 273: > 271: * > 272: * @param testWindow test window that the test is created. It can be > null. > 273: * @param position position can either be: position must be one of test/jdk/java/awt/regtesthelpers/PassFailJFrame.java line 278: > 276: * center and the test window (if not null) is > placed to > 277: * the right of the instruction frame. > 278: * "vertical center" means half way between the top and the bottom which is absolutely not what you mean, is it ? horizontal center surely ? But what if the test window is > 0.5 the width of the screen ? Now it is off the edge .. wouldn't it be better to move the instruction window to the left to accommodate it ? Although that might be tricky if we don't know its size yet .. ------------- PR: https://git.openjdk.org/jdk/pull/9525