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

[Tejesh R]

On Tue, 25 Jul 2023 12:50:35 GMT, Alexey Ivanov <aivanov at openjdk.org> wrote:

>> Tejesh R has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Spacing fix
>
> test/jdk/javax/swing/JTableHeader/JTableHeaderLabelRightAlignTest.java line 85:
> 
>> 83:         final JTableHeader header = table.getTableHeader();
>> 84:         TableCellRenderer renderer = header.getDefaultRenderer();
>> 85:         header.setDefaultRenderer(renderer);
> 
> This doesn't make sense to me: you get the renderer from the header and set it back. Nothing's changed here, is it?

Yeah, not required.

> test/jdk/javax/swing/JTableHeader/JTableHeaderLabelRightAlignTest.java line 108:
> 
>> 106:         for (int i = 1; i < imgHeader.getHeight()-3; i++) {
>> 107:             for (int j = verticalLineCol; j < verticalLineCol + 1; j++) {
>> 108:                 if (expectedRGB != imgHeader.getRGB(j, i)) {
> 
> Suggestion:
> 
>         for (int i = 1; i < imgHeader.getHeight()-3; i++) {
>             for (int j = verticalLineCol; j < verticalLineCol + 1; j++) {
>                 if (expectedRGB != imgHeader.getRGB(j, i)) {
> 
> These are coordinates, so name them as `x` and `y`. Adding the coordinates would help an engineer dealing with the test failure the clue what went wrong, that engineer could be you.
> 
> The height should also be scaled: `(int) (imgHeader.getHeight() * SCALE - 3);`

For height, I am using BufferedImage height, not TableHeader height. So I guess it is fine.

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

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