RFR: 8297554: Remove Scene.KeyHandler

Ambarish Rapte arapte at openjdk.org
Tue Dec 6 08:17:17 UTC 2022


On Thu, 24 Nov 2022 06:49:02 GMT, Michael Strauß <mstrauss at 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.

The changes look equivalent to the code before. 
Suggesting minor changes.
Shall do some sanity testing with focus and update.

modules/javafx.graphics/src/main/java/javafx/scene/Scene.java line 2091:

> 2089:      **************************************************************************/
> 2090: 
> 2091:     private void windowForSceneChanged(Window oldWindow, Window window) {

Minor: rename the second parameter `window` to `newWindow`. 
As the method source is shown modified in the patch it seems Okay to make this change.

modules/javafx.graphics/src/main/java/javafx/scene/Scene.java line 2208:

> 2206: 
> 2207:     private class FocusOwnerProperty extends ReadOnlyObjectWrapper<Node> {
> 2208:         Node oldValue;

The variable name `oldFocusOwner` seems more readable and using the same name as before will reduce the diff.
Could you please change the name to `oldFocusOwner`

-------------

Changes requested by arapte (Reviewer).

PR: https://git.openjdk.org/jfx/pull/962


More information about the openjfx-dev mailing list