[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-2d-dev
Subject:    Re: RFR: 8318590: JButton ignores margin when painting HTML text [v3]
From:       Damon Nguyen <dnguyen () openjdk ! org>
Date:       2023-11-30 18:34:21
Message-ID: AdAku37pNzsb3TrutQ6yc2NEMF8nl7jq730gOkS0DW4=.d97aa443-4288-490a-9253-85a68dd61524 () github ! com
[Download RAW message or body]

On Wed, 29 Nov 2023 19:48:27 GMT, Damon Nguyen <dnguyen@openjdk.org> wrote:

> > src/java.desktop/share/classes/javax/swing/plaf/basic/BasicLookAndFeel.java line \
> > 464: 
> > > 462:      * <p>
> > > 463:      * The default button margin value is (2, 14, 2, 14), which may
> > > 464:      * greatly differ from other LookAndFeel defaults.
> > 
> > This will need a CSR, which is fine.
> > But
> > (1) Basic is an abstract L&F .. it can't be the place to say what Metal does \
> > since most L&Fs derive from Basic And something about a JButton ought to be \
> > closer to ButtonUI or JButton. (2) Do we have precedent for calling out the exact \
> > number of (user space) pixels ? I'd prefer some more waffly wording like 
> > "The default margins may vary greatly depending on the L&F".
> 
> For (1), I've considered some alternative spots for this note. MetalButtonUI and \
> MetalLookAndFeel for example, but I didn't see any natural spot to add this to. I \
> decided to put it in BasicLookAndFeel since the default value is set here, but I \
> see your point. I can add an additional note in the doc for, say, MetalButtonUI if \
> preferred. 
> In (2), you're right that there's no explicit pixel count for anything. I'll update \
> this to be more general. Do you think this additional note is enough to avoid \
> adding more doc changes to Metal classes?

I have removed the doc change from this PR to focus mainly on fixing the regression \
first. Could you re-review when you get the chance?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16869#discussion_r1411110847


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic