feat: add logic for Popover auto-adding to the UI on open

vaadin · Aug 21, 2024 · 8b9b7bd · 8b9b7bd
1 parent d8a9405
commit 8b9b7bd
Show file tree

Hide file tree

Showing 2 changed files with 147 additions and 2 deletions.
diff --git a/...w-parent/vaadin-popover-flow/src/main/java/com/vaadin/flow/component/popover/Popover.java b/...w-parent/vaadin-popover-flow/src/main/java/com/vaadin/flow/component/popover/Popover.java
@@ -43,6 +43,8 @@
 import com.vaadin.flow.dom.ElementDetachEvent;
 import com.vaadin.flow.dom.ElementDetachListener;
 import com.vaadin.flow.dom.Style;
+import com.vaadin.flow.internal.StateTree;
+import com.vaadin.flow.router.NavigationTrigger;
 import com.vaadin.flow.shared.Registration;
 
 import elemental.json.Json;
@@ -64,6 +66,8 @@ public class Popover extends Component implements HasAriaLabel, HasComponents,
 
     private Component target;
     private Registration targetAttachRegistration;
+    private Registration afterProgrammaticNavigationListenerRegistration;
+    private boolean autoAddedToTheUi;
 
     private boolean openOnClick = true;
     private boolean openOnHover = false;
@@ -78,6 +82,14 @@ public Popover() {
         // Workaround for: https://github.com/vaadin/flow/issues/3496
         getElement().setProperty("opened", false);
 
+        getElement().addPropertyChangeListener("opened", event -> {
+            // Only handle client-side changes, server-side changes are already
+            // handled by setOpened
+            if (event.isUserOriginated()) {
+                doSetOpened(this.isOpened(), event.isUserOriginated());
+            }
+        });
+
         updateTrigger();
         setOverlayRole("dialog");
     }
@@ -120,20 +132,41 @@ public boolean isOpened() {
      */
     public void setOpened(boolean opened) {
         if (opened != isOpened()) {
-            getElement().setProperty("opened", opened);
-            fireEvent(new OpenedChangeEvent(this, false));
+            doSetOpened(opened, false);
         }
     }
 
+    private void doSetOpened(boolean opened, boolean fromClient) {
+        if (opened) {
+            ensureAttached();
+        } else if (autoAddedToTheUi) {
+            getElement().removeFromParent();
+            autoAddedToTheUi = false;
+        }
+        getElement().setProperty("opened", opened);
+        fireEvent(new OpenedChangeEvent(this, fromClient));
+    }
+
     /**
      * Opens the popover.
+     * <p>
+     * Note: You don't need to add the popover component before opening it,
+     * cause opening a dialog will automatically add it to the {@code <body>} if
+     * it's not yet attached anywhere.
+     * <p>
+     * When using {@link #setFor(String)} it is recommended to manually add the
+     * popover to the UI instead, and to ensure it is placed in the same DOM
+     * scope (document or shadow root) as the component with the given ID.
      */
     public void open() {
         setOpened(true);
     }
 
     /**
      * Closes the popover.
+     * <p>
+     * Note: This method also removes the popover component from the DOM after
+     * closing it, unless you have added the component manually.
      */
     public void close() {
         setOpened(false);
@@ -736,6 +769,45 @@ protected void onAttach(AttachEvent attachEvent) {
         updateVirtualChildNodeIds();
     }
 
+    private UI getCurrentUI() {
+        UI ui = UI.getCurrent();
+        if (ui == null) {
+            throw new IllegalStateException("UI instance is not available. "
+                    + "It means that you are calling this method "
+                    + "out of a normal workflow where it's always implicitly set. "
+                    + "That may happen if you call the method from the custom thread without "
+                    + "'UI::access' or from tests without proper initialization.");
+        }
+        return ui;
+    }
+
+    private void ensureAttached() {
+        UI ui = getCurrentUI();
+        StateTree.ExecutionRegistration addToUiRegistration = ui
+                .beforeClientResponse(ui, context -> {
+                    if (getElement().getNode().getParent() == null
+                            && isOpened()) {
+                        ui.addToModalComponent(this);
+                        autoAddedToTheUi = true;
+                    }
+                    if (afterProgrammaticNavigationListenerRegistration != null) {
+                        afterProgrammaticNavigationListenerRegistration
+                                .remove();
+                    }
+                });
+        if (ui.getSession() != null) {
+            afterProgrammaticNavigationListenerRegistration = ui
+                    .addAfterNavigationListener(event -> {
+                        if (event.getLocationChangeEvent()
+                                .getTrigger() == NavigationTrigger.PROGRAMMATIC) {
+                            addToUiRegistration.remove();
+                            afterProgrammaticNavigationListenerRegistration
+                                    .remove();
+                        }
+                    });
+        }
+    }
+
     /**
      * Updates the virtualChildNodeIds property of the popover element.
      * <p>

diff --git a/...adin-popover-flow/src/test/java/com/vaadin/flow/component/popover/PopoverAutoAddTest.java b/...adin-popover-flow/src/test/java/com/vaadin/flow/component/popover/PopoverAutoAddTest.java
@@ -0,0 +1,73 @@
+/*
+ * Copyright 2000-2024 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.popover;
+
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+import com.vaadin.flow.component.UI;
+import com.vaadin.flow.server.VaadinSession;
+
+/**
+ * @author Vaadin Ltd.
+ */
+public class PopoverAutoAddTest {
+    private UI ui = new UI();
+
+    @Before
+    public void setup() {
+        UI.setCurrent(ui);
+
+        VaadinSession session = Mockito.mock(VaadinSession.class);
+        Mockito.when(session.hasLock()).thenReturn(true);
+        ui.getInternals().setSession(session);
+    }
+
+    @After
+    public void tearDown() {
+        UI.setCurrent(null);
+    }
+
+    @Test
+    public void open_autoAttachedInBeforeClientResponse() {
+        Popover popover = new Popover();
+        popover.open();
+
+        fakeClientResponse();
+        Assert.assertNotNull(popover.getElement().getParent());
+    }
+
+    @Test
+    public void open_close_notAutoAttachedInBeforeClientResponse() {
+        Popover popover = new Popover();
+        popover.open();
+        fakeClientResponse();
+
+        popover.close();
+
+        fakeClientResponse();
+        Assert.assertNull(popover.getElement().getParent());
+    }
+
+    private void fakeClientResponse() {
+        ui.getInternals().getStateTree().runExecutionsBeforeClientResponse();
+        ui.getInternals().getStateTree().collectChanges(ignore -> {
+        });
+    }
+}