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@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@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@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@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:javali...@cbfiddle.com] >>>> Sent: 11 May 2016 03:39 >>>> To: Alexandr Scherbatiy >>>> Cc: swing-dev@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.scherba...@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.bylok...@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. >> > >