<Swing Dev> [13] RFR JDK-8226964 [Yaru] - GTK L&F: There is no difference between menu selected and de-selected

[Pankaj Bansal]

Hi Phil,

 

<< nit pick : you spelled luminance wrong here : 

<<int actualLuminaceDifference

Corrected

 

<< Some of the variable naming is confusing me.

<< So until I get clarity on the intent here I can't verify the correctness of
the logic that follows although I understand your intent as being to make
the highlight lighter so long as it is already lighter than the background and
you don't have to exceed max luminance to do it, and if that won't work make
the highlight darker instead. And so forth for the other cases.

 

Yes, this is exactly what I am doing. I understand your point about naming. Once I have defined what is highlight and what is background, I should just use these for naming instead of menubar and menuitem. I have corrected the naming of variables used. 

 

webrev: http://cr.openjdk.java.net/~pbansal/8226964/webrev01/

 

Regards,

Pankaj

From: Philip Race 
Sent: Thursday, August 1, 2019 6:16 AM
To: Pankaj Bansal
Cc: swing-dev at openjdk.java.net
Subject: Re: <Swing Dev> [13] RFR JDK-8226964 [Yaru] - GTK L&F: There is no difference between menu selected and de-selected

 

nit pick : you spelled luminance wrong here : 

int actualLuminaceDifference

Some of the variable naming is confusing me.

Color highlightColor = style.getGTKColor(
+                        GTKEngine.WidgetType.MENU_ITEM.ordinal(),
 
ok so Highlight comes from MENU_ITEM
 
and background comes from MENU_BAR :
+                Color backgroundColor = style.getGTKColor(
+                        GTKEngine.WidgetType.MENU_BAR.ordinal(),

but then we assign menubar luminance from highlight which was got from menu item
and menu item luminance from background which was got from menu bar 

  double menubarLuminance = getY(highlightColor.getRed(),
+                        highlightColor.getGreen(), highlightColor.getBlue());
+                double menuitemLuminance = getY(backgroundColor.getRed(),
+                        backgroundColor.getGreen(), backgroundColor.getBlue());

Can you explain this ? Or is it wrong ?

I get further confused when I read .. 

+                    // when the highlight has more luminance and it's luminance
+                    // can be increased further by minLuminanceDifference
+                    if ((menubarLuminance < menuitemLuminance) &&

.. so menuItemLuminance is greater implying it is the highlight but we derived
it from the background.

So until I get clarity on the intent here I can't verify the correctness of
the logic that follows although I understand your intent as being to make
the highlight lighter so long as it is already lighter than the background and
you don't have to exceed max luminance to do it, and if that won't work make
the highlight darker instead. And so forth for the other cases.

-phil.



On 7/31/19, 3:16 PM, Pankaj Bansal wrote: 

Hi All,

Please review the following fix for jdk13.


Bug: 

https://bugs.openjdk.java.net/browse/JDK-8226964

 

webrev

HYPERLINK "http://cr.openjdk.java.net/%7Epbansal/8226964/webrev00/"http://cr.openjdk.java.net/~pbansal/8226964/webrev00/

 

 

Issue:

The menu is not getting highlighted on being selected. 

The issue is only reproducible on Yaru theme as the menu highlight color used by java is very similar to the background color of menubar. So the highlight is not properly visible due to poor contrast. 

 

Fix:

The fix checks that if the background menubar color and highlight color are very close, make the highlight more darker or lighter, so that it is clearly visible.

 

Testing:

The fix can be verified by running SwingSet2. I have verified this on Ubuntu 16.04, 18.04, 19.04 and OEL 7.5.


Regards,
Pankaj Bansal

 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/swing-dev/attachments/20190801/da66371c/attachment-0001.html>