<Swing Dev> [15] RFR JDK-8216329: Cannot resize CheckBoxItemMenu in Synth L&F with setHorizontalTextPosition

[Pankaj Bansal]

Hello Prasanta,

Thanks for the review.
I don’t think this field needs to be accessed outside the class windowsMenuItemUI. So we should keep it private only. Please let me know if you think otherwise, else I will be pushing this as it is.

Regards,
Pankaj

-----Original Message-----
From: Prasanta Sadhukhan 
Sent: Tuesday, February 18, 2020 3:40 PM
To: Pankaj Bansal <pankaj.b.bansal at oracle.com>; swing-dev at openjdk.java.net
Subject: Re: <Swing Dev> [15] RFR JDK-8216329: Cannot resize CheckBoxItemMenu in Synth L&F with setHorizontalTextPosition

Looks ok to me. Only thing I have some doubt is "propertyChangeListener" 
object is "private" in windowsMenuItemUI whereas it was "protected" in BasicMenuItemUI. I guess it should be protected too.

Regards

Prasanta

On 08-Feb-20 4:14 AM, Sergey Bylokhov wrote:
> Looks fine.
>
> On 2/5/20 5:00 am, Pankaj Bansal wrote:
>> Hello Sergey,
>>
>> << ok.
>> <<Do we need to remove the listener added to the menuItem?
>> <<I guess it will be added every time we change L&F to the windows 
>> and will never be removed.
>>
>>
>> Yes, it should be done.
>> Webrev: http://cr.openjdk.java.net/~pbansal/8216329/webrev02/
>>
>> Regards,
>> Pankaj
>>
>> -----Original Message-----
>> From: Sergey Bylokhov
>> Sent: Thursday, January 30, 2020 5:59 AM
>> To: Pankaj Bansal <pankaj.b.bansal at oracle.com>; 
>> swing-dev at openjdk.java.net
>> Subject: Re: <Swing Dev> [15] RFR JDK-8216329: Cannot resize 
>> CheckBoxItemMenu in Synth L&F with setHorizontalTextPosition
>>
>> On 1/29/20 2:25 am, Pankaj Bansal wrote:
>>> One more point, I am able to reproduce the current issue with Synth 
>>> LookAndFeel in all platforms without fix and it works fine with the 
>>> fix.
>>
>> ok.
>>
>> Do we need to remove the listener added to the menuItem?
>> I guess it will be added every time we change L&F to the windows and 
>> will never be removed.
>>
>>
>>>
>>> Regards,
>>> Pankaj
>>>
>>> -----Original Message-----
>>> From: Pankaj Bansal
>>> Sent: Wednesday, January 29, 2020 3:19 PM
>>> To: Sergey Bylokhov; swing-dev at openjdk.java.net
>>> Subject: Re: <Swing Dev> [15] RFR JDK-8216329: Cannot resize 
>>> CheckBoxItemMenu in Synth L&F with setHorizontalTextPosition
>>>
>>> Hello Sergey,
>>>
>>> << Can you please double check that it is not possible to reproduce
>>> JDK-8152981 even if the test is modified in some way?
>>> <<For example if some other "basic" L&F will be used(Motif, Aqua)?
>>> I changed the test in JDK-8152981 to run on all installed 
>>> LookAndFeels on windows, linux and Mac after removing the windows 
>>> only condition. The tests passes on all platforms with all 
>>> LookAndFeels with the current fix.
>>> I can check in this change in JDK-8152981  test along with the 
>>> current fix if needed, though I feel it is not required as the issue 
>>> was originally only in WindowsLookAndFeel.
>>>
>>> Regards,
>>> Pankaj Bansal
>>>
>>> -----Original Message-----
>>> From: Sergey Bylokhov
>>> Sent: Wednesday, January 29, 2020 1:17 PM
>>> To: Pankaj Bansal; swing-dev at openjdk.java.net
>>> Subject: Re: <Swing Dev> [15] RFR JDK-8216329: Cannot resize 
>>> CheckBoxItemMenu in Synth L&F with setHorizontalTextPosition
>>>
>>> On 1/28/20 4:33 pm, Sergey Bylokhov wrote:
>>>> On 1/27/20 7:15 am, Pankaj Bansal wrote:
>>>>> << It is not a big issue, but for such a fix we will need a proper 
>>>>> specification and CSR, it is like adding a new method to the 
>>>>> public class. It is preferable to try to fix it in some other way 
>>>>> first.
>>>>> I did not realize earlier that this can be done by making changes 
>>>>> in WindowsMenuItemUI without calling the updateCheckIcon by moving 
>>>>> the code in updateCheckIcon method in WindowsMenuItemUI class. I 
>>>>> have made the changes for the same and all works fine. Also, I 
>>>>> have removed the updateCheckIcon method from BasicMenuItemUI class 
>>>>> as it is not needed.
>>>>
>>>> Can you please double check that it is not possible to reproduce
>>>> JDK-8152981 even if the test is modified in some way?
>>>
>>> For example if some other "basic" L&F will be used(Motif, Aqua)?
>>>
>>>
>>
>>
>
>