On Thu, 7 Mar 2024 12:42:05 GMT, Abhishek Kumar <abhis...@openjdk.org> wrote:

>>> Backtracking a bit, regarding `Label.foreground` why it needs to be done 
>>> here?
>>> 
>>> Other L&F, it is added in corresponding LookAndFeel class for example, in 
>>> [Metal](https://github.com/openjdk/jdk/blob/master/src/java.desktop/share/classes/javax/swing/plaf/metal/MetalLookAndFeel.java#L1001)
>>>  and in 
>>> [Windows](https://github.com/openjdk/jdk/blob/master/src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/WindowsLookAndFeel.java#L880)
>>>  etc so I think it is needed to be added in GTKLookAndFeel class
>> 
>> IN GTKLookAndFeel, the color is returned specific to the style and state 
>> from native side . So, even if it is not added the default color for 
>> enabled, disabled or any other state is returned by native. Whereas In Metal 
>> the color is set by using UIManager define color.
>> 
>>> and additionally there is setting of Label.foreground and other Label. 
>>> property in BasicLabelUI.java 
>>> [installDefaults](https://github.com/openjdk/jdk/blob/master/src/java.desktop/share/classes/javax/swing/plaf/basic/BasicLabelUI.java#L368)
>> but we dont do it for Nimbus or GTK or Synth so I guess we need to call 
>> super.installDefaults(c) in SynthLabelUI class since there is no GTKLabelUI 
>> class
>> or
>> maybe do
>> LookAndFeel.installColorsAndFont(c, "Label.background", "Label.foreground", 
>> "Label.font"); alone to avoid other properties being set and to not fallback
>> 
>> This properties are set via SynthStyle 
>> [installDefaults](https://github.com/kumarabhi006/jdk/blob/master/src/java.desktop/share/classes/javax/swing/plaf/synth/SynthStyle.java#L928)
>>  method where the foreground, background are set by getting the value from 
>> `getColorForState` method. So, calling super.installDefaults(c) in 
>> SynthLabelUI will not make any difference.
>> 
>> I checked for JLabel properties set in enabled and disabled state. 
>> 
>> 
>> In Nimbus LAF
>>             UIManager.getDefaults().put("Label[Enabled].textForeground", 
>> labelColor);
>>             UIManager.getDefaults().put("Label[Disabled].textForeground", 
>> labelDisabledColor);
>> 
>> Other LAF
>>             UIManager.getDefaults().put("Label.foreground", labelColor);
>>             UIManager.getDefaults().put("Label.disabledForeground", 
>> labelDisabledColor);
>> 
>> 
>> It works as expected for Metal and GTK. For Nimbus LAF, disabled color is 
>> not painted correctly. In stead the default color specified in skin.laf is 
>> rendered.
>
>> It works as expected for Metal and GTK. For Nimbus LAF, disabled color is 
>> not painted correctly. In stead the default color specified in skin.laf is 
>> rendered.
> 
> For HTML text, disabled state doesn't render in LAF defined color for Metal 
> and GTK as well.

This looks weird… So you're saying `Label[Enabled].textForeground` and 
`Label[Disabled].textForeground` are used for Nimbus (and Synth and GTK) 
instead of `Label.foreground` and `Label.disabledForeground` which are used for 
other L&Fs.

Shouldn't we fix the problem by correcting the keys instead? It looks like it's 
what you're doing for specific components.

Is it specified anywhere that Synth-based L&Fs use different constants? It 
results in incorrect colors.

If a developer sets the common properties, should they override Look-and-Feel 
defaults?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17763#discussion_r1519696320

Reply via email to