Re: Review Request JDK-6897701 : In Nimbus Disabled Menus and Menu Items don't look disabled

[Semyon Sadetsky]

Looks good.

--Semyon

On 4/1/2016 11:18 AM, Prem Balakrishnan wrote:

Hi Alexander,

Please review the updated patch as per review comments.

http://cr.openjdk.java.net/~arajkumar/prem/6897701/webrev.02/ <http://cr.openjdk.java.net/%7Earajkumar/prem/6897701/webrev.02/>

Regards,
Prem

*From:*Alexander Scherbatiy
*Sent:* Thursday, March 31, 2016 10:45 PM

*To:* Prem Balakrishnan; Semyon Sadetsky; Sergey Bylokhov; Rajeev Chamyal; swing-dev@openjdk.java.net *Subject:* Re: Review Request JDK-6897701 : In Nimbus Disabled Menus and Menu Items don't look disabled

On 31/03/16 15:30, Prem Balakrishnan wrote:

Hi Semyon and Alexander

Thankyou for the review.

As per review comments,

Suggested fix will affect JRadioButton and JCheckBox states.

Hence modified with the NEW Fix .

Also Test automated.

Please review the updated patch.

http://cr.openjdk.java.net/~arapte/prem/6897701/webrev.01/ <http://cr.openjdk.java.net/%7Earapte/prem/6897701/webrev.01/>

 - JMenu extends JMenuItem so the "c instanceof JMenu" is not necessary
 - The caught exception in the test should be re-thrown

 Thanks,
 Alexandr.

Regards,

Prem

*From:*Semyon Sadetsky
*Sent:* Wednesday, March 30, 2016 10:30 PM

*To:* Prem Balakrishnan; Sergey Bylokhov; Alexander Scherbatiy; Rajeev Chamyal; swing-dev@openjdk.java.net <mailto:swing-dev@openjdk.java.net> *Subject:* Re: Review Request JDK-6897701 : In Nimbus Disabled Menus and Menu Items don't look disabled

Hi Prem,

In the block above the line you've changed:

764 if ((context.getComponentState() & SynthConstants.DISABLED) != 0) { 765 //This component is disabled, so return the disabled color. 766 //In some cases this means ignoring the color specified by the 767 //developer on the component. In other cases it means using a 768 //specified disabledTextColor, such as on JTextComponents. 769 //For example, JLabel doesn't specify a disabled color that the 770 //developer can set, yet it should have a disabled color to the 771 //text when the label is disabled. This code allows for that.

does not it solve the similar problem? Maybe it would be just better to add JMenu and JMenuItem case in it?

I'm just not sure that the condition (state == SynthConstants.ENABLED) will work if the menu item is a radio or a checkbox because the may have SynthConstants.SELECTED flag as well. Will this work for selected radio & checkbox?

--Semyon

On 3/30/2016 3:02 PM, Prem Balakrishnan wrote:

    Hi*,*

    Please review fix for JDK9,

    *Bug:*https://bugs.openjdk.java.net/browse/JDK-6897701

    *Webrev:*http://cr.openjdk.java.net/~arapte/prem/6897701/webrev.00/ 
<http://cr.openjdk.java.net/%7Earapte/prem/6897701/webrev.00/>

    *Issue:*

    In Nimbus Disabled Menus and Menu Items don't look disabled

    *Cause:*

    For JMenu and JMenuItem, Developer specified TEXT_FOREGROUND color
    was set even when they were Disabled.

    (Instead of setting Color for the State)

    *Fix:*

    When JMenu and JMenuItem is disabled, Color for the State is used.

    *Test: *Manual Test(since visual validation is required)

    Regards,
    Prem