On Sat, 11 Apr 2020 09:53:46 GMT, Ambarish Rapte <ara...@openjdk.org> wrote:
> The issue occurs because the key events are consumed by the `ListView` in > `Popup`, which displays the items. > This is a regression of > [JDK-8077916](https://bugs.openjdk.java.net/browse/JDK-8077916). This change > aadded several > `KeyMapping`s for focus traversals to `ListView`, which consume the `Left`, > `Right` and `Ctrl+A` key events. > Fix: > 1. Remove the four focus traversal arrow `KeyMapping`s from > `ListViewBehavior`. > 2. Add the `Ctrl + A` `KeyMapping` to `ListViewBehavior` only if the > `ListView`'s selection mode is set to > `SelectionMode.MULTIPLE`. `ComboBox` uses the `ListView` with > `SelectionMode.SINGLE` mode. > Change unrelated to fix: > `ComboBoxListViewBehavior` adds `KeyMapping` for `Up` and `Down` keys, which > are not invoked when the `ComboBox` popup > is showing. When the popup is shown, the Up and Down key events are handled > by the `ListView` and the `KeyMapping` code > from `ComboBoxListViewBehavior` does not get executed. These KeyMapping are > removed. However this change is not needed > for the fix. But this seems to be dead code. Verification: > Added new unit tests to verify the change. > Also verified that the behavior ListView behaves same before and after this > change. Hi @arapte, Great work on the PR 👍 Don't you think that all the changes in `ListViewSkin` can be moved to `ListViewBehavior`? All that we do in the skin class is to call `ListViewBehavior#updateSelectionModeKeyMapping`, which smells like feature envy. Moreover, `ListViewBehavior` already has change listener attached to `selectionModelProperty`, waiting for us to re-use it 😉 ------------- PR: https://git.openjdk.java.net/jfx/pull/172
