RFR: 8340852: ScrollPane should not consume navigation keys when it doesn't have direct focus

[John Hendrikx]

On Thu, 26 Sep 2024 21:17:55 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

> This change modifies `ScrollPaneBehavior` to only consume keys that are targetted at it.  As `KeyEvent`s are in almost all cases only intended for the targetted node (as visually that's where the user expects the keyboard input to go, as per normal UI rules) consuming key events that bubble up is simply incorrect.  When the `ScrollPane` is focused directly (it has the focused border) then it is fine for it to respond to all kinds of keys.
> 
> In FX controls normally there is no need to check if a `Control` is focused (although they probably should **all** do this) as they do not have children that could have received the Key Events involved, and Key Events are always sent to the focused Node.  When `ScrollPane` was developed this was not taken into account, leading to it consuming keys not intended for it.
> 
> This fixes several unexpected problems for custom control builders.  A custom control normally benefits from standard navigation out of the box (TAB/shift+TAB) and directional keys.  However, this breaks down as soon as this custom control is positioned within a `ScrollPane`, which is very surprising behavior and not at all expected.  This makes it harder than needed for custom control developers to get the standard navigation for the directional keys, as they would have to specifically capture those keys before they reach the `ScrollPane` and trigger the correct navigation action themselves (for which as of this writing there is no public API).
> 
> The same goes for all the other keys captured by `ScrollPane` when it does not have focus, although not as critical as the UP/DOWN/LEFT/RIGHT keys.

Here is a minimal working example that demonstrates the issue when people create a custom control.

Note that the standard controls and the custom control all have directional navigation working, except in the case where the custom control is embedded inside a `ScrollPane`.  This creates a big burden on custom control developers as to make their control work when used in a scroll pane, they now need to implement their own navigation...


package app;

import javafx.application.Application;
import javafx.scene.Node;
import javafx.scene.Scene;
import javafx.scene.control.Button;
import javafx.scene.control.Label;
import javafx.scene.control.ScrollPane;
import javafx.scene.control.Skin;
import javafx.scene.layout.GridPane;
import javafx.scene.layout.HBox;
import javafx.scene.layout.StackPane;
import javafx.scene.layout.VBox;
import javafx.stage.Stage;

public class App extends Application {

    @Override
    public void start(Stage primaryStage) {

        GridPane gp = new GridPane(10, 10);

        gp.add(new VBox(new Label("Standard Buttons in normal container"), new HBox(5, new Button("A"), new Button("B"), new Button("C"))), 0, 0);
        gp.add(new VBox(new Label("Standard Buttons in ScrollPane"), new ScrollPane(new HBox(5, new Button("A"), new Button("B"), new Button("C")))), 1, 0);
        gp.add(new VBox(new Label("Custom Buttons in normal container"), new HBox(5, new CustomButton("A"), new CustomButton("B"), new CustomButton("C"))), 0, 1);
        gp.add(new VBox(new Label("Custom Buttons in ScrollPane"), new ScrollPane(new HBox(5, new CustomButton("A"), new CustomButton("B"), new CustomButton("C")))), 1, 1);

        Scene scene = new Scene(gp);

        primaryStage.setScene(scene);

        primaryStage.show();
    }

    static class CustomButton extends Button {

        CustomButton(String title) {
            super(title);

            setSkin(new CustomButtonSkin(this));
        }
    }

    static class CustomButtonSkin implements Skin<Button> {
        private final Button control;
        private final StackPane container;

        public CustomButtonSkin(Button button) {
            this.control = button;
            this.container = new StackPane();
            this.container.getChildren().add(new Label(button.getText()));
        }

        @Override
        public Button getSkinnable() {
            return control;
        }

        @Override
        public Node getNode() {
            return container;
        }

        @Override
        public void dispose() {
            container.getChildren().clear();
        }
    }
}

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

PR Comment: https://git.openjdk.org/jfx/pull/1582#issuecomment-2385454753