<Swing Dev> [9] Review request for JDK-8147521 [macosx] Internal API Usage: setPopupType used to force creation of heavyweight popup

[Phil Race]

Approved

-phil.

On 05/24/2016 05:27 AM, Rajeev Chamyal wrote:
> Hello Phil,
>
> Please review the updated webrev.
> http://cr.openjdk.java.net/~rchamyal/8147521/webrev.07/
> Changes: Updated the Javadoc.
> Added the following note:
> * This method is intended to be used only by  PopupFactory sub-classes.
>
> Regards,
> Rajeev Chamyal
>
> -----Original Message-----
> From: Alexander Scherbatiy
> Sent: 12 May 2016 17:05
> To: Sergey Bylokhov; Rajeev Chamyal; swing-dev at openjdk.java.net; Alan Snyder
> Subject: Re: <Swing Dev> [9] Review request for JDK-8147521 [macosx] Internal API Usage: setPopupType used to force creation of heavyweight popup
>
>
> The fix looks good to me.
>
> Thanks,
> Alexandr.
>
> On 12/05/16 15:26, Sergey Bylokhov wrote:
>> Looks fine to me.
>>
>> On 12.05.16 14:07, Rajeev Chamyal wrote:
>>> Hello All,
>>>
>>> Please review the updated webrev.
>>>
>>> http://cr.openjdk.java.net/~rchamyal/8147521/webrev.06/
>>> Changes: Updated the documentation.
>>>
>>> Regards,
>>> Rajeev Chamyal
>>>
>>> -----Original Message-----
>>> From: Rajeev Chamyal
>>> Sent: 12 May 2016 13:32
>>> To: Sergey Bylokhov; Alexander Scherbatiy;
>>> swing-dev at openjdk.java.net; Alan Snyder
>>> Subject: RE: <Swing Dev> [9] Review request for JDK-8147521 [macosx]
>>> Internal API Usage: setPopupType used to force creation of
>>> heavyweight popup
>>>
>>> Hello Sergey,
>>>
>>> Please review the updated webrev as per review comments.
>>>
>>> http://cr.openjdk.java.net/~rchamyal/8147521/webrev.05/
>>>
>>> Regards,
>>> Rajeev Chamyal
>>>
>>> -----Original Message-----
>>> From: Sergey Bylokhov
>>> Sent: 11 May 2016 18:16
>>> To: Rajeev Chamyal; Alexander Scherbatiy; swing-dev at openjdk.java.net;
>>> Alan Snyder
>>> Subject: Re: <Swing Dev> [9] Review request for JDK-8147521 [macosx]
>>> Internal API Usage: setPopupType used to force creation of
>>> heavyweight popup
>>>
>>> Code change looks fine.
>>> I think that the spec of isHeavyWeightPopup() should be updated. the
>>> "true" means that the HW popup will be forced, but the false does not
>>> mean that LW will be used(I guess in this case some default type
>>> will/can be selected).
>>>
>>> On 11.05.16 14:45, Rajeev Chamyal wrote:
>>>> Hello Alexandr,
>>>>
>>>> Please review the updated webrev.
>>>>
>>>> http://cr.openjdk.java.net/~rchamyal/8147521/webrev.04/
>>>>
>>>> Regards,
>>>> Rajeev Chamyal
>>>>
>>>> -----Original Message-----
>>>> From: Alexander Scherbatiy
>>>> Sent: 11 May 2016 16:29
>>>> To: Rajeev Chamyal
>>>> Cc: swing-dev at openjdk.java.net; Sergey Bylokhov; Alan Snyder
>>>> Subject: Re: <Swing Dev> [9] Review request for JDK-8147521 [macosx]
>>>> Internal API Usage: setPopupType used to force creation of
>>>> heavyweight popup
>>>>
>>>> On 5/11/2016 1:17 PM, Rajeev Chamyal wrote:
>>>>> Hello All,
>>>>>
>>>>> Please review the updated webrev.
>>>>> Bug : https://bugs.openjdk.java.net/browse/JDK-8147521
>>>>>
>>>>> Webrev: http://cr.openjdk.java.net/~rchamyal/8147521/webrev.03/
>>>>>
>>>>> Update: Added a new protected method getpop in PopupFactory.java
>>>>> with a Boolean parameter for specifying heavy weight popup.
>>>>      The provided method looks good to me.
>>>>
>>>>      I would leave the javadoc review to the native speakers. What I
>>>> only see that there is a missed space in {@codey}, typo in
>>>> "paramaters" word and probably there should be a comma before
>>>> "otherwise false".
>>>>
>>>>     Thanks,
>>>>     Alexandr.
>>>>
>>>>> Regards,
>>>>> Rajeev Chamyal
>>>>>
>>>>> -----Original Message-----
>>>>> From: Alan Snyder [mailto:javalists at cbfiddle.com]
>>>>> Sent: 11 May 2016 03:39
>>>>> To: Alexandr Scherbatiy
>>>>> Cc: swing-dev at openjdk.java.net
>>>>> Subject: Re: <Swing Dev> [9] Review request for JDK-8147521
>>>>> [macosx] Internal API Usage: setPopupType used to force creation of
>>>>> heavyweight popup
>>>>>
>>>>> I use only heavy weight.
>>>>>
>>>>>
>>>>>> On May 10, 2016, at 12:46 PM, Alexandr Scherbatiy
>>>>>> <alexandr.scherbatiy at oracle.com> wrote:
>>>>>>
>>>>>>
>>>>>> Do you need to use medium-weight popups in your application or
>>>>>> light/heavy-weight popups are enough?
>>>>>>
>>>>>> Thanks,
>>>>>> Alexandr.
>>>>>>
>>>>>> On 5/10/2016 10:23 PM, Alan Snyder wrote:
>>>>>>>> On May 10, 2016, at 5:58 AM, Sergey Bylokhov
>>>>>>>> <Sergey.Bylokhov at oracle.com> wrote:
>>>>>>>>
>>>>>>>> Hi, Alan.
>>>>>>>> Can you please take a look to the proposed solutions? Thanks.
>>>>>>>>
>>>>>>>>
>>>>>>> Approach 2 matches what I currently do. The problem noted by
>>>>>>> Rajeev does not happen because my popup factory calls
>>>>>>> setPopupType() on each call to the public getPopup() before
>>>>>>> invoking the superclass method.
>>>>>>>
>>>>>>> I think the original version of Approach 1 would work. My factory
>>>>>>> would override the public getPopup() method and pass true to the
>>>>>>> five parameter method.
>>>>>>>
>>>>>>> I think the revised version of Approach 1 does not work for me
>>>>>>> because the new flag is only tested if the first attempt to
>>>>>>> create a popup fails.
>>>>>>>
>>>
>>> --
>>> Best regards, Sergey.
>>>
>>