fix: fix shortcuts when listenOn is a dialog (#10264)

This makes it possible for a component to specify a client side only
element that should be used for listening keydown events on when the
component is setup as the listenOn target for a Shortcut. This is needed
for components like Vaadin Dialog which transport the dialog contents
inside an overlay that only exists on the client side and thus breaks
shortcut event handling.

Related to #7799, vaadin/vaadin-dialog#229

flow-html-components-testbench/src/main/java/com/vaadin/flow/component/html/testbench/NativeButtonElement.java

@@ -27,3 +27,4 @@
 public class NativeButtonElement extends TestBenchElement {
 
 }
+

flow-server/src/main/java/com/vaadin/flow/component/ShortcutRegistration.java

@@ -46,6 +46,18 @@
 public class ShortcutRegistration implements Registration, Serializable {
     static final String LISTEN_ON_COMPONENTS_SHOULD_NOT_CONTAIN_NULL = "listenOnComponents should not contain null!";
     static final String LISTEN_ON_COMPONENTS_SHOULD_NOT_HAVE_DUPLICATE_ENTRIES = "listenOnComponents should not have duplicate entries!";
+    static final String ELEMENT_LOCATOR_JS = //@formatter:off
+            "const listenOn=this;" // this is the listenOn component's element
+            + "const delegate=%1$s;" // the output of the JsLocator
+            + "if (delegate) {"
+            + "delegate.addEventListener('keydown', function(event) {"
+            + "const new_event = new event.constructor(event.type, event);"
+            + "listenOn.dispatchEvent(new_event);"
+            + "event.preventDefault();" // the propagation and default actions are left for the new event
+            + "event.stopPropagation();});"
+            + "} else {"
+            + "throw \"Shortcut listenOn element not found with JS locator string '%1$s'\""
+            + "}";//@formatter:on
     private boolean allowDefaultBehavior = false;
     private boolean allowEventPropagation = false;
 
@@ -560,7 +572,6 @@ private void updateHandlerListenerRegistration(int listenOnIndex) {
         if (shortcutListenerRegistrations[listenOnIndex] == null) {
             if (component.getUI().isPresent()) {
                 shortcutListenerRegistrations[listenOnIndex] = new CompoundRegistration();
-
                 Registration keyDownRegistration = ComponentUtil
                         .addListener(component, KeyDownEvent.class, e -> {
                             if (lifecycleOwner.isVisible() && lifecycleOwner
@@ -574,6 +585,7 @@ private void updateHandlerListenerRegistration(int listenOnIndex) {
                         });
                 shortcutListenerRegistrations[listenOnIndex]
                         .addRegistration(keyDownRegistration);
+                setupKeydownEventDelegateIfNeeded(component);
             }
         } else {
             configureHandlerListenerRegistration(listenOnIndex);
@@ -745,6 +757,21 @@ private static String generateEventKeyFilter(Key key) {
                 + ".indexOf(event.key) !== -1)";
     }
 
+    /*
+     * #7799, vaadin/vaadin-dialog#229 This could be changed to use a generic
+     * feature for DOM events by either using existing DOM event handling or by
+     * using the JS execution return channel feature.
+     */
+    private void setupKeydownEventDelegateIfNeeded(Component listenOn) {
+        final String elementLocatorJs = (String) ComponentUtil.getData(listenOn,
+                Shortcuts.ELEMENT_LOCATOR_JS_KEY);
+        if (elementLocatorJs != null) {
+            final String jsExpression = String.format(ELEMENT_LOCATOR_JS,
+                    elementLocatorJs);
+            listenOn.getElement().executeJs(jsExpression);
+        }
+    }
+
     /**
      * Wraps a {@link Key} instance. Makes it easier to compare the keys and
      * store them by hash.

flow-server/src/main/java/com/vaadin/flow/component/Shortcuts.java

@@ -17,6 +17,7 @@
 package com.vaadin.flow.component;
 
 import com.vaadin.flow.server.Command;
+import com.vaadin.flow.shared.Registration;
 
 
 /**
@@ -39,6 +40,7 @@
  */
 public final class Shortcuts {
     static final String NULL = "Parameter '%s' must not be null!";
+    static final String ELEMENT_LOCATOR_JS_KEY = "_element_locator_js_key";
 
     private Shortcuts() {
     }
@@ -128,4 +130,48 @@ public static ShortcutRegistration addShortcutListener(
         return new ShortcutRegistration(lifecycleOwner, () -> new Component[] { lifecycleOwner.getUI().get() },
                 listener, key).withModifiers(keyModifiers);
     }
+
+    /**
+     * Setup an element, that is only accessible on the browser (not server
+     * side), to listen for shortcut events on and delegate to the given
+     * component. The element will be located in the browser by executing the
+     * given JS statement. This needs to be set for each listen-on component
+     * instance.
+     * <p>
+     * <b>This should be only used by component developers</b>, when their
+     * component is used as the {@code listenOn} component for shortcuts, and
+     * their component does some magic on the browser which means that the
+     * shortcut events are not coming through from the actual element. Thus when
+     * an application developer calls e.g. <br>
+     * {@code myButton.addClickShortcut(Key.ENTER).listenOn(dialog);} <br>
+     * the framework will automatically make sure the events are passed from the
+     * browser only element to the listenOn component (dialog in this case).
+     *
+     * @param elementLocatorJs
+     *            js execution string that references the desired element in DOM
+     *            or {@code null} to any remove existing locator
+     * @param listenOnComponent
+     *            the component that is setup for listening shortcuts on
+     *            {@link ShortcutRegistration#listenOn(Component...)}
+     * @return a registration for removing the locator, does not affect active
+     *         shortcuts or if the locator has changed from what was set for
+     *         this registration
+     * @since
+     */
+    public static Registration setShortcutListenOnElement(
+            String elementLocatorJs, Component listenOnComponent) {
+        // a bit wasteful to store this for each component instance but it
+        // stays in memory only as long as the component itself does
+        ComponentUtil.setData(listenOnComponent, ELEMENT_LOCATOR_JS_KEY,
+                elementLocatorJs);
+        return () -> {
+            if (elementLocatorJs != null
+                    && elementLocatorJs.equals(ComponentUtil.getData(
+                            listenOnComponent, ELEMENT_LOCATOR_JS_KEY))) {
+                ComponentUtil.setData(listenOnComponent, ELEMENT_LOCATOR_JS_KEY,
+                        null);
+            }
+        };
+    }
+
 }

flow-server/src/test/java/com/vaadin/flow/component/ShortcutRegistrationTest.java

@@ -17,6 +17,7 @@
 package com.vaadin.flow.component;
 
 import java.util.Arrays;
+import java.util.List;
 import java.util.Optional;
 
 import org.junit.Assert;
@@ -27,11 +28,13 @@
 import org.mockito.ArgumentCaptor;
 import org.mockito.Mockito;
 
+import com.vaadin.flow.component.internal.PendingJavaScriptInvocation;
 import com.vaadin.flow.dom.Element;
 import com.vaadin.flow.function.SerializableConsumer;
 import com.vaadin.flow.internal.ExecutionContext;
 import com.vaadin.flow.internal.nodefeature.ElementListenerMap;
 import com.vaadin.flow.internal.nodefeature.ElementListenerMapUtil;
+import com.vaadin.flow.server.VaadinSession;
 import com.vaadin.flow.shared.Registration;
 
 import static org.junit.Assert.assertArrayEquals;
@@ -274,7 +277,7 @@ public void listenOnUIIsClosing_eventIsPopulatedForANewUI() {
         }, Key.KEY_A);
 
         UI newUI = Mockito.spy(UI.class);
-        // close the prevopus UI
+        // close the previous UI
         ui.close();
         components[0] = newUI;
 
@@ -363,6 +366,56 @@ public void listenOnItemsAreChangedAfterCallingListenOnShouldNotHaveAnyEffect()
         assertTrue(registration.isShortcutActive());
     }
 
+    @Test
+    public void listenOnComponentHasElementLocatorJs_jsExecutionScheduled() {
+        VaadinSession session = Mockito.mock(VaadinSession.class);
+        Mockito.when(session.hasLock()).thenReturn(true);
+        UI ui = Mockito.spy(UI.class);
+        ui.getInternals().setSession(session);
+
+        Component owner = new FakeComponent();
+        Component initialComponentToListenOn = new FakeComponent();
+
+        Component[] components = new Component[] { initialComponentToListenOn };
+
+        ui.add(owner);
+        ui.add(initialComponentToListenOn);
+
+        final String elementLocatorJs = "foobar";
+        final Registration registration = Shortcuts
+                .setShortcutListenOnElement(elementLocatorJs,
+                        initialComponentToListenOn);
+
+        new ShortcutRegistration(owner, () -> components, event -> {
+        }, Key.KEY_A);
+
+        ui.getInternals().getStateTree().runExecutionsBeforeClientResponse();
+
+        List<PendingJavaScriptInvocation> pendingJavaScriptInvocations = ui
+                .getInternals().dumpPendingJavaScriptInvocations();
+        Assert.assertEquals(1, pendingJavaScriptInvocations.size());
+
+        final PendingJavaScriptInvocation js = pendingJavaScriptInvocations
+                .get(0);
+        final String expectedExecutionString = "return (function() { "
+                + String.format(ShortcutRegistration.ELEMENT_LOCATOR_JS,
+                        elementLocatorJs)
+                + "}).apply($0)";
+        Assert.assertEquals(expectedExecutionString,
+                js.getInvocation().getExpression());
+
+        registration.remove();
+
+        new ShortcutRegistration(owner, () -> components, event -> {
+        }, Key.KEY_A);
+
+        ui.getInternals().getStateTree().runExecutionsBeforeClientResponse();
+
+        pendingJavaScriptInvocations = ui.getInternals()
+                .dumpPendingJavaScriptInvocations();
+        Assert.assertEquals(0, pendingJavaScriptInvocations.size());
+    }
+
     /**
      * Works only with the {@code registration} member variable.
      *

flow-server/src/test/java/com/vaadin/flow/component/ShortcutsTest.java

@@ -24,6 +24,9 @@
 import org.junit.Assert;
 import org.junit.Test;
 
+import com.vaadin.flow.router.RouterLink;
+import com.vaadin.flow.shared.Registration;
+
 public class ShortcutsTest {
 
     @Test
@@ -36,9 +39,50 @@ public void hasOnlyStaticMethods() {
                         method.getName(),
                         Stream.of(method.getParameterTypes())
                                 .map(Class::getSimpleName)
-                                .collect(Collectors.joining(", "))
-                ));
+                                .collect(Collectors.joining(", "))));
             }
         }
     }
+
+    @Test
+    public void setShortcutListenOnElementLocatorJs_storesLocatorOnComponentData() {
+        final RouterLink routerLink = new RouterLink();
+        final String locator = "foobar";
+        final Registration registration = Shortcuts
+                .setShortcutListenOnElement(locator, routerLink);
+
+        Assert.assertEquals(locator, ComponentUtil.getData(routerLink,
+                Shortcuts.ELEMENT_LOCATOR_JS_KEY));
+
+        registration.remove();
+
+        Assert.assertNull(ComponentUtil.getData(routerLink,
+                Shortcuts.ELEMENT_LOCATOR_JS_KEY));
+    }
+
+    @Test
+    public void setShortcutListenOnElementLocatorJs_registrationDoesNotRemoveModifiedData_nullClearsAlways() {
+        final RouterLink routerLink = new RouterLink();
+        final String locator = "foobar";
+        final Registration registration = Shortcuts
+                .setShortcutListenOnElement(locator, routerLink);
+
+        Assert.assertEquals(locator, ComponentUtil.getData(routerLink,
+                Shortcuts.ELEMENT_LOCATOR_JS_KEY));
+
+        Shortcuts.setShortcutListenOnElement("another", routerLink);
+
+        registration.remove();
+
+        Assert.assertEquals("another", ComponentUtil.getData(routerLink,
+                Shortcuts.ELEMENT_LOCATOR_JS_KEY));
+
+        final Registration nullRegistration = Shortcuts
+                .setShortcutListenOnElement(null, routerLink);
+
+        Assert.assertNull(ComponentUtil.getData(routerLink,
+                Shortcuts.ELEMENT_LOCATOR_JS_KEY));
+
+        nullRegistration.remove();
+    }
 }

flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/ComplexDialogShortcutView.java

@@ -0,0 +1,45 @@
+package com.vaadin.flow.uitest.ui;
+
+import com.vaadin.flow.component.Shortcuts;
+import com.vaadin.flow.component.html.Div;
+import com.vaadin.flow.router.PreserveOnRefresh;
+import com.vaadin.flow.router.Route;
+
+@PreserveOnRefresh
+@Route(value = "com.vaadin.flow.uitest.ui.ComplexDialogShortcutView")
+public class ComplexDialogShortcutView extends DialogShortcutView {
+
+    public static final String OVERLAY_ID = "overlay";
+    private Div fakeOverlayElement;
+
+    public ComplexDialogShortcutView() {
+    }
+
+    @Override
+    public void open(Dialog dialog) {
+        super.open(dialog);
+        // the shortcut listener is not added yet, add the element locator data
+        final String overlayId = OVERLAY_ID + dialog.index;
+        final String overlayFetchJS = "document.getElementById('" + overlayId
+                + "')";
+        Shortcuts.setShortcutListenOnElement(overlayFetchJS,
+                dialog);
+        // simulate a fake overlay element
+        fakeOverlayElement = new Div();
+        fakeOverlayElement.setId(overlayId);
+        getUI().ifPresent(ui -> ui.add(fakeOverlayElement));
+        // transport the dialog contents to overlay element
+        dialog.getElement().executeJs(
+                overlayFetchJS + ".appendChild(this.firstElementChild);");
+        dialog.content.getStyle().set("position", "fixed").set("inset",
+                "calc(50% + " + (dialog.index * 100) + "px)");
+    }
+
+    @Override
+    public void close(Dialog dialog) {
+        super.close(dialog);
+
+        fakeOverlayElement.getUI()
+                .ifPresent(ui -> ui.remove(fakeOverlayElement));
+    }
+}