fix: set shortcut listener to modal only if owner is child

The previous fix for #13205 moved all shortcut registration event source from UI to the modal component if one was active; this caused shortcuts owned by components not under the modal tree to also be moved if they were registered when a modal component was active, and hence they stopped working when the modal component tree was removed. This modifies the previous fix to only change the event source if the shortcut owner is under the an active modal tree.
vaadin · Mar 10, 2022 · 736af20 · 736af20
1 parent 5fa90d6
commit 736af20
Show file tree

Hide file tree

Showing 4 changed files with 68 additions and 15 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
@@ -532,16 +532,7 @@ private String filterText() {
     }
 
     private void updateHandlerListenerRegistration(int listenOnIndex) {
-        Component component = listenOnComponents[listenOnIndex];
-        assert component != null;
-
-        if (component instanceof UI) {
-            UIInternals uiInternals = ((UI) component).getInternals();
-            if (uiInternals.hasModalComponent()) {
-                component = uiInternals.getActiveModalComponent();
-            }
-        }
-        final Component source = component;
+        final Component component = getComponentEventSource(listenOnIndex);
 
         if (shortcutListenerRegistrations == null) {
             shortcutListenerRegistrations = new CompoundRegistration[listenOnComponents.length];
@@ -552,7 +543,8 @@ private void updateHandlerListenerRegistration(int listenOnIndex) {
                 shortcutListenerRegistrations[listenOnIndex] = new CompoundRegistration();
                 Registration keyDownRegistration = ComponentUtil.addListener(
                         component, KeyDownEvent.class,
-                        event -> fireShortcutEvent(source), domRegistration -> {
+                        event -> fireShortcutEvent(component),
+                        domRegistration -> {
                             shortcutListenerRegistrations[listenOnIndex]
                                     .addRegistration(domRegistration);
                             configureHandlerListenerRegistration(listenOnIndex);
@@ -566,6 +558,30 @@ private void updateHandlerListenerRegistration(int listenOnIndex) {
         }
     }
 
+    private Component getComponentEventSource(int listenOnIndex) {
+        Component component = listenOnComponents[listenOnIndex];
+        assert component != null;
+
+        if (component instanceof UI) {
+            UIInternals uiInternals = ((UI) component).getInternals();
+            // Shortcut events go to UI by default, which will be inert from
+            // receiving RPCs if there is an active modal component. In this
+            // case, check if the owner component is a child of the modal
+            // component, and if so, send via the modal component.
+            if (uiInternals.hasModalComponent()) {
+                Optional<Component> maybeAncestor = Optional.of(lifecycleOwner);
+                while (maybeAncestor.isPresent()) {
+                    Component ancestor = maybeAncestor.get();
+                    if (ancestor == uiInternals.getActiveModalComponent()) {
+                        return ancestor;
+                    }
+                    maybeAncestor = ancestor.getParent();
+                }
+            }
+        }
+        return component;
+    }
+
     private void fireShortcutEvent(Component component) {
         if (ancestorsOrSelfAreVisible(lifecycleOwner)
                 && lifecycleOwner.getElement().isEnabled()) {

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
@@ -502,6 +502,7 @@ public void uiRegistration_uiHasModalComponent_eventIsSentFromModalComponentInst
         when(modal.getUI()).thenReturn(Optional.of(ui));
         when(modal.getEventBus()).thenReturn(new ComponentEventBus(modal));
         when(modal.getElement()).thenReturn(new Element("tag"));
+        when(modal.isVisible()).thenReturn(true);
 
         UIInternals uiInternals = Mockito.mock(UIInternals.class);
         Mockito.when(uiInternals.hasModalComponent()).thenReturn(true);
@@ -515,8 +516,7 @@ public void uiRegistration_uiHasModalComponent_eventIsSentFromModalComponentInst
                 Key.KEY_A);
 
         mockLifecycle(true);
-        Mockito.when(lifecycleOwner.getParent())
-                .thenReturn(Optional.of(new FakeComponent()));
+        Mockito.when(lifecycleOwner.getParent()).thenReturn(Optional.of(modal));
 
         clientResponse(listenOn);
 

diff --git a/flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/ModalDialogView.java b/flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/ModalDialogView.java
@@ -16,6 +16,9 @@
 
 package com.vaadin.flow.uitest.ui;
 
+import java.util.List;
+import java.util.Map;
+
 import com.vaadin.flow.component.ClickEvent;
 import com.vaadin.flow.component.Component;
 import com.vaadin.flow.component.ComponentEventListener;
@@ -25,10 +28,14 @@
 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.router.BeforeEvent;
+import com.vaadin.flow.router.HasUrlParameter;
+import com.vaadin.flow.router.Location;
+import com.vaadin.flow.router.OptionalParameter;
 import com.vaadin.flow.router.Route;
 
 @Route(value = "com.vaadin.flow.uitest.ui.ModalDialogView")
-public class ModalDialogView extends Div {
+public class ModalDialogView extends Div implements HasUrlParameter<String> {
 
     public static final String EVENT_LOG = "event-log";
     public static final String UI_BUTTON = "ui-button";
@@ -58,6 +65,20 @@ public ModalDialogView() {
         setId("main-div");
     }
 
+    @Override
+    public void setParameter(BeforeEvent event,
+            @OptionalParameter String parameter) {
+        Location location = event.getLocation();
+        Map<String, List<String>> queryParameters = location
+                .getQueryParameters().getParameters();
+        if (queryParameters.containsKey("open_dialog")) {
+            boolean modal = queryParameters.get("open_dialog")
+                    .contains("modal");
+            final Dialog dialog = new Dialog(modal);
+            dialog.open();
+        }
+    }
+
     private void logClickEvent(ClickEvent<?> event) {
         eventLog.addComponentAsFirst(new Div(new Text((eventCounter++) + "-"
                 + event.getSource().getId().orElse("NO-SOURCE-ID"))));
@@ -124,7 +145,7 @@ public Dialog(boolean modal) {
         }
 
         public void open() {
-            final UI ui = ModalDialogView.this.getUI().get();
+            final UI ui = UI.getCurrent();
             if (modal) {
                 ui.addModal(this);
             } else {

diff --git a/flow-tests/test-root-context/src/test/java/com/vaadin/flow/uitest/ui/ModalDialogIT.java b/flow-tests/test-root-context/src/test/java/com/vaadin/flow/uitest/ui/ModalDialogIT.java
@@ -49,6 +49,22 @@ public void modalDialogOpened_sameShortcutsListeningOnUi_noShortcutTriggered() {
         validateLatestShortcutEvent(2, ModalDialogView.UI_BUTTON);
     }
 
+    @Test
+    public void modalDialogOpenInitially_dialogClosed_shortcutsInViewTrigger() {
+        open("open_dialog=modal");
+
+        // shortcuts on view should not trigger while dialog is open
+        pressShortcutKey(
+                $(NativeButtonElement.class).id(ModalDialogView.UI_BUTTON));
+
+        closeDialog();
+
+        // shortcuts on view should not trigger when dialog has been closed
+        pressShortcutKey(
+                $(NativeButtonElement.class).id(ModalDialogView.UI_BUTTON));
+        validateLatestShortcutEvent(1, ModalDialogView.UI_BUTTON);
+    }
+
     @Test
     public void modalDialogOpened_sameShortcutListeningOnUiAndDialog_onlyDialogShortcutExecuted() {
         pressShortcutKey(