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

[Alexey Ivanov]

On Thu, 27 Jul 2023 14:30:23 GMT, Prasanta Sadhukhan <psadhukhan at openjdk.org> wrote:

> Adding buttons in a JToolBar after a JSeparator will push the button to the far right of the frame instead of just after the separator
> 
> ![image](https://github.com/openjdk/jdk/assets/43534309/7fcad657-493f-4370-b046-b31c212a8aa7)
> 
> This is because BasicSeparatorUI does not define a maximum size. This leads to the separator getting all the extra width.
> Fix is made to limit the separator's maximum size to the preferred size of corresponding orientation (i.e. width for VERTICAL and height for HORIZONTAL)

Changes requested by aivanov (Reviewer).

src/java.desktop/share/classes/javax/swing/plaf/basic/BasicSeparatorUI.java line 1:

> 1: /*

Could you please expand the wildcard imports and update the copyright year, please?

src/java.desktop/share/classes/javax/swing/plaf/basic/BasicSeparatorUI.java line 155:

> 153: 
> 154:     public Dimension getMinimumSize( JComponent c ) { return null; }
> 155:     public Dimension getMaximumSize( JComponent c ) {

A blank line before the method would visually separate it from the previous method. The updated implementation isn't trivial.

src/java.desktop/share/classes/javax/swing/plaf/basic/BasicSeparatorUI.java line 162:

> 160:         else {
> 161:             return new Dimension(Integer.MAX_VALUE, d.height);
> 162:         }

`WindowsToolBarSeparatorUI` uses `Short.MAX_VALUE` instead of `Interger`:

https://github.com/openjdk/jdk/blob/ee3e0917b393b879a543060ace2537be84f20e82/src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/WindowsToolBarSeparatorUI.java#L74-L81

Should we follow the pattern here?

In Java Code Style, the `else` keyword should be on the same line as the closing brace.

test/jdk/javax/swing/JToolBar/ToolBarSeparatorTest.java line 71:

> 69:                 toolBar.add(btn);
> 70:                 toolBar.add(new JButton("button 2"));
> 71:                 toolBar.add(new JSeparator(SwingConstants.VERTICAL));

Would it be easier to save `JSeparator` into a field, then get its size and compare it to its preferred size?


if (separator.getSize().width != separator.getPreferredSize().width) {
    // Fail the test because the separator is too wide
}

-------------

PR Review: https://git.openjdk.org/jdk/pull/15054#pullrequestreview-1557022453
PR Review Comment: https://git.openjdk.org/jdk/pull/15054#discussion_r1280662321
PR Review Comment: https://git.openjdk.org/jdk/pull/15054#discussion_r1280655962
PR Review Comment: https://git.openjdk.org/jdk/pull/15054#discussion_r1280661723
PR Review Comment: https://git.openjdk.org/jdk/pull/15054#discussion_r1280666570