RFR: 8301606: JFileChooser file chooser details view "size" label cut off in Metal Look&Feel [v3]
Tejesh R
tr at openjdk.org
Wed Jul 26 08:13:31 UTC 2023
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
More information about the client-libs-dev
mailing list