fix: make dialog scoped shortcuts work (#725)

Uses API provided by Flow to make keydown events passed from the overlay
to the Dialog element when the dialog has been used for listening for
shortcuts with .listenOn(dialog).

Depends on vaadin/flow#10264

Fixes vaadin/flow#7799, vaadin/vaadin-dialog#229

Co-authored-by: David Sosa <76832183+sosa-vaadin@users.noreply.github.com>

vaadin-dialog-flow-parent/vaadin-dialog-flow-integration-tests/src/main/java/com/vaadin/flow/component/dialog/tests/DialogWithShortcutPage.java

@@ -0,0 +1,117 @@
+package com.vaadin.flow.component.dialog.tests;
+
+import java.util.EventObject;
+
+import com.vaadin.flow.component.AttachEvent;
+import com.vaadin.flow.component.Component;
+import com.vaadin.flow.component.Key;
+import com.vaadin.flow.component.ShortcutRegistration;
+import com.vaadin.flow.component.Text;
+import com.vaadin.flow.component.dialog.Dialog;
+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.orderedlayout.VerticalLayout;
+import com.vaadin.flow.router.Route;
+
+@Route("vaadin-dialog/shortcuts")
+public class DialogWithShortcutPage extends VerticalLayout {
+
+    public static final Key SHORTCUT_KEY = Key.KEY_X;
+    public static final String SHORTCUT = String.join("", SHORTCUT_KEY.getKeys());
+    public static final String EVENT_LOG = "event-log";
+    public static final String UI_BUTTON = "ui-button";
+    public static final String MODELESS_SHORTCUT_ON_UI = "modeless-shortcut-on-ui";
+    public static final String MODELESS_SHORTCUT_LISTEN_ON_DIALOG = "modeless-shortcur-listen-on-dialog";
+    public static final String LISTEN_ON_DIALOG = "listen-on-dialog";
+    public static final String SHORTCUT_ON_UI = "shortcut-on-ui";
+    public static final String DIALOG_ID = "dialog";
+    public static final String REUSABLE_DIALOG = "reusable-dialog";
+    public static final String UI_ID = "ui-id";
+    public static final String DIALOG_BUTTON_MESSAGE_ID = "dialog-button-message";
+    private int eventCounter;
+    private int dialogCounter;
+
+    private final Div eventLog;
+    private Dialog reusableDialog;
+
+    public DialogWithShortcutPage() {
+        eventLog = new Div();
+        eventLog.setId(EVENT_LOG);
+        final NativeButton modelessWithShortcutOnUi = new NativeButton(
+                "Modeless with shortcut on UI",
+                e -> createAndOpenDialog(false).setModal(false));
+        modelessWithShortcutOnUi.setId(MODELESS_SHORTCUT_ON_UI);
+        final NativeButton modelessWithShortcutListenOnDialog = new NativeButton(
+                "Modeless with shortcut listenOn(dialog)",
+                e -> createAndOpenDialog(true).setModal(false));
+        modelessWithShortcutListenOnDialog
+                .setId(MODELESS_SHORTCUT_LISTEN_ON_DIALOG);
+        final NativeButton dialogWithShortcutListenOnDialog = new NativeButton(
+                "Dialog with shortcut listenOn(dialog)",
+                e -> createAndOpenDialog(true));
+        dialogWithShortcutListenOnDialog.setId(LISTEN_ON_DIALOG);
+        final NativeButton dialogWithShortcutOnUi = new NativeButton(
+                "Dialog with shortcut on UI", e -> createAndOpenDialog(false));
+        dialogWithShortcutOnUi.setId(SHORTCUT_ON_UI);
+        final NativeButton reusableDialogButton = new NativeButton("Reusable dialog",
+                event -> {
+                    if (reusableDialog == null) {
+                        reusableDialog = createAndOpenDialog(true);
+                    } else {
+                        reusableDialog.open();
+                    }
+                });
+        reusableDialogButton.setId(REUSABLE_DIALOG);
+        add(modelessWithShortcutOnUi, modelessWithShortcutListenOnDialog,
+                dialogWithShortcutOnUi, dialogWithShortcutListenOnDialog,
+                reusableDialogButton);
+
+        NativeButton nonDialogButton = new NativeButton("Button on UI with shortcut on UI",
+                this::onEvent);
+        nonDialogButton.addClickShortcut(SHORTCUT_KEY);
+        nonDialogButton.setId(UI_BUTTON);
+
+        add(nonDialogButton, eventLog);
+    }
+
+    @Override
+    protected void onAttach(AttachEvent attachEvent) {
+        super.onAttach(attachEvent);
+        attachEvent.getUI().setId(UI_ID);
+    }
+
+    private Dialog createAndOpenDialog(boolean listenOnDialog) {
+        int index = dialogCounter++;
+        final String dialogId = DIALOG_ID + index;
+        NativeButton myDialogButton = createDialogButton();
+        myDialogButton.setId(dialogId + "-button");
+        Dialog dialog = new Dialog(
+                new Div(new Div(new Text("" + index)), myDialogButton,
+                        new Input()));
+        NativeButton closeButton = new NativeButton("Close", buttonClickEvent -> dialog.close());
+        dialog.add(closeButton);
+        dialog.setDraggable(true);
+        dialog.open();
+        dialog.setId(dialogId);
+        final ShortcutRegistration registration = myDialogButton
+                .addClickShortcut(SHORTCUT_KEY);
+        if (listenOnDialog) {
+            registration.listenOn(dialog);
+        }
+        return dialog;
+    }
+
+    private void onEvent(EventObject event) {
+        final Div div = new Div();
+        final int index = eventCounter++;
+        div.setText(index + "-"
+                + ((Component) event.getSource()).getId().orElse("NO-ID!"));
+        div.setId(DIALOG_BUTTON_MESSAGE_ID + "-" + index);
+        eventLog.addComponentAsFirst(div);
+    }
+
+    private NativeButton createDialogButton() {
+        return new NativeButton("Hit " + SHORTCUT, this::onEvent);
+    }
+}

vaadin-dialog-flow-parent/vaadin-dialog-flow-integration-tests/src/test/java/com/vaadin/flow/component/dialog/tests/DialogWithShortcutIT.java

@@ -0,0 +1,165 @@
+package com.vaadin.flow.component.dialog.tests;
+
+import com.vaadin.flow.component.html.testbench.NativeButtonElement;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.openqa.selenium.By;
+import org.openqa.selenium.Keys;
+import org.openqa.selenium.WebElement;
+import org.openqa.selenium.interactions.Actions;
+
+import com.vaadin.flow.component.dialog.testbench.DialogElement;
+import com.vaadin.flow.component.html.testbench.DivElement;
+import com.vaadin.flow.testutil.TestPath;
+import com.vaadin.testbench.TestBenchElement;
+import com.vaadin.tests.AbstractComponentIT;
+
+@TestPath("vaadin-dialog/shortcuts")
+public class DialogWithShortcutIT extends AbstractComponentIT {
+
+    private TestBenchElement eventLog;
+    private TestBenchElement openDialogButton;
+    private NativeButtonElement uiLevelButton;
+
+    @Before
+    public void init() {
+        open();
+        eventLog = $(DivElement.class).id(DialogWithShortcutPage.EVENT_LOG);
+        uiLevelButton = $(NativeButtonElement.class)
+                .id(DialogWithShortcutPage.UI_BUTTON);
+    }
+
+    // #7799
+    @Test
+    public void dialogOpenedWithListenOnShortcut_sameShortcutListeningOnUi_focusDecidesWhichIsExecuted() {
+        openDialogButton = $(NativeButtonElement.class)
+                .id(DialogWithShortcutPage.LISTEN_ON_DIALOG);
+        pressShortcutKey(uiLevelButton);
+        validateLatestShortcutEvent(0, DialogWithShortcutPage.UI_BUTTON);
+
+        openNewDialog();
+        pressShortcutKey(getFirstDialogInput());
+        validateLatestShortcutEventOnDialog(1, 0);
+
+        pressShortcutKey(getFirstDialogInput());
+        validateLatestShortcutEventOnDialog(2, 0);
+
+        closeDialog();
+        pressShortcutKey(uiLevelButton);
+        validateLatestShortcutEvent(3, DialogWithShortcutPage.UI_BUTTON);
+    }
+
+    @Test
+    public void dialogOpenedWithShortcutNoListenOn_sameShortcutListeningOnUi_bothExecuted() {
+        openDialogButton = $(NativeButtonElement.class)
+                .id(DialogWithShortcutPage.SHORTCUT_ON_UI);
+        pressShortcutKey(uiLevelButton);
+        validateLatestShortcutEvent(0, DialogWithShortcutPage.UI_BUTTON);
+
+        openNewDialog();
+
+        pressShortcutKey(getFirstDialogInput());
+        // last event is on dialog
+        validateLatestShortcutEventOnDialog(2, 0);
+        validateShortcutEvent(1, 1, DialogWithShortcutPage.UI_BUTTON);
+
+        closeDialog();
+        pressShortcutKey(uiLevelButton);
+        validateLatestShortcutEvent(3, DialogWithShortcutPage.UI_BUTTON);
+    }
+
+    @Test
+    public void dialogOpenedWithListenOnShortcut_dialogReopened_oldShortcutStillWorks() {
+        openDialogButton = $(NativeButtonElement.class)
+                .id(DialogWithShortcutPage.REUSABLE_DIALOG);
+
+        pressShortcutKey(uiLevelButton);
+        validateLatestShortcutEvent(0, DialogWithShortcutPage.UI_BUTTON);
+
+        openNewDialog();
+
+        pressShortcutKey(getFirstDialogInput());
+        validateLatestShortcutEventOnDialog(1, 0);
+
+        pressShortcutKey(uiLevelButton);
+        validateLatestShortcutEvent(2, DialogWithShortcutPage.UI_BUTTON);
+
+        closeDialog();
+
+        pressShortcutKey(uiLevelButton);
+        validateLatestShortcutEvent(3, DialogWithShortcutPage.UI_BUTTON);
+
+        openNewDialog();
+
+        pressShortcutKey(getFirstDialogInput());
+        validateLatestShortcutEventOnDialog(4, 0);
+    }
+
+    // vaadin/vaadin-dialog#229
+    @Test
+    public void twoModelessDialogsOpenedWithSameShortcutKeyOnListenOn_dialogWithFocusExecuted() {
+        openDialogButton = $(NativeButtonElement.class)
+                .id(DialogWithShortcutPage.MODELESS_SHORTCUT_LISTEN_ON_DIALOG);
+
+        openNewDialog();
+        openNewDialog();
+
+        pressShortcutKey(getFirstDialogInput());
+        validateLatestShortcutEventOnDialog(0, 0);
+
+        pressShortcutKey(getDialogInput(1));
+        validateLatestShortcutEventOnDialog(1, 1);
+
+        pressShortcutKey(getFirstDialogInput());
+        validateLatestShortcutEventOnDialog(2, 0);
+
+        pressShortcutKey(uiLevelButton);
+        validateLatestShortcutEvent(3, DialogWithShortcutPage.UI_BUTTON);
+
+        pressShortcutKey(getDialogInput(1));
+        validateLatestShortcutEventOnDialog(4, 1);
+    }
+
+    private void openNewDialog() {
+        openDialogButton.click();
+    }
+
+    private void closeDialog() {
+        new Actions(getDriver()).sendKeys(Keys.ESCAPE).build().perform();
+    }
+
+    private TestBenchElement getFirstDialogInput() {
+        return getDialogInput(0);
+    }
+
+    private TestBenchElement getDialogInput(int dialogIndex) {
+        return $(DialogElement.class)
+                .id(DialogWithShortcutPage.DIALOG_ID + dialogIndex).$("input")
+                .first();
+    }
+
+    private void pressShortcutKey(TestBenchElement elementToFocus) {
+        elementToFocus.focus();
+        elementToFocus.sendKeys("x");
+    }
+
+    private void validateLatestShortcutEventOnDialog(int eventCounter,
+            int dialogId) {
+        validateShortcutEvent(0, eventCounter,
+                DialogWithShortcutPage.DIALOG_ID + dialogId + "-button");
+    }
+
+    private void validateLatestShortcutEvent(int eventCounter,
+            String eventSourceId) {
+        validateShortcutEvent(0, eventCounter, eventSourceId);
+    }
+
+    private void validateShortcutEvent(int indexFromTop, int eventCounter,
+            String eventSourceId) {
+        final WebElement latestEvent = eventLog.findElements(By.tagName("div"))
+                .get(indexFromTop);
+        Assert.assertEquals("Invalid latest event",
+                eventCounter + "-" + eventSourceId, latestEvent.getText());
+    }
+}

vaadin-dialog-flow-parent/vaadin-dialog-flow/src/main/java/com/vaadin/flow/component/dialog/Dialog.java

@@ -30,6 +30,8 @@
 import com.vaadin.flow.component.HasComponents;
 import com.vaadin.flow.component.HasSize;
 import com.vaadin.flow.component.HasStyle;
+import com.vaadin.flow.component.HasTheme;
+import com.vaadin.flow.component.Shortcuts;
 import com.vaadin.flow.component.UI;
 import com.vaadin.flow.component.dependency.HtmlImport;
 import com.vaadin.flow.component.dependency.JsModule;
@@ -47,6 +49,7 @@
 public class Dialog extends GeneratedVaadinDialog<Dialog>
         implements HasComponents, HasSize {
 
+    private static final String OVERLAY_LOCATOR_JS = "this.$.overlay";
     private Element template;
     private Element container;
     private boolean autoAddedToTheUi;
@@ -623,9 +626,47 @@ public Registration addDetachListener(
         return super.addDetachListener(listener);
     }
 
+
+    /**
+     * Adds theme variants to the component.
+     *
+     * @param variants
+     *            theme variants to add
+     */
+    public void addThemeVariants(DialogVariant... variants) {
+        getThemeNames()
+                .addAll(Stream.of(variants)
+                        .map(DialogVariant::getVariantName)
+                        .collect(Collectors.toList()));
+    }
+
+    /**
+     * Removes theme variants from the component.
+     *
+     * @param variants
+     *            theme variants to remove
+     */
+    public void removeThemeVariants(DialogVariant... variants) {
+        getThemeNames()
+                .removeAll(Stream.of(variants)
+                        .map(DialogVariant::getVariantName)
+                        .collect(Collectors.toList()));
+    }
+
+    @Override
+    protected void onAttach(AttachEvent attachEvent) {
+        super.onAttach(attachEvent);
+
+        // vaadin/flow#7799,vaadin/vaadin-dialog#229
+        // as the locator is stored inside component's attributes, no need to
+        // remove the data as it should live as long as the component does
+        Shortcuts.setShortcutListenOnElement(OVERLAY_LOCATOR_JS, this);
+    }
+
     private void setDimension(String dimension, String value) {
         getElement()
-            .executeJs("this.$.overlay.$.overlay.style[$0]=$1", dimension, value);
+                .executeJs(OVERLAY_LOCATOR_JS + ".$.overlay.style[$0]=$1",
+                        dimension, value);
     }
 
     private void attachComponentRenderer() {

vaadin-dialog-flow-parent/vaadin-dialog-flow/src/test/java/com/vaadin/flow/component/dialog/DialogTest.java

@@ -26,6 +26,8 @@
 import org.mockito.Mockito;
 
 import com.vaadin.flow.component.Component;
+import com.vaadin.flow.component.Key;
+import com.vaadin.flow.component.Shortcuts;
 import com.vaadin.flow.component.UI;
 import com.vaadin.flow.component.html.Div;
 import com.vaadin.flow.component.html.Label;
@@ -318,6 +320,29 @@ public void setModal_dialogCanBeModeless() {
         Assert.assertFalse("modal can be set to false", !dialog.getElement().getProperty("modeless", false));
     }
 
+    // vaadin/flow#7799,vaadin/vaadin-dialog#229
+    @Test
+    public void dialogAttached_targetedWithShortcutListenOn_addsJsExecutionForTransportingShortcutEvents() {
+        Dialog dialog = new Dialog();
+        dialog.open();
+        // there are a 6 invocations pending after opening a dialog (???) clear
+        // those first
+        ui.getInternals().getStateTree().runExecutionsBeforeClientResponse();
+        ui.getInternals().dumpPendingJavaScriptInvocations();
+
+        // adding a shortcut with listenOn(dialog) makes flow pass events from
+        // overlay to dialog so that shortcuts inside dialog work
+        Shortcuts.addShortcutListener(dialog, event -> {
+        }, Key.KEY_A).listenOn(dialog);
+        ui.getInternals().getStateTree().runExecutionsBeforeClientResponse();
+
+        final List<PendingJavaScriptInvocation> pendingJavaScriptInvocations = ui
+                .getInternals().dumpPendingJavaScriptInvocations();
+        Assert.assertEquals(
+                "Shortcut transferring invocation should be pending", 1,
+                pendingJavaScriptInvocations.size());
+    }
+
     private void addDivAtIndex(int index) {
         Dialog dialog = new Dialog();