[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