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

Alexey Ivanov aivanov at openjdk.org
Wed Jul 26 11:06:00 UTC 2023


On Wed, 26 Jul 2023 08:13:25 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

Changes requested by aivanov (Reviewer).

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

> 52:         SwingUtilities.invokeAndWait(() -> {
> 53:             test();
> 54:         });

Suggestion:

        SwingUtilities.invokeAndWait(JTableHeaderLabelRightAlignTest::test);

Let's resolve the warning from IDEA.

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

> 56: 
> 57:     private static void test() {
> 58:         JTable table;

`JTable` is subject to the same rule: declare it where it's first used. There's no need to declare all the local variables in advance.

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

> 62:         };
> 63: 
> 64:         String[] columnNames = { "Size", "Size"};

Suggestion:

                {"1", "1"}
        };

        String[] columnNames = {"Size", "Size"};

The spaces are either on both sides or neither side.

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

> 67:         table.setSize(WIDTH, HEIGHT);
> 68:         ((JLabel)table.getTableHeader().getDefaultRenderer())
> 69:                 .setHorizontalAlignment( JLabel.RIGHT );

Suggestion:

                .setHorizontalAlignment(JLabel.RIGHT);

No spaces inside the parentheses.

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

> 75:         int w = (int)Math.ceil(SCALE * size.width);
> 76:         int h = (int)Math.ceil(SCALE * size.height);
> 77:         BufferedImage imgHeader = new BufferedImage((w), (h),

Suggestion:

        BufferedImage imgHeader = new BufferedImage(w, h,

Reduce the noise by removing the redundant parentheses.

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

> 95:                         ImageIO.write(imgHeader, "png",
> 96:                                 new File("FailureImage.png"));
> 97:                     } catch (IOException ioException){

Suggestion:

                    } catch (IOException ioException) {

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

> 98:                         ioException.printStackTrace();
> 99:                     }
> 100:                     throw new RuntimeException("Test Failed");

Can you add the coordinates to the failure message? It will greatly help anyone who will be dealing with the test failure in the future.

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

PR Review: https://git.openjdk.org/jdk/pull/14464#pullrequestreview-1547394594
PR Review Comment: https://git.openjdk.org/jdk/pull/14464#discussion_r1274787037
PR Review Comment: https://git.openjdk.org/jdk/pull/14464#discussion_r1274745113
PR Review Comment: https://git.openjdk.org/jdk/pull/14464#discussion_r1274780474
PR Review Comment: https://git.openjdk.org/jdk/pull/14464#discussion_r1274781411
PR Review Comment: https://git.openjdk.org/jdk/pull/14464#discussion_r1274782429
PR Review Comment: https://git.openjdk.org/jdk/pull/14464#discussion_r1274783146
PR Review Comment: https://git.openjdk.org/jdk/pull/14464#discussion_r1274785323



More information about the client-libs-dev mailing list