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.
