RFR: 8321140: Add comment to note difference in Metal's JButton margins [v2]

Damon Nguyen dnguyen at openjdk.org
Thu Aug 15 19:15:21 UTC 2024


On Thu, 15 Aug 2024 18:32:08 GMT, Alexey Ivanov <aivanov at 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


More information about the client-libs-dev mailing list