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

[Prasanta Sadhukhan]

OK.

On 18-Feb-20 3:58 PM, Pankaj Bansal wrote:
> 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)?
>>>>
>>>>
>>>
>>