RFR: 4346610: Adding JSeparator to JToolBar "pushes" buttons added after separator to edge [v2]

[Prasanta Sadhukhan]

On Wed, 2 Aug 2023 12:12:05 GMT, Alexey Ivanov <aivanov at openjdk.org> 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