fix: close a notification on detach (vaadin#3154)

Author: tomivirkki

vaadin-notification-flow-parent/vaadin-notification-flow-integration-tests/src/main/java/com/vaadin/flow/component/notification/tests/NotificationView.java

@@ -76,7 +76,7 @@ private void createNotificationUsingStaticConvenienceMethod() {
         Notification notification = Notification.show(
                 "This is a notification created with static convenience method");
         notification.setId("static-notification");
-        addCard("Notification using static convenience method", notification);
+        addCard("Notification using static convenience method");
     }
 
     private void createNotificationWithComponents() {

vaadin-notification-flow-parent/vaadin-notification-flow/src/main/java/com/vaadin/flow/component/notification/Notification.java

@@ -202,13 +202,25 @@ private void initBaseElementsAndListeners() {
         getElement().appendChild(templateElement);
         getElement().appendVirtualChild(container);
 
-        getElement().addEventListener("opened-changed", event -> {
-            if (autoAddedToTheUi && !isOpened()) {
-                getElement().removeFromParent();
-                autoAddedToTheUi = false;
-            }
+        getElement().addEventListener("opened-changed",
+                event -> removeAutoAdded());
+
+        addDetachListener(event -> {
+            // If the notification gets detached, it needs to be marked
+            // as closed so it won't auto-open when reattached.
+            setOpened(false);
+            removeAutoAdded();
         });
+    }
 
+    /**
+     * Removes the notification from its parent if it was added automatically.
+     */
+    private void removeAutoAdded() {
+        if (autoAddedToTheUi && !isOpened()) {
+            autoAddedToTheUi = false;
+            getElement().removeFromParent();
+        }
     }
 
     /**
@@ -433,12 +445,14 @@ public void setOpened(boolean opened) {
                     + "That may happen if you call the method from the custom thread without "
                     + "'UI::access' or from tests without proper initialization.");
         }
-        if (opened && getElement().getNode().getParent() == null) {
-            ui.beforeClientResponse(ui, context -> {
+
+        ui.beforeClientResponse(ui, context -> {
+            if (isOpened() && getElement().getNode().getParent() == null) {
                 ui.addToModalComponent(this);
                 autoAddedToTheUi = true;
-            });
-        }
+            }
+        });
+
         super.setOpened(opened);
     }
 

vaadin-notification-flow-parent/vaadin-notification-flow/src/test/java/com/vaadin/flow/component/notification/NotificationTest.java

@@ -185,10 +185,7 @@ public void setText_notificationHasAddedComponents_innerHtmlIsTextValue() {
 
         notification.open();
 
-        UIInternals internals = ui.getInternals();
-        VaadinSession session = Mockito.mock(VaadinSession.class);
-        internals.setSession(session);
-        internals.getStateTree().runExecutionsBeforeClientResponse();
+        flushBeforeClientResponse();
 
         Element templateElement = notification.getElement().getChildren()
                 .findFirst().get();
@@ -206,10 +203,7 @@ public void add_notificationHasText_innerHtmlIsTemplateValue() {
 
         notification.open();
 
-        UIInternals internals = ui.getInternals();
-        VaadinSession session = Mockito.mock(VaadinSession.class);
-        internals.setSession(session);
-        internals.getStateTree().runExecutionsBeforeClientResponse();
+        flushBeforeClientResponse();
 
         Element templateElement = notification.getElement().getChildren()
                 .findFirst().get();
@@ -241,4 +235,87 @@ public void hasStyle() {
         Notification notification = new Notification();
         Assert.assertTrue(notification instanceof HasStyle);
     }
+
+    @Test
+    public void createNotificationh_closeOnParentDetach() {
+        // Create a Notification manually and add it to a parent container
+        Notification notification = new Notification();
+        notification.open();
+        Div parent = new Div(notification);
+        // Add the parent to the UI
+        ui.add(parent);
+
+        // The notification was opened manually. Check that it's still open.
+        Assert.assertTrue(notification.isOpened());
+
+        // Remove the parent container from the UI
+        ui.remove(parent);
+
+        // The notification should have been closed on detach, even if it was
+        // the parent that was removed
+        Assert.assertFalse(notification.isOpened());
+        // The parent reference should not have changed
+        Assert.assertEquals(notification.getParent().get(), parent);
+    }
+
+    @Test
+    public void showNotification_closeAndDetachOnParentDetach() {
+        // Create a modal parent container and add it to the UI
+        Div parent = new Div();
+        ui.add(parent);
+        ui.setChildComponentModal(parent, true);
+
+        // Use Notification.show() helper to create a notification.
+        // It will be automatically added to the modal parent container (before
+        // client response)
+        Notification notification = Notification.show("foo");
+        flushBeforeClientResponse();
+
+        // Check that the notification is opened and attached to the parent
+        // container
+        Assert.assertTrue(notification.isOpened());
+        Assert.assertTrue(parent.getChildren().collect(Collectors.toList())
+                .contains(notification));
+
+        // Remove the modal parent container from the UI
+        ui.remove(parent);
+
+        // The notification should have been closed on detach, even if it was
+        // the parent that was removed
+        Assert.assertFalse(notification.isOpened());
+        // The notification should have been automatically removed from the
+        // parent container
+        Assert.assertFalse(parent.getChildren().collect(Collectors.toList())
+                .contains(notification));
+    }
+
+    @Test
+    public void showNotification_addManually_dontDetachOnParentDetach() {
+        // Use Notification.show() helper to create a notification.
+        Notification notification = Notification.show("foo");
+
+        // Manually add the notification to a parent
+        Div parent = new Div(notification);
+        ui.add(parent);
+        // Flush
+        flushBeforeClientResponse();
+
+        // Check that the notification is attached to the parent container
+        Assert.assertEquals(notification.getParent().get(), parent);
+
+        // Remove the modal parent container from the UI
+        ui.remove(parent);
+
+        // Even though the notification was created using Notification.show(),
+        // it got was manually added to the parent container so it should not
+        // have been automatically removed from it.
+        Assert.assertEquals(notification.getParent().get(), parent);
+    }
+
+    private void flushBeforeClientResponse() {
+        UIInternals internals = ui.getInternals();
+        VaadinSession session = Mockito.mock(VaadinSession.class);
+        internals.setSession(session);
+        internals.getStateTree().runExecutionsBeforeClientResponse();
+    }
 }