On Thu, 7 Nov 2024 09:05:02 GMT, Damon Nguyen <[email protected]> wrote:
>> In a JComboBox, if the user opens the dropdown list and clicks and holds the
>> down-button, then ALT-TABs to switch focus, when the user re-focuses the
>> frame with the JComboBox and opens the dropdown list again, the list will
>> still be scrolling even though the down-button isn't pressed.
>>
>> This isn't OS or L&F specific, although Aqua L&F does not have any
>> directional arrows in the dropdown list (and is thus exempt). This led me to
>> believe it could be handled in BasicComboBoxUI where focusLost and focusGain
>> are used or isPopupVisible but the scroll behavior cannot be altered here.
>> Likewise for BasicComboPopup where `autoscroll` is used. However, this
>> behavior isn't related to autoscroll and is actually found in the JScrollbar
>> of the JScrollpane inside of the JComboBox. The timer for the scroll action
>> starts but is never stopped if focus is lost, so a new listener is created
>> and used. The proposed solution uses `KeyboardFocusManager` to track the
>> focus owner. The listener stops the `scrollTimer` when the `focusOwner`
>> property is changed. With this change, the list no longer automatically
>> scrolls when re-focused and instead opens normally.
>>
>> The included test is manual due to the need to confirm that the list still
>> scrolls after ALT-TABing. The L&F is set to Metal since it is the
>> cross-platform lookandfeel and has directional buttons for the JScrollPane
>> list.
>
> Damon Nguyen has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Add space
src/java.desktop/share/classes/javax/swing/plaf/basic/BasicScrollBarUI.java
line 1244:
> 1242: } else if (decrButton.isFocusPainted()) {
> 1243: decrButton.doClick();
> 1244: }
Why this doClick block is required?
src/java.desktop/share/classes/javax/swing/plaf/basic/BasicScrollBarUI.java
line 1592:
> 1590:
> 1591: public void mousePressed(MouseEvent e) {
> 1592: System.out.println("MOUSEPRESSED: " + e);
Remove this println
src/java.desktop/share/classes/javax/swing/plaf/basic/BasicScrollBarUI.java
line 1685:
> 1683: >= trackListener.currentMouseY)
> 1684: ((Timer)e.getSource()).stop();
> 1685: } else if (getThumbBounds().y <=
> trackListener.currentMouseY) {
GUess you are not changing this block so no need for formatting cleanup in this
PR.. infact the "{" are also way off if you had to change...there are other
non-touched area like l1593...better to cleanup in separate cleanup..
test/jdk/javax/swing/JComboBox/JComboBoxScrollFocusTest.java line 43:
> 41: popup list. While holding left click, the list should be
> scrolling
> 42: down. Press ALT + TAB while holding down left click to switch
> 43: focus to a different window. Focus the test frame again and
> click
left click => left click button
focus to a different window => and release the left click mouse button
test/jdk/javax/swing/JComboBox/JComboBoxScrollFocusTest.java line 72:
> 70: frame.setSize(400, 200);
> 71: frame.setLocationRelativeTo(null);
> 72: frame.setVisible(true);
no need of setVisible, will be taken care by PFJ
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20845#discussion_r1836638995
PR Review Comment: https://git.openjdk.org/jdk/pull/20845#discussion_r1836639441
PR Review Comment: https://git.openjdk.org/jdk/pull/20845#discussion_r1836643345
PR Review Comment: https://git.openjdk.org/jdk/pull/20845#discussion_r1836644539
PR Review Comment: https://git.openjdk.org/jdk/pull/20845#discussion_r1836644960
