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

Alexey Ivanov aivanov at openjdk.org
Wed Jul 26 18:21:56 UTC 2023


On Wed, 26 Jul 2023 11:29:22 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

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

> 85:         int expectedRGB = imgHeader.getRGB(verticalLine, 1);
> 86: 
> 87:         for (int y = 1; y < (imgHeader.getHeight() -3); y++) {

Suggestion:

        for (int y = 1; y < (imgHeader.getHeight() - 3); y++) {

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

> 86: 
> 87:         for (int y = 1; y < (imgHeader.getHeight() -3); y++) {
> 88:             for (int x = verticalLine; x < verticalLine + 1; x++) {

This means `x` is always equal to `verticalLine`, does it? Is the for-loop needed then?

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

> 94:                         ioException.printStackTrace();
> 95:                     }
> 96:                     throw new RuntimeException("Test Failed at Pos_x : " + x + ", Pos_y : " + y );

Perhaps, "Test failed at <x, y>" would be enough but I'm nitpicking…

More importantly, for me the test fails at 168, 24 which falls between the header cells, more specifically between the shadow and highlight. Should we verify the colours to the left of shadow border?

In the updated version, the `verticalLine` is at the same location. The text in the first column touches the shadow border of the header cell; the text in the second column has 1-pixel gap. With the increased insets, I expected that the text wouldn't touch the shadow.

At the same time, I can *confirm the fix is correct*: the header cell has 1-pixel border, the insets reserved the space for the 1-pixel border and 1-pixel gap; the right column didn't but it should have.

Because of rounding, the header cells still look inconsistent. At 225%, 2-pixel inset is 4.5 pixels but fractional pixels are impossible, and this contributes the offsets we see in the image.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14464#discussion_r1275287311
PR Review Comment: https://git.openjdk.org/jdk/pull/14464#discussion_r1275287142
PR Review Comment: https://git.openjdk.org/jdk/pull/14464#discussion_r1275281294



More information about the client-libs-dev mailing list