On Wed, 26 Jul 2023 17:28:23 GMT, Alexey Ivanov <[email protected]> wrote:

>> 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 
> 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.

I don't think we should verify the colors to the left of shadow border, for 
this particular test verifying right column of header border would do the job. 
VerticalLine is at the same location, can remove the for-loop.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14464#discussion_r1275725972

Reply via email to