fix: auto-attach Popover based on helper annotation (#8110)

DiegoCardoso · web-flow · commit 4cd7d5360c59 · 2025-10-06T17:02:56.000+02:00
Introduces a new internal Java annotation interface to help Popover
decide where to attach itself when the component is auto added to the
UI. Adds the `@ModalComponent` annotation to the following components:
- Dialog
- ConfirmDialog
- LoginOverlay

When the Popover target's element is inside any of the components above,
the component will be placed in the same parent as the target's to allow
events to be listened from the Popover content.
diff --git a/vaadin-confirm-dialog-flow-parent/vaadin-confirm-dialog-flow/src/main/java/com/vaadin/flow/component/confirmdialog/ConfirmDialog.java b/vaadin-confirm-dialog-flow-parent/vaadin-confirm-dialog-flow/src/main/java/com/vaadin/flow/component/confirmdialog/ConfirmDialog.java
@@ -32,6 +32,7 @@
 import com.vaadin.flow.component.dependency.JsModule;
 import com.vaadin.flow.component.dependency.NpmPackage;
 import com.vaadin.flow.component.shared.SlotUtils;
+import com.vaadin.flow.component.shared.internal.ModalRoot;
 import com.vaadin.flow.component.shared.internal.OverlayAutoAddController;
 import com.vaadin.flow.dom.Style;
 import com.vaadin.flow.shared.Registration;
@@ -62,6 +63,7 @@
 @Tag("vaadin-confirm-dialog")
 @NpmPackage(value = "@vaadin/confirm-dialog", version = "25.0.0-alpha20")
 @JsModule("@vaadin/confirm-dialog/src/vaadin-confirm-dialog.js")
+@ModalRoot
 public class ConfirmDialog extends Component
         implements HasComponents, HasSize, HasStyle {
 
diff --git a/vaadin-dialog-flow-parent/vaadin-dialog-flow/src/main/java/com/vaadin/flow/component/dialog/Dialog.java b/vaadin-dialog-flow-parent/vaadin-dialog-flow/src/main/java/com/vaadin/flow/component/dialog/Dialog.java
@@ -39,6 +39,7 @@
 import com.vaadin.flow.component.dependency.JsModule;
 import com.vaadin.flow.component.dependency.NpmPackage;
 import com.vaadin.flow.component.shared.HasThemeVariant;
+import com.vaadin.flow.component.shared.internal.ModalRoot;
 import com.vaadin.flow.component.shared.internal.OverlayAutoAddController;
 import com.vaadin.flow.dom.Element;
 import com.vaadin.flow.dom.ElementConstants;
@@ -78,6 +79,7 @@
 @NpmPackage(value = "@vaadin/dialog", version = "25.0.0-alpha20")
 @JsModule("@vaadin/dialog/src/vaadin-dialog.js")
 @JsModule("./flow-component-renderer.js")
+@ModalRoot
 public class Dialog extends Component implements HasComponents, HasSize,
         HasStyle, HasThemeVariant<DialogVariant> {
 
diff --git a/vaadin-flow-components-shared-parent/vaadin-flow-components-base/src/main/java/com/vaadin/flow/component/shared/internal/ModalRoot.java b/vaadin-flow-components-shared-parent/vaadin-flow-components-base/src/main/java/com/vaadin/flow/component/shared/internal/ModalRoot.java
@@ -0,0 +1,50 @@
+/*
+ * Copyright 2000-2025 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.component.shared.internal;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * The purpose of this annotation is to help {@code Popover} decide where to
+ * auto-attach itself based on the target component.
+ * <p>
+ * Normally, {@code Popover} attaches to the UI's root element, using the
+ * {@code UI#addToModalComponent} method. But that is problematic when the
+ * target component is attached inside an element with modality, such as
+ * {@code Dialog}. This prevents any events in the client-side fired from the
+ * {@code Popover} component or any of its children from being listened to in
+ * the server-side.
+ * <p>
+ * To solve this, {@code Popover} tries to find the closest parent component
+ * that has this annotation, and attaches to that component instead of the UI
+ * root.
+ * <p>
+ * <strong> Internal use only. May be renamed or removed in a future release.
+ * </strong>
+ */
+@Retention(RetentionPolicy.RUNTIME)
+@Target(ElementType.TYPE)
+public @interface ModalRoot {
+
+    /**
+     * The slot to use when attaching to the modal root. If empty, no slot
+     * attribute is set.
+     */
+    String slot() default "";
+}
diff --git a/vaadin-login-flow-parent/vaadin-login-flow/src/main/java/com/vaadin/flow/component/login/LoginOverlay.java b/vaadin-login-flow-parent/vaadin-login-flow/src/main/java/com/vaadin/flow/component/login/LoginOverlay.java
@@ -27,6 +27,7 @@
 import com.vaadin.flow.component.dependency.JsModule;
 import com.vaadin.flow.component.dependency.NpmPackage;
 import com.vaadin.flow.component.shared.SlotUtils;
+import com.vaadin.flow.component.shared.internal.ModalRoot;
 import com.vaadin.flow.component.shared.internal.OverlayAutoAddController;
 import com.vaadin.flow.dom.Element;
 import com.vaadin.flow.dom.Style;
@@ -48,6 +49,7 @@
 @Tag("vaadin-login-overlay")
 @NpmPackage(value = "@vaadin/login", version = "25.0.0-alpha20")
 @JsModule("@vaadin/login/src/vaadin-login-overlay.js")
+@ModalRoot(slot = "footer")
 public class LoginOverlay extends AbstractLogin implements HasStyle {
 
     private Component title;
diff --git a/vaadin-popover-flow-parent/vaadin-popover-flow/src/main/java/com/vaadin/flow/component/popover/Popover.java b/vaadin-popover-flow-parent/vaadin-popover-flow/src/main/java/com/vaadin/flow/component/popover/Popover.java
@@ -33,6 +33,7 @@
 import com.vaadin.flow.component.dependency.JsModule;
 import com.vaadin.flow.component.dependency.NpmPackage;
 import com.vaadin.flow.component.shared.HasThemeVariant;
+import com.vaadin.flow.component.shared.internal.ModalRoot;
 import com.vaadin.flow.dom.Style;
 import com.vaadin.flow.internal.JacksonUtils;
 import com.vaadin.flow.server.VaadinService;
@@ -729,17 +730,76 @@ private void removeFromUiIfAutoAdded() {
     }
 
     private void onTargetAttach(UI ui) {
+
         if (target != null) {
             if (getElement().getNode().getParent() == null) {
                 // Remove the popover from its current state tree
                 getElement().removeFromTree(false);
-                ui.addToModalComponent(this);
+                getModalParentComponent().ifPresentOrElse(modalParent -> {
+                    updateSlotAttribute(modalParent);
+
+                    if (modalParent instanceof HasComponents) {
+                        ((HasComponents) modalParent).add(this);
+                    } else {
+                        modalParent.getElement().appendChild(getElement());
+                    }
+                }, () -> {
+                    ui.addToModalComponent(this);
+                });
+
                 autoAddedToTheUi = true;
             }
             getElement().executeJs("this.target = $0", target.getElement());
         }
     }
 
+    /**
+     * Finds the closest parent component that is annotated with
+     * {@link ModalRoot}, if any.
+     *
+     * @return an optional with the closest modal parent component, or an empty
+     *         optional if none found
+     */
+    private Optional<Component> getModalParentComponent() {
+        var parent = target.getParent();
+        while (parent.isPresent()) {
+            var parentComponent = parent.get();
+            if (parentComponent.getClass()
+                    .isAnnotationPresent(ModalRoot.class)) {
+                return parent;
+            }
+            parent = parentComponent.getParent();
+        }
+        return Optional.empty();
+    }
+
+    /**
+     * Updates the {@code slot} attribute based on the target or its ancestors
+     * up to the modal parent.
+     * <p>
+     * It starts by removing any existing {@code slot} attribute from the
+     * popover. Then, it gets the {@link ModalRoot} annotation from the modal
+     * parent component to determine the appropriate slot value. If the modal
+     * parent defines a value for the {@link ModalRoot#slot()} property, it sets
+     * the {@code slot} attribute on the popover element.
+     * 
+     * <p>
+     * This ensures that the popover is rendered in the correct slot when used
+     * inside a modal component.
+     *
+     * @param modalParent
+     *            the modal parent component
+     */
+    private void updateSlotAttribute(Component modalParent) {
+        getElement().removeAttribute("slot");
+        var annotation = modalParent.getClass().getAnnotation(ModalRoot.class);
+
+        var slotValue = annotation.slot();
+        if (slotValue != null && !slotValue.isEmpty()) {
+            getElement().setAttribute("slot", slotValue);
+        }
+    }
+
     /**
      * Gets the target component of this popover, or {@code null} if it doesn't
      * have a target.
diff --git a/vaadin-popover-flow-parent/vaadin-popover-flow/src/test/java/com/vaadin/flow/component/popover/PopoverAutoAddTest.java b/vaadin-popover-flow-parent/vaadin-popover-flow/src/test/java/com/vaadin/flow/component/popover/PopoverAutoAddTest.java
@@ -21,8 +21,12 @@
 import org.junit.Test;
 import org.mockito.Mockito;
 
+import com.vaadin.flow.component.Component;
+import com.vaadin.flow.component.HasComponents;
+import com.vaadin.flow.component.Tag;
 import com.vaadin.flow.component.UI;
 import com.vaadin.flow.component.html.Div;
+import com.vaadin.flow.component.shared.internal.ModalRoot;
 import com.vaadin.flow.server.VaadinSession;
 
 /**
@@ -184,9 +188,127 @@ public void setTarget_openModal_popoverIsAttachedToUi() {
         Assert.assertEquals(ui, popover.getParent().orElseThrow());
     }
 
+    @Test
+    public void popoverWithTargetInModalComponent_popoverAttachedToModal() {
+        var modal = new TestModalComponent();
+        ui.add(modal);
+        var target = new Div();
+        modal.getElement().appendChild(target.getElement());
+        var popover = new Popover();
+        popover.setTarget(target);
+        fakeClientResponse();
+        Assert.assertEquals("Popover should be attached to modal", modal,
+                popover.getParent().orElse(null));
+    }
+
+    @Test
+    public void popoverWithTargetInModalComponent_targetRemoved_popoverDetached() {
+        var modal = new TestModalComponent();
+        ui.add(modal);
+        var target = new Div();
+        modal.getElement().appendChild(target.getElement());
+        var popover = new Popover();
+        popover.setTarget(target);
+        fakeClientResponse();
+
+        target.removeFromParent();
+        fakeClientResponse();
+        Assert.assertFalse("Popover should be detached",
+                popover.getParent().isPresent());
+    }
+
+    @Test
+    public void popoverWithTargetInModalContainer_popoverAttachedToModal() {
+        var modal = new TestModalContainer();
+        ui.add(modal);
+        var target = new Div();
+        modal.add(target);
+        var popover = new Popover();
+        popover.setTarget(target);
+        fakeClientResponse();
+        Assert.assertEquals("Popover should be attached to modal", modal,
+                popover.getParent().orElse(null));
+    }
+
+    @Test
+    public void popoverWithTargetInModalContainer_targetRemoved_popoverDetached() {
+        var modal = new TestModalContainer();
+        ui.add(modal);
+        var target = new Div();
+        modal.add(target);
+        var popover = new Popover();
+        popover.setTarget(target);
+        fakeClientResponse();
+
+        target.removeFromParent();
+        fakeClientResponse();
+        Assert.assertFalse("Popover should be detached",
+                popover.getParent().isPresent());
+    }
+
+    @Test
+    public void targetWithinModalWithSlotDefined_popoverInheritsSlotAttribute() {
+        var modal = new TestModalContainerWithSlot();
+        ui.add(modal);
+        var target = new Div();
+        var popover = new Popover();
+        popover.setTarget(target);
+        modal.add(target);
+        fakeClientResponse();
+
+        Assert.assertEquals("custom-slot",
+                popover.getElement().getAttribute("slot"));
+
+        fakeClientResponse();
+        var newModal = new TestModalContainer();
+        ui.add(newModal);
+        newModal.add(target);
+        fakeClientResponse();
+        Assert.assertFalse("Popover should not have value for slot attribute",
+                popover.getElement().hasAttribute("slot"));
+    }
+
+    @Test
+    public void popoverWithTargetAddedAsVirtualChild_popoverAttachedToModal() {
+        var modal = new TestModalComponent();
+        ui.add(modal);
+        var target = new Div();
+        modal.getElement().appendVirtualChild(target.getElement());
+        var popover = new Popover();
+        popover.setTarget(target);
+        fakeClientResponse();
+        Assert.assertEquals("Popover should be attached to modal", modal,
+                popover.getParent().orElse(null));
+    }
+
     private void fakeClientResponse() {
         ui.getInternals().getStateTree().runExecutionsBeforeClientResponse();
         ui.getInternals().getStateTree().collectChanges(ignore -> {
         });
     }
+
+    @ModalRoot
+    @Tag("div")
+    public class TestModalContainer extends Component implements HasComponents {
+        public TestModalContainer() {
+            super();
+        }
+    }
+
+    @ModalRoot
+    @Tag("div")
+    public class TestModalComponent extends Component {
+        public TestModalComponent() {
+            super();
+        }
+    }
+
+    @ModalRoot(slot = "custom-slot")
+    @Tag("div")
+    public class TestModalContainerWithSlot extends Component
+            implements HasComponents {
+        public TestModalContainerWithSlot() {
+            super();
+        }
+    }
 }
