RFR: 8301606: JFileChooser file chooser details view "size" label cut off in Metal Look&Feel [v11]

Alexey Ivanov aivanov at openjdk.org
Wed Aug 2 16:56:00 UTC 2023


On Wed, 2 Aug 2023 16:26:42 GMT, Tejesh R <tr at openjdk.org> wrote:

>> "size" label which is _RIGHT_ aligned is cut off on header cell. The issue is not only w.r.t to `JFileChooser` rather it is part of `JTable`. The root caused is found to be that in metal L&F the border insets is set to `(2,2,2,0)` meaning the right most inset value is 0. Hence when UIScaling increases the issue will be visible clearly. The fix addresses the issue by setting the right `inset` to 2 similar to other `inset` values. (Though the reason for setting it to 0 is unclear since it was initial load). 
>> CI testing shows green.
>> After the fix at 225% scaling:
>> ![image](https://github.com/openjdk/jdk/assets/94159358/f3e5a88a-1710-4ee0-84aa-338bc93010b2)
>
> Tejesh R has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Review fix

Changes requested by aivanov (Reviewer).

test/jdk/javax/swing/JTableHeader/JTableHeaderLabelRightAlignTest.java line 59:

> 57: 
> 58:         JTable table = new JTable(data, columnNames);
> 59:         table.setSize(WIDTH, HEIGHT);

Setting size of the table seems unnecessary, the test works correctly without it. If so, both `WIDTH` and `HEIGHT` could be removed.

test/jdk/javax/swing/JTableHeader/JTableHeaderLabelRightAlignTest.java line 69:

> 67:         int w = (int)Math.ceil(SCALE * size.width);
> 68:         int h = (int)Math.ceil(SCALE * size.height);
> 69:         BufferedImage imgHeader = new BufferedImage(w, h, BufferedImage.TYPE_INT_RGB);

Suggestion:

        int w = (int)Math.ceil(SCALE * size.width);
        int h = (int)Math.ceil(SCALE * size.height);
        BufferedImage imgHeader =
                new BufferedImage((int)Math.ceil(SCALE * size.width),
                        (int)Math.ceil(SCALE * size.height),
                        BufferedImage.TYPE_INT_RGB);

Inline `w` and `h` as in `TableHeaderBorderPositionTest.java`?

https://github.com/openjdk/jdk/blob/be04454fce1e298655154603700580d11bad2c5f/test/jdk/javax/swing/JTableHeader/TableHeaderBorderPositionTest.java#L66-L69

test/jdk/javax/swing/JTableHeader/JTableHeaderLabelRightAlignTest.java line 79:

> 77: 
> 78:         int x = (int) (table.getTableHeader().getColumnModel().getColumn(0)
> 79:                             .getWidth() * SCALE);

Suggestion:

        int x = (int)(table.getTableHeader()
                           .getColumnModel()
                           .getColumn(0)
                           .getWidth() * SCALE);


To make it look similar to `TableHeaderBorderPositionTest.java`:

https://github.com/openjdk/jdk/blob/be04454fce1e298655154603700580d11bad2c5f/test/jdk/javax/swing/JTableHeader/TableHeaderBorderPositionTest.java#L82-L85

After all, these two tests are very similar.

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

PR Review: https://git.openjdk.org/jdk/pull/14464#pullrequestreview-1559453623
PR Review Comment: https://git.openjdk.org/jdk/pull/14464#discussion_r1282178025
PR Review Comment: https://git.openjdk.org/jdk/pull/14464#discussion_r1282183345
PR Review Comment: https://git.openjdk.org/jdk/pull/14464#discussion_r1282179571



More information about the client-libs-dev mailing list