[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-swing-dev
Subject:    Re: <Swing Dev> [9] Review Request for 7072653: JComboBox popup mispositioned if its height exceeds 
From:       Alexander Zvegintsev <alexander.zvegintsev () oracle ! com>
Date:       2015-04-28 13:21:06
Message-ID: 553F8942.4030100 () oracle ! com
[Download RAW message or body]

Still looks good to me too.

--
Thanks,
Alexander.

On 28.04.2015 14:30, Alexander Scherbatiy wrote:
>
>   The fix looks good to me.
>
>   Thanks,
>   Alexandr.
>
> On 4/27/2015 7:43 PM, Semyon Sadetsky wrote:
>>
>> On 4/27/2015 4:58 PM, Alexander Scherbatiy wrote:
>>> On 4/23/2015 2:02 PM, Semyon Sadetsky wrote:
>>>> Updated fix: http://cr.openjdk.java.net/~ssadetsky/7072653/webrev.02/
>>>>
>>>> (Aqua LnF had own logic need to be fixed as well as it was noticed 
>>>> by Alexander Z.)
>>>>
>>>> Alexander,
>>>>
>>>> This is because the screenBounds are in relative coordinates and we 
>>>> need to check if there are enough space to show popup above the combo.
>>>     The screenBoundsare created in two ways: gc.getBounds() and new 
>>> Rectangle(p, toolkit.getScreenSize()).
>>>
>>>    The javadoc for the getBounds() claims: "In a multi-screen 
>>> environment with a virtual device, the bounds can have negative X or 
>>> Y origins."
>>>    Does your solution work both for the positive and negative Y value?
>>>
>>>   Thanks,
>>>   Alexandr.
>> Yes, it now works for muli-monitor env. I specially pointed that in 
>> the description.
>> Did you find any issues?
>>
>> --Semyon
>>
>>>
>>>>
>>>> --Semyon
>>>>
>>>> On 4/22/2015 2:33 PM, Alexander Scherbatiy wrote:
>>>>> On 4/21/2015 5:31 PM, Semyon Sadetsky wrote:
>>>>>> Hi Alexander,
>>>>>>
>>>>>> Thanks for the review.
>>>>>> The updated webrev: 
>>>>>> http://cr.openjdk.java.net/~ssadetsky/7072653/webrev.01/
>>>>>
>>>>> 1299         if (py + ph > screenBounds.y + screenBounds.height) {
>>>>> 1300             if (ph <= -screenBounds.y - borderHeight) {
>>>>>
>>>>>
>>>>>   Is it correct to compare the popup height with the 
>>>>> screenBounds.y on the line 1300?
>>>>>
>>>>>  Thanks,
>>>>>  Alexandr.
>>>>>
>>>>>>
>>>>>> --Semyon
>>>>>>
>>>>>>
>>>>>> On 4/21/2015 5:17 PM, Alexander Zvegintsev wrote:
>>>>>>> Hello Semyon,
>>>>>>>
>>>>>>> it looks like that second call to getBorder() is unnecessary, we 
>>>>>>> already have its result in popupBorder variable.
>>>>>>> borderHeight looks strange to me, it is initialized with left 
>>>>>>> and right insets(related to width) and then used as height.
>>>>>>>
>>>>>>> The test uses full screen bounds and does not subtracts screen 
>>>>>>> insets.
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Alexander.
>>>>>>>
>>>>>>> On 04/16/2015 04:14 PM, Semyon Sadetsky wrote:
>>>>>>>> Subject corrected.
>>>>>>>>
>>>>>>>> On 4/16/2015 3:55 PM, Semyon Sadetsky wrote:
>>>>>>>>> Hello,
>>>>>>>>>
>>>>>>>>> Please review fix for JDK9:
>>>>>>>>>
>>>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-7072653
>>>>>>>>> webrev: http://cr.openjdk.java.net/~ssadetsky/7072653/webrev.00/
>>>>>>>>>
>>>>>>>>> *THE ROOT CAUSE
>>>>>>>>> Incorrect bounds calculation of heavy weight popup window in 
>>>>>>>>> the BasicComboPopup when the requested popup menu height 
>>>>>>>>> exceeds screen height.
>>>>>>>>> Additional issue found: popup border thickness is not taken 
>>>>>>>>> into account when popup direction switched to up as the result 
>>>>>>>>> popup overlaps its combo-box by 2 pixels.
>>>>>>>>> Also in multi-monitor desktop if screen are arranged 
>>>>>>>>> vertically popup can be shown on another monitor.
>>>>>>>>>
>>>>>>>>> *SOLUTION
>>>>>>>>> The popup location algorithm is corrected to take into account 
>>>>>>>>> the current screen height and border insets.
>>>>>>>>>
>>>>>>>>> *TESTING
>>>>>>>>> Test is added to ensure popup window height when combo-box 
>>>>>>>>> contains 1000 items.
>>>>>>>>>
>>>>>>>>> --Semyon
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>

[prev in list] [next in list] [prev in thread] [next in thread]