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

Alexey Ivanov aivanov at openjdk.org
Fri Jul 28 14:30:49 UTC 2023


On Thu, 27 Jul 2023 04:53:28 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

Looks good to me.

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

> 82: 
> 83:         int x = (int)(table.getTableHeader().
> 84:                 getColumnModel().getColumn(0).getWidth() * SCALE);

Suggestion:

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

It's a matter of (my) preference, and it's stated in Java Coding Style that wrapping the operator to the next line makes it clearer it's a continuation line.

I'd wrap on each method call:

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

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

> 94:                 }
> 95:                 throw new RuntimeException("Test Failed at <" +
> 96:                         x + ", " + y + ">");

Suggestion:

                throw new RuntimeException("Test Failed at <" + x + ", " + y + ">");

It doesn't fit 80-column limit, but I think it is okay to keep it on one line without wrapping. It doesn't affect the readability of the code, and it's only 5 chars above the limit.

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

Marked as reviewed by aivanov (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/14464#pullrequestreview-1552214586
PR Review Comment: https://git.openjdk.org/jdk/pull/14464#discussion_r1277608914
PR Review Comment: https://git.openjdk.org/jdk/pull/14464#discussion_r1277600275



More information about the client-libs-dev mailing list