I noticed following problem with the current webrev.
http://cr.openjdk.java.net/~rchamyal/8147521/webrev.01/

If we don't override the BasicPopupMenuUI :: getPopup method and use the below 
code for displaying popup.

JPopupMenu jpopup = new JPopupMenu();
PopupFactory popupFactory = PopupFactory.getSharedInstance();

//sets popup type to be heavy weight
popupFactory.setHeavyWeightPopupEnabled(true);
jpopup.show(frame, e.getX(), e.getY());

The JPopupMenu ::showPopup() method resets the popup type to lightweight. 
Unless we set popupMenu.setLightWeightPopupEnabled(false);

Regards,
Rajeev Chamyal

-----Original Message-----
From: Alexandr Scherbatiy 
Sent: 10 May 2016 20:06
To: Sergey Bylokhov; Rajeev Chamyal; 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

On 5/10/2016 3:05 PM, Sergey Bylokhov wrote:

> Since both of these methods are public I am not sure how it will solve 
> the initial requested problem: enable HW popup w/o possibility of 
> change it by the user's code?

    I believe that a user can do something like:
    --------------
    JPopupMenu popupMenu = new JPopupMenu();
    popupMenu.setLightWeightPopupEnabled(true);
    popupMenu.showPopup();
   -------------
  and it overrides the popup factory type.
  Probably the JPopupMenu.showPopup() should be updated to create a popup with 
necessary type without rewriting the type from popup factory.

   Thanks,
   Alexandr.

>
> On 10.05.16 14:49, Alexandr Scherbatiy wrote:
>>     What should be results of the
>> PopupFactory.setHeavyWeightPopupEnabled(false) call in case if the 
>> current popup factory type is heavy-weight? Is it correct to leave it 
>> as heavy weight or it should be changed to another type?
>>
>>   It is better to use {@code } tag instead of <code> in a javadoc 
>> description.
>>   The test extends JPanel and is created on main thread. It is better 
>> to move its creation to EDT to avoid possible race conditions between 
>> using the JPanel on both main and EDT threads.
>>
>>   Thanks,
>>   Alexandr.
>>>
>>>
>>>
>>> Regards,
>>>
>>> Rajeev Chamyal
>>>
>>>
>>>
>>> *From:*Alexandr Scherbatiy
>>> *Sent:* 10 May 2016 16:07
>>> *To:* Rajeev Chamyal; Sergey Bylokhov; 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
>>>
>>>
>>>
>>> On 5/10/2016 1:10 PM, Rajeev Chamyal wrote:
>>>
>>>     Hello Alexandr,
>>>
>>>
>>>
>>>     Thanks for the review.
>>>
>>>     Please review the updated webrev.
>>>
>>> <http://cr.openjdk.java.net/%7Erchamyal/8147521/webrev.00/>http://cr
>>> .openjdk.java.net/~rchamyal/8147521/webrev.00/
>>>
>>>
>>>
>>>     Updates: Added a boolean field heavyWeightPopupEnabled and
>>>     corresponding property methods.
>>>
>>>     Applications can set this property to true for forcing popup to be
>>>     heavyweight.
>>>
>>>    Is it possible to implement setHeavyWeightPopupEnabled() method 
>>> body as setPopupType(HEAVY_WEIGHT_POPUP) and the
>>> isHeavyWeightPopupEnabled() as checking that current popup factory 
>>> type is HEAVY_WEIGHT_POPUP?
>>>
>>>   Thanks,
>>>   Alexandr.
>>>
>>>
>>>
>>>     I am not sure about applications using MEDIUM_WEIGHT_POPUP, but in
>>>     JDK source ToolTipManager.java /showTipWindow/ method sets popup
>>>     to be MEDIUM_WEIGHT_POPUP.
>>>
>>>
>>>
>>>     Regards,
>>>
>>>     Rajeev Chamyal
>>>
>>>
>>>
>>>
>>>
>>>     *From:*Alexandr Scherbatiy
>>>     *Sent:* 10 May 2016 12:02
>>>     *To:* Rajeev Chamyal; Sergey Bylokhov; 
>>> <mailto:swing-dev@openjdk.java.net>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
>>>
>>>
>>>
>>>
>>>     The first approach implies that a user should change all
>>>     PopupFactory.getPopup(owner, contents, x, y) calls to
>>>     OverridenPopupFactory.getPopup(owner, contents, x, y, true) to get
>>>     a heavy-weight popup in his code.
>>>
>>>     The second one looks better to me. It may have sense to restrict
>>>     it only to 2 possibilities: heavy-weight and light-weight popup if
>>>     the medium-weight popup is not really required to be used by users
>>>     applications.
>>>     Something like:
>>> setHeavyWeightPopupEnabled(boolean)/isHeavyWeightPopupEnabled().
>>>
>>>     Thanks,
>>>     Alexandr.
>>>
>>>     On 5/6/2016 2:44 PM, Rajeev Chamyal wrote:
>>>
>>>         Hello All,
>>>
>>>
>>>
>>>         Please review the below 2 webrevs.
>>>
>>>
>>>
>>>         Webrev:
>>>
>>> http://cr.openjdk.java.net/~rchamyal/8147521/webrev.app.00/
>>> <http://cr.openjdk.java.net/%7Erchamyal/8147521/webrev.app.00/>
>>>
>>> http://cr.openjdk.java.net/~rchamyal/8147521/webrev.app.01/
>>> <http://cr.openjdk.java.net/%7Erchamyal/8147521/webrev.app.01/>
>>>
>>>
>>>
>>>         Bug : https://bugs.openjdk.java.net/browse/JDK-8147521
>>>
>>>
>>>
>>>         Approach 1:
>>>
>>> http://cr.openjdk.java.net/~rchamyal/8147521/webrev.app.00/
>>> <http://cr.openjdk.java.net/%7Erchamyal/8147521/webrev.app.00/>
>>>
>>>
>>>
>>>         A new protected API is provided in PopupFactory.java.
>>>
>>>         protected Popup getPopup(Component owner, Component contents,
>>>         int x, int y,
>>>
>>>         boolean isHeavyWeightPopup)
>>>
>>>
>>>
>>>         Applications can override the new protected method and pass
>>>         true value to isHeavyWeightPopup for forcing popup to be heavy
>>>         weight.
>>>
>>>         Passing false will result in default behaviour.
>>>
>>>
>>>
>>>         Approach 2:
>>>
>>> http://cr.openjdk.java.net/~rchamyal/8147521/webrev.app.01/
>>> <http://cr.openjdk.java.net/%7Erchamyal/8147521/webrev.app.01/>
>>>
>>>
>>>
>>>         In this approach access level of existingmethodssetPopupType
>>>         andgetPopupType has been changed to protected.
>>>
>>>         Applications can override these methods to set or return
>>>         different types of popups.
>>>
>>>
>>>
>>>         Following  values can be passed to setPopupType.
>>>
>>>         0 : LIGHT_WEIGHT_POPUP
>>>
>>>         1 : MEDIUM_WEIGHT_POPUP
>>>
>>>         2: HEAVY_WEIGHT_POPUP
>>>
>>>
>>>
>>>
>>>
>>>         Regards,
>>>
>>>         Rajeev Chamyal
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>
>
>

Reply via email to