On Thu, 3 Oct 2024 17:00:04 GMT, Alexey Ivanov <aiva...@openjdk.org> wrote:
>> Jayathirth D V has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - Remove not needed setVisible calls
>> - Update based on review comments
>
> test/jdk/java/awt/Window/LocationByPlatform/TestLocationByPlatform.java line
> 48:
>
>> 46: intersecting other Frames or stacked over normal frame with
>> some
>> 47: offset. Another has its location explicitly set to (0, 450).
>> 48: Please verify that the frames are situated correctly.
>
> Suggestion:
>
> Please verify that the frames are located correctly on the screen.
Updated.
> test/jdk/java/awt/Window/LocationByPlatform/TestLocationByPlatform.java line
> 54:
>
>> 52: surrounding the client area of frame with no pixels between
>> it and
>> 53: the frame's decorations. Press Pass if this all is true,
>> 54: otherwise press Fail.
>
> Suggestion:
>
>
> Please verify that the picture inside of frames looks the same
> and consists of red descending triangle occupying exactly the
> bottom
> half of the frame. Also, verify that there is a blue rectangle
> exactly
> surrounding the client area of frame with no pixels between it and
> the frame's decorations. Press Pass if this all is true,
> otherwise press Fail.
>
>
> Adding a blank line adds structure to the instructions and makes them easier
> to read.
Updated. I have added new line also i have re-worded it differently instead of
continuous statements with "Please verify"
> test/jdk/java/awt/Window/LocationByPlatform/TestLocationByPlatform.java line
> 63:
>
>> 61: .columns(40)
>> 62: .build();
>> 63: EventQueue.invokeAndWait(() -> createUI());
>
> Suggestion:
>
> EventQueue.invokeAndWait(TestLocationByPlatform::createUI);
>
> I prefer using method references; I don't insist on it though.
Updated.
> test/jdk/java/awt/Window/LocationByPlatform/TestLocationByPlatform.java line
> 86:
>
>> 84: g.drawLine(399, 0, 399, 399);
>> 85: }
>> 86: };
>
> I suggest creating a custom class that extends `Canvas`. The class will
> encapsulate all this logic.
>
> You'll create an instance of this class and put it to each frame without
> duplicating all the code to render the red triangle and blue rectangle.
Good idea updated.
> test/jdk/java/awt/Window/OwnedWindowShowTest/OwnedWindowShowTest.java line 38:
>
>> 36:
>> 37: public class OwnedWindowShowTest {
>> 38: public static void main(String args[]) throws Exception {
>
> Use Java-style array declaration:
> Suggestion:
>
> public static void main(String[] args) throws Exception {
Updated.
> test/jdk/java/awt/Window/OwnedWindowShowTest/OwnedWindowShowTest.java line 39:
>
>> 37: public class OwnedWindowShowTest {
>> 38: public static void main(String args[]) throws Exception {
>> 39: EventQueue.invokeAndWait(() -> runTest());
>
> Suggestion:
>
> EventQueue.invokeAndWait(OwnedWindowShowTest::runTest);
Updated.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21284#discussion_r1787231272
PR Review Comment: https://git.openjdk.org/jdk/pull/21284#discussion_r1787232040
PR Review Comment: https://git.openjdk.org/jdk/pull/21284#discussion_r1787232971
PR Review Comment: https://git.openjdk.org/jdk/pull/21284#discussion_r1787232341
PR Review Comment: https://git.openjdk.org/jdk/pull/21284#discussion_r1787233409
PR Review Comment: https://git.openjdk.org/jdk/pull/21284#discussion_r1787233238
