On Tue, 6 Dec 2022 16:46:19 GMT, Michael Strauß <mstra...@openjdk.org> wrote:
>> The `Scene.KeyHandler` class doesn't seem to have a clear purpose, mixing
>> focus handling with event propagation. Since #852,
>> `KeyHandler.setFocusVisible` is also called from mouse and touch event
>> handlers, which makes the purpose of the class even less pronounced.
>>
>> Moving the focus-related functionality next to the other focus functions in
>> the `Scene` class makes it easier to work with the code in the future.
>>
>> With the focus-related functions gone, `KeyHandler` only contains a single,
>> small method that is called from `Scene.processKeyEvent`. For simplicity,
>> this code can be rolled into `Scene.processKeyEvent` and the now-empty
>> `KeyHandler` class can be removed.
>
> Michael Strauß has updated the pull request with a new target base due to a
> merge or a rebase. The pull request now contains three commits:
>
> - Merge branch 'master' into fixes/keyhandler-refactor
> - review changes
> - Removed KeyHandler
Change looks good to me. Added a comment which a second reviewer should look
too.
Did some sanity testing with Focus handling, All behavior seemed same as before.
modules/javafx.graphics/src/main/java/javafx/scene/Scene.java line 2279:
> 2277: }
> 2278:
> 2279: private void setFocusOwner(Node node, boolean focusVisible) {
The earlier method `setFocusVisible()` is now merged into this method
`setFocusOwner()`. and also the call to this method from `requestFocus()` is
changed.
That seems Ok to me, But would like a second pair of eye on it.
-------------
PR: https://git.openjdk.org/jfx/pull/962
