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