On Wed, 2 Aug 2023 12:12:05 GMT, Alexey Ivanov <[email protected]> wrote:
>> Prasanta Sadhukhan has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Review comments
>
> src/java.desktop/share/classes/javax/swing/plaf/basic/BasicSeparatorUI.java
> line 32:
>
>> 30: import java.awt.Graphics;
>> 31: import java.awt.Insets;
>> 32: import java.awt.Rectangle;
>
> Neither `Insets`, nor `Rectangle` are used here.
> Suggestion:
it was there even before this PR but anyway have removed...
> test/jdk/javax/swing/JToolBar/ToolBarSeparatorTest.java line 71:
>
>> 69: toolBar.setMargin(new Insets(0,0,0,0));
>> 70: btn = new JButton("button 1");
>> 71: toolBar.add(btn);
>
> Suggestion:
>
> toolBar.add(new JButton("button 1"));
>
> The field `btn` is not used for anything else any more, it can be removed.
ok
> test/jdk/javax/swing/JToolBar/ToolBarSeparatorTest.java line 88:
>
>> 86: SwingUtilities.invokeAndWait(() -> {
>> 87: pt = toolBar.getLocationOnScreen();
>> 88: size = toolBar.getSize();
>
> Suggestion:
>
> toolBarBounds = new Rectangle(toolBar.getLocationOnScreen(),
> toolBar.getSize());
>
> Where `toolBarBounds` is a static field of `Rectangle` which replaces both
> `pt` and `size`. Thus, you already have the rectangle to capture the
> screenshot if the test fails.
ok
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15054#discussion_r1282598052
PR Review Comment: https://git.openjdk.org/jdk/pull/15054#discussion_r1282598289
PR Review Comment: https://git.openjdk.org/jdk/pull/15054#discussion_r1282598368
