On Thu, 15 Aug 2024 18:32:08 GMT, Alexey Ivanov <aiva...@openjdk.org> wrote:
>> Damon Nguyen has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Move note > > src/java.desktop/share/classes/javax/swing/plaf/metal/MetalLookAndFeel.java > line 810: > >> 808: "released SPACE", "released" >> 809: }), >> 810: // margin is (2, 14, 2, 14) which is vastly larger in >> horizontal > > I still suggest adding a reference to `BasicBorder` class or > `BasicLookAndFeel` where this border is defined. > > The borders for check boxes and radio buttons reference > `BasicBorders.RadioButtonBorder` which helps to understand where the values > are coming from. > > Specifically, `BasicLookAndFeel` defines `Button.margin` property: > > https://github.com/openjdk/jdk/blob/1cd488436880b00c55fa91f44c115999cf686afd/src/java.desktop/share/classes/javax/swing/plaf/basic/BasicLookAndFeel.java#L729 > > It is set to the button in `installDefaults`: > > https://github.com/openjdk/jdk/blob/1cd488436880b00c55fa91f44c115999cf686afd/src/java.desktop/share/classes/javax/swing/plaf/basic/BasicButtonUI.java#L158-L160 > > `BasicBorders.getButtonBorder` returns a border which defines colours and > margins; the margins come from `MarginBorder` class which just returns the > margins of a button component: > > https://github.com/openjdk/jdk/blob/1cd488436880b00c55fa91f44c115999cf686afd/src/java.desktop/share/classes/javax/swing/plaf/basic/BasicBorders.java#L493-L495 Agreed. This is what I found as well. Had a bit of trouble figuring out how to phrase the note since the comment was meant to differentiate Metal from the other default installed LAFs. I moved it to `BasicLookAndFeel.java` since that's where the values are specifically defined. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/20482#discussion_r1718876976
