feat: reset focus on active element in ShortcutRegistration (#18126)

* feat: reset focus on active element in ShortcutRegistration Adds setResetFocusOnActiveElement, resetFocusOnActiveElement and isResetFocusOnActiveElement to ShortcutRegistration class to optionally reset focus on active element (focused element in browser). Resetting means calling blur() and focus() on the active element to ensure that input fields with ValueChangeMode ON_CHANGE will fire value change event before shortcut handler is run. Updates ClickNotifier#addClickShortcut(Key,KeyModifier...) to reset focus on active element by default for click shortcuts. Fixes: #5959 * test: fixed tests * chore: added null checking and text changes * chore: added window.Vaadin.Flow.resetFocus function * fix: revert addClickShortcut to not reset focus by default Removes a call `setResetFocusOnActiveElement(true)` in `addClickShortcut` method in `ClickNotifier` to revert default behavior changes from #17826. Fixes: #18048 * chore: update Reset focus on active element to work in 23 Adjusted reset focus script snippet from Flow 24 to work with 23. * chore: formatting --------- Co-authored-by: Mikhail Shabarov <[email protected]>
vaadin · Nov 30, 2023 · 09e4737 · 09e4737
1 parent b2bdaba
commit 09e4737
Show file tree

Hide file tree

Showing 10 changed files with 347 additions and 19 deletions.
diff --git a/flow-server/src/main/java/com/vaadin/flow/component/ShortcutRegistration.java b/flow-server/src/main/java/com/vaadin/flow/component/ShortcutRegistration.java
@@ -54,16 +54,21 @@ public class ShortcutRegistration implements Registration, Serializable {
                     + "if (delegate) {"
                     + "delegate.addEventListener('keydown', function(event) {"
                     + "if (%2$s) {" // the filter text to match the key
+                    + "%3$s" // allow reset focus on active element if desired
                     + "const new_event = new event.constructor(event.type, event);"
                     + "listenOn.dispatchEvent(new_event);"
-                    + "%3$s" // the new event allows default if desired
+                    + "%4$s" // the new event allows default if desired
                     + "event.stopPropagation();}" // the new event bubbles if desired
                     + "});" // end matches filter
                     + "} else {"
                     + "throw \"Shortcut listenOn element not found with JS locator string '%1$s'\""
                     + "}";//@formatter:on
+
     private boolean allowDefaultBehavior = false;
     private boolean allowEventPropagation = false;
+
+    private boolean resetFocusOnActiveElement = false;
+
     static final String LISTEN_ON_INITIALIZED = "_initialized_listen_on_for_component";
 
     private Set<Key> modifiers = new HashSet<>(2);
@@ -255,6 +260,20 @@ public ShortcutRegistration allowEventPropagation() {
         return this;
     }
 
+    /**
+     * Reset focus on active element before triggering shortcut event handler.
+     *
+     * @return this <code>ShortcutRegistration</code>
+     * @see #setResetFocusOnActiveElement(boolean)
+     */
+    public ShortcutRegistration resetFocusOnActiveElement() {
+        if (!resetFocusOnActiveElement) {
+            resetFocusOnActiveElement = true;
+            prepareForClientResponse();
+        }
+        return this;
+    }
+
     /**
      * Binds the shortcut's life cycle to that of the given {@link Component}.
      * When the given {@code component} is attached, the shortcut's listener is
@@ -426,6 +445,37 @@ public void setEventPropagationAllowed(boolean eventPropagationAllowed) {
         }
     }
 
+    /**
+     * Checks if shortcut resets focus on active element before triggering
+     * shortcut event handler.
+     *
+     * @return True when focus is reset on the active element, false otherwise.
+     */
+    public boolean isResetFocusOnActiveElement() {
+        return resetFocusOnActiveElement;
+    }
+
+    /**
+     * Set if shortcut resets focus on active element before triggering shortcut
+     * event handler. Reset means to first <code>blur()</code> and then
+     * <code>focus()</code> the element. If element is an input field with a
+     * ValueChangeMode.ON_CHANGE, then resetting focus ensures that value change
+     * event is triggered before shortcut event is handled. Active element is
+     * <code>document.activeElement</code> or
+     * <code>shadowRoot.activeElement</code> if active element is the
+     * <code>shadowRoot</code>.
+     *
+     * @param resetFocusOnActiveElement
+     *            Reset focus on active element
+     */
+    public void setResetFocusOnActiveElement(
+            boolean resetFocusOnActiveElement) {
+        if (resetFocusOnActiveElement != this.resetFocusOnActiveElement) {
+            this.resetFocusOnActiveElement = resetFocusOnActiveElement;
+            prepareForClientResponse();
+        }
+    }
+
     /**
      * {@link Component} which owns the first shortcuts key event listener.
      *
@@ -626,8 +676,10 @@ private void configureHandlerListenerRegistration(int listenOnIndex) {
                 if (!allowEventPropagation) {
                     filterText += " && (event.stopPropagation() || true)";
                 }
+                if (resetFocusOnActiveElement) {
+                    filterText += " && window.Vaadin.Flow.resetFocus()";
+                }
                 listenerRegistration.setFilter(filterText);
-
                 shortcutActive = true;
             });
         }
@@ -812,11 +864,14 @@ private void setupKeydownEventDelegateIfNeeded(Component listenOn) {
             // #10362 only prevent default when key filter matches to not block
             // typing or existing shortcuts
             final String filterText = filterText();
+            final String focusJs = resetFocusOnActiveElement
+                    ? "window.Vaadin.Flow.resetFocus();"
+                    : "";
             // enable default actions if desired
             final String preventDefault = allowDefaultBehavior ? ""
                     : "event.preventDefault();";
             final String jsExpression = String.format(ELEMENT_LOCATOR_JS,
-                    elementLocatorJs, filterText, preventDefault);
+                    elementLocatorJs, filterText, focusJs, preventDefault);
 
             final String expressionHash = StringUtil.getHash(jsExpression);
             final Set<String> expressions = getOrInitListenData(listenOn);

diff --git a/flow-server/src/main/java/com/vaadin/flow/server/frontend/AbstractUpdateImports.java b/flow-server/src/main/java/com/vaadin/flow/server/frontend/AbstractUpdateImports.java
@@ -80,6 +80,10 @@ abstract class AbstractUpdateImports implements Runnable {
     private static final String THEMABLE_MIXIN_IMPORT = "import { css, unsafeCSS, registerStyles } from '@vaadin/vaadin-themable-mixin';";
     private static final String REGISTER_STYLES_FOR_TEMPLATE = CSS_IMPORT + "%n"
             + "registerStyles('%s', $css_%1$d%s);";
+    static final String RESET_FOCUS_JS = "() => {"
+            + " let ae=document.activeElement;"
+            + " while(ae&&ae.shadowRoot) ae = ae.shadowRoot.activeElement;"
+            + " return !ae || ae.blur() || ae.focus() || true;" + "}";
 
     private static final String IMPORT_TEMPLATE = "import '%s';";
 
@@ -121,6 +125,9 @@ public void run() {
         lines.addAll(getExportLines());
         lines.addAll(getThemeLines());
         lines.addAll(getCssLines());
+        lines.add("window.Vaadin = window.Vaadin || {};");
+        lines.add("window.Vaadin.Flow = window.Vaadin.Flow || {};");
+        lines.add("window.Vaadin.Flow.resetFocus = " + RESET_FOCUS_JS);
         if (!productionMode && useLegacyV14Bootstrap) {
             // This is only needed for v14bootstrap mode
             lines.add(TaskGenerateBootstrap.DEV_TOOLS_IMPORT);

diff --git a/flow-server/src/test/java/com/vaadin/flow/component/ShortcutRegistrationTest.java b/flow-server/src/test/java/com/vaadin/flow/component/ShortcutRegistrationTest.java
@@ -148,6 +148,17 @@ public void preventDefaultAndStopPropagationValuesDefaultToTrue() {
         assertTrue(registration.isEventPropagationAllowed());
     }
 
+    @Test
+    public void resetFocusOnActiveElementValuesDefaultToTrue() {
+        ShortcutRegistration registration = new ShortcutRegistration(
+                lifecycleOwner, () -> listenOn, event -> {
+                }, Key.KEY_A);
+
+        assertFalse(registration.isResetFocusOnActiveElement());
+        registration.resetFocusOnActiveElement();
+        assertTrue(registration.isResetFocusOnActiveElement());
+    }
+
     @Test
     public void bindLifecycleToChangesLifecycleOwner() {
         Component newOwner = mock(Component.class);
@@ -174,23 +185,29 @@ public void settersAndGettersChangeValuesCorrectly() {
 
         registration.setBrowserDefaultAllowed(true);
         registration.setEventPropagationAllowed(true);
+        registration.setResetFocusOnActiveElement(true);
 
         clientResponse();
 
         assertTrue("Allow default was not set to true",
                 registration.isBrowserDefaultAllowed());
         assertTrue("Allow propagation was not set to true",
                 registration.isBrowserDefaultAllowed());
+        assertTrue("Reset focus on active element was not set to true",
+                registration.isResetFocusOnActiveElement());
 
         registration.setBrowserDefaultAllowed(false);
         registration.setEventPropagationAllowed(false);
+        registration.setResetFocusOnActiveElement(false);
 
         clientResponse();
 
         assertFalse("Allow default was not set to false",
                 registration.isBrowserDefaultAllowed());
         assertFalse("Allow propagation was not set to false",
                 registration.isEventPropagationAllowed());
+        assertFalse("Reset focus on active element was not set to false",
+                registration.isResetFocusOnActiveElement());
     }
 
     @Test
@@ -316,6 +333,9 @@ public void shortcutRegistrationReturnedByClickNotifierHasCorrectDefault() {
 
         assertFalse("Allows propagation was not false",
                 registration.isEventPropagationAllowed());
+
+        assertFalse("Reset focus on active element was not set to false",
+                registration.isResetFocusOnActiveElement());
     }
 
     @Test
@@ -330,6 +350,9 @@ public void shortcutRegistrationReturnedByFocusableHasCorrectDefaults() {
 
         assertFalse("Allows propagation was not false",
                 registration.isEventPropagationAllowed());
+
+        assertFalse("Reset focus on active element was not set to false",
+                registration.isResetFocusOnActiveElement());
     }
 
     @Test
@@ -399,6 +422,10 @@ public void listenOnComponentHasElementLocatorJs_jsExecutionScheduled() {
                 expression.contains("event.stopPropagation();"));
         Assert.assertTrue("JS execution string missing the key" + key,
                 expression.contains(key.getKeys().get(0)));
+        Assert.assertFalse(
+                "JS execution string should not have blur() and focus() on active element in it"
+                        + expression,
+                expression.contains("window.Vaadin.Flow.resetFocus()"));
 
         fixture.registration.remove();
 
@@ -426,6 +453,24 @@ public void listenOnComponentHasElementLocatorJs_allowBrowserDefault_JsExecution
                 expression.contains("event.preventDefault();"));
     }
 
+    @Test
+    public void listenOnComponentHasElementLocatorJs_resetFocusOnActiveElement_JsExecutionResetFocusOnActiveElement() {
+        final ElementLocatorTestFixture fixture = new ElementLocatorTestFixture();
+        final Key key = Key.KEY_A;
+        fixture.createNewShortcut(key).resetFocusOnActiveElement();
+
+        List<PendingJavaScriptInvocation> pendingJavaScriptInvocations = fixture
+                .writeResponse();
+
+        final PendingJavaScriptInvocation js = pendingJavaScriptInvocations
+                .get(0);
+        final String expression = js.getInvocation().getExpression();
+        Assert.assertTrue(
+                "JS execution string should have blur() and focus() on active element in it"
+                        + expression,
+                expression.contains("window.Vaadin.Flow.resetFocus()"));
+    }
+
     @Test
     public void constructedRegistration_lifecycleIsVisibleAndEnabled_shorcutEventIsFired() {
         AtomicReference<ShortcutEvent> event = new AtomicReference<>();

diff --git a/flow-server/src/test/java/com/vaadin/flow/server/frontend/AbstractUpdateImportsTest.java b/flow-server/src/test/java/com/vaadin/flow/server/frontend/AbstractUpdateImportsTest.java
@@ -497,6 +497,10 @@ public void assertFullSortOrder() throws MalformedURLException {
         List<String> expectedImports = new ArrayList<>();
         expectedImports.addAll(updater.getExportLines());
         expectedImports.addAll(updater.getThemeLines());
+        expectedImports.add("window.Vaadin = window.Vaadin || {};");
+        expectedImports.add("window.Vaadin.Flow = window.Vaadin.Flow || {};");
+        expectedImports.add("window.Vaadin.Flow.resetFocus = "
+                + AbstractUpdateImports.RESET_FOCUS_JS);
 
         getAnntotationsAsStream(JsModule.class, testClasses)
                 .map(JsModule::value).map(this::updateToImport).sorted()

diff --git a/...t/src/main/java/com/vaadin/flow/uitest/ui/ShadowRootShortcutsWithValueChangeModeView.java b/...t/src/main/java/com/vaadin/flow/uitest/ui/ShadowRootShortcutsWithValueChangeModeView.java
@@ -0,0 +1,85 @@
+/*
+ * Copyright 2000-2023 Vaadin Ltd.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package com.vaadin.flow.uitest.ui;
+
+import com.vaadin.flow.component.Key;
+import com.vaadin.flow.component.KeyModifier;
+import com.vaadin.flow.component.html.Div;
+import com.vaadin.flow.component.html.Input;
+import com.vaadin.flow.component.html.NativeButton;
+import com.vaadin.flow.component.html.Paragraph;
+import com.vaadin.flow.data.value.ValueChangeMode;
+import com.vaadin.flow.dom.Element;
+import com.vaadin.flow.dom.ElementFactory;
+import com.vaadin.flow.dom.ShadowRoot;
+import com.vaadin.flow.router.AfterNavigationEvent;
+import com.vaadin.flow.router.AfterNavigationObserver;
+import com.vaadin.flow.router.Route;
+import com.vaadin.flow.uitest.servlet.ViewTestLayout;
+
+@Route(value = "com.vaadin.flow.uitest.ui.ShadowRootShortcutsWithValueChangeModeView", layout = ViewTestLayout.class)
+public class ShadowRootShortcutsWithValueChangeModeView extends Div
+        implements AfterNavigationObserver {
+
+    private final Input input;
+
+    public ShadowRootShortcutsWithValueChangeModeView() {
+        setId("test-element");
+
+        ShadowRoot shadowRoot = getElement().attachShadow();
+        Element shadowDiv = ElementFactory.createDiv();
+        shadowDiv.setText("Div inside shadow DOM");
+        shadowDiv.setAttribute("id", "shadow-div");
+
+        input = new Input();
+        input.setId("input");
+        shadowDiv.appendChild(input.getElement());
+
+        NativeButton button = new NativeButton("Report value");
+        button.setId("button");
+
+        Paragraph value = new Paragraph();
+        value.setId("value");
+        value.setText("");
+
+        input.setValueChangeMode(ValueChangeMode.LAZY);
+        // make this really big to make testing easier
+        input.setValueChangeTimeout(3000);
+
+        // clickShortcutWorks
+        button.setText(
+                "Button triggered by CTRL + ALT + S and CTRL + ENTER (with reset focus)");
+        button.addClickShortcut(Key.KEY_S, KeyModifier.CONTROL,
+                KeyModifier.ALT);
+        button.addClickShortcut(Key.ENTER, KeyModifier.CONTROL)
+                .setResetFocusOnActiveElement(true);
+        button.addClickListener(e -> value.setText(input.getValue()));
+
+        shadowRoot.appendChild(shadowDiv);
+        shadowRoot.appendChild(button.getElement());
+        shadowRoot.appendChild(value.getElement());
+    }
+
+    @Override
+    public void afterNavigation(AfterNavigationEvent event) {
+        String valueChangeMode = event.getLocation().getQueryParameters()
+                .getQueryString();
+        if (valueChangeMode != null && !valueChangeMode.isBlank()) {
+            input.setValueChangeMode(ValueChangeMode.valueOf(valueChangeMode));
+        }
+    }
+}
diff --git a/flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/ShortcutsView.java b/flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/ShortcutsView.java
@@ -173,10 +173,11 @@ public ShortcutsView() {
 
         NativeButton clickButton2 = new NativeButton("CB2",
                 event -> actual.setValue("click: " + clickInput2.getValue()));
-        clickButton2.addClickShortcut(Key.ENTER).listenOn(wrapper2)
-                // this matches the default of other shortcuts but changes
-                // the default of the click shortcut
-                .setBrowserDefaultAllowed(false);
+        ShortcutRegistration reg = clickButton2.addClickShortcut(Key.ENTER)
+                .listenOn(wrapper2);
+        // this matches the default of other shortcuts but changes
+        // the default of the click shortcut
+        reg.setBrowserDefaultAllowed(false);
 
         wrapper1.add(clickInput1, clickButton1);
         wrapper2.add(clickInput2, clickButton2);
@@ -210,16 +211,30 @@ public ShortcutsView() {
         clickInput6.setType("text");
         clickInput6.setId("click-input-6");
 
+        final Input clickInput7 = new Input();
+        clickInput7.setType("text");
+        clickInput7.setId("click-input-7");
+
         NativeButton clickButton4 = new NativeButton("CB4",
                 event -> actual.setValue("click4: " + clickInput5.getValue()
                         + "," + clickInput6.getValue()));
         clickButton4.addClickShortcut(Key.ENTER)
                 .listenOn(clickInput5, clickInput6)
-                // this matches the default of other shortcuts but changes
-                // the default of the click shortcut
-                .setBrowserDefaultAllowed(false);
-
-        wrapper4.add(clickInput5, clickInput6, clickButton4);
+                // sets setResetFocusOnActiveElement
+                // to true and this will cause input value change being
+                // triggered no matter what
+                // setBrowserDefaultAllowed is.
+                .resetFocusOnActiveElement().setBrowserDefaultAllowed(false);
+
+        NativeButton clickButton5 = new NativeButton("CB5",
+                event -> actual.setValue("click5: " + clickInput7.getValue()));
+        reg = clickButton5.addClickShortcut(Key.ENTER).listenOn(clickInput7);
+        // this matches the default of other shortcuts but changes
+        // the default of the click shortcut
+        reg.setBrowserDefaultAllowed(false);
+
+        wrapper4.add(clickInput5, clickInput6, clickInput7, clickButton4,
+                clickButton5);
         add(wrapper4);
 
         // removingShortcutCleansJavascriptEventSettingsItUsed

diff --git a/...oot-context/src/main/java/com/vaadin/flow/uitest/ui/ShortcutsWithValueChangeModeView.java b/...oot-context/src/main/java/com/vaadin/flow/uitest/ui/ShortcutsWithValueChangeModeView.java
@@ -51,9 +51,12 @@ public ShortcutsWithValueChangeModeView() {
         input.setValueChangeTimeout(3000);
 
         // clickShortcutWorks
-        button.setText("Button triggered by CTRL + ALT + S");
+        button.setText(
+                "Button triggered by CTRL + ALT + S and CTRL + ENTER (with reset focus)");
         button.addClickShortcut(Key.KEY_S, KeyModifier.CONTROL,
                 KeyModifier.ALT);
+        button.addClickShortcut(Key.ENTER, KeyModifier.CONTROL)
+                .setResetFocusOnActiveElement(true);
         button.addClickListener(e -> value.setText(input.getValue()));
 
         add(input, button, value);