On Thu, 27 Jul 2023 04:53:28 GMT, Tejesh R <[email protected]> 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:
>> 
>
> 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
