fix: prevent notification from affecting overlay interactions (#8291)

vaadin · Dec 6, 2024 · 63018cc · 63018cc
1 parent 9d24214
commit 63018cc
Show file tree

Hide file tree

Showing 3 changed files with 97 additions and 8 deletions.
diff --git a/packages/overlay/src/vaadin-overlay-stack-mixin.js b/packages/overlay/src/vaadin-overlay-stack-mixin.js
@@ -13,13 +13,20 @@ const getAttachedInstances = () =>
     .filter((el) => el instanceof HTMLElement && el._hasOverlayStackMixin && !el.hasAttribute('closing'))
     .sort((a, b) => a.__zIndex - b.__zIndex || 0);
 
+/**
+ * Returns all attached overlay instances excluding notification container,
+ * which only needs to be in the stack for zIndex but not pointer-events.
+ * @private
+ */
+const getOverlayInstances = () => getAttachedInstances().filter((el) => el.$.overlay);
+
 /**
  * Returns true if the overlay is the last one in the opened overlays stack.
  * @param {HTMLElement} overlay
  * @return {boolean}
  * @protected
  */
-export const isLastOverlay = (overlay) => overlay === getAttachedInstances().pop();
+export const isLastOverlay = (overlay) => overlay === getOverlayInstances().pop();
 
 /**
  * @polymerMixin
@@ -68,8 +75,8 @@ export const OverlayStackMixin = (superClass) =>
       }
 
       // Disable pointer events in other attached overlays
-      getAttachedInstances().forEach((el) => {
-        if (el !== this && el.$.overlay) {
+      getOverlayInstances().forEach((el) => {
+        if (el !== this) {
           el.$.overlay.style.pointerEvents = 'none';
         }
       });
@@ -84,7 +91,7 @@ export const OverlayStackMixin = (superClass) =>
       }
 
       // Restore pointer events in the previous overlay(s)
-      const instances = getAttachedInstances();
+      const instances = getOverlayInstances();
 
       let el;
       // Use instances.pop() to ensure the reverse order
@@ -93,9 +100,7 @@ export const OverlayStackMixin = (superClass) =>
           // Skip the current instance
           continue;
         }
-        if (el.$.overlay) {
-          el.$.overlay.style.removeProperty('pointer-events');
-        }
+        el.$.overlay.style.removeProperty('pointer-events');
         if (!el.modeless) {
           // Stop after the last modal
           break;

diff --git a/test/integration/not-animated-styles.js b/test/integration/not-animated-styles.js
@@ -2,7 +2,7 @@ import { css, registerStyles } from '@vaadin/vaadin-themable-mixin/vaadin-themab
 
 // Disable animations for all overlays
 registerStyles(
-  'vaadin-*-overlay',
+  'vaadin-*-overlay vaadin-notification-card',
   css`
     :host([opening]),
     :host([closing]),

diff --git a/test/integration/notification-overlay.test.js b/test/integration/notification-overlay.test.js
@@ -1,5 +1,8 @@
 import { expect } from '@vaadin/chai-plugins';
 import { fixtureSync, nextRender } from '@vaadin/testing-helpers';
+import { sendKeys } from '@web/test-runner-commands';
+import './not-animated-styles.js';
+import '@vaadin/dialog';
 import '@vaadin/notification';
 import '@vaadin/popover';
 import '@vaadin/tooltip';
@@ -61,4 +64,85 @@ describe('notification and overlays', () => {
       expect(popoverZIndex).to.be.above(notificationZIndex);
     });
   });
+
+  describe('notification and dialog', () => {
+    let dialog1, dialog2, notification;
+
+    beforeEach(async () => {
+      dialog1 = fixtureSync('<vaadin-dialog></vaadin-dialog>');
+      await nextRender();
+
+      dialog1.renderer = (root) => {
+        if (!root.firstChild) {
+          notification = document.createElement('vaadin-notification');
+          notification.renderer = (root) => {
+            root.textContent = 'Hello!';
+          };
+
+          dialog2 = document.createElement('vaadin-dialog');
+          dialog2.renderer = (root2) => {
+            if (!root2.firstChild) {
+              const close = document.createElement('button');
+              close.textContent = 'Close and show notification';
+              close.addEventListener('click', () => {
+                console.log('close and show');
+                notification.opened = true;
+                dialog2.opened = false;
+              });
+              root2.appendChild(close);
+            }
+          };
+
+          const open = document.createElement('button');
+          open.setAttribute('id', 'open');
+          open.textContent = 'Open dialog 2';
+          open.addEventListener('click', () => {
+            dialog2.opened = true;
+          });
+
+          const show = document.createElement('button');
+          show.setAttribute('id', 'show');
+          show.textContent = 'Show notification';
+          show.addEventListener('click', () => {
+            notification.opened = true;
+          });
+
+          root.append(notification, dialog2, open, show);
+        }
+      };
+    });
+
+    afterEach(() => {
+      notification.opened = false;
+    });
+
+    it('should remove pointer-events when closing dialog and opening notification', async () => {
+      dialog1.opened = true;
+      await nextRender();
+
+      // Open dialog 2
+      dialog1.$.overlay.querySelector('#open').click();
+      await nextRender();
+      expect(getComputedStyle(dialog1.$.overlay.$.overlay).pointerEvents).to.equal('none');
+
+      // Close dialog 2 and show notification
+      dialog2.$.overlay.querySelector('button').click();
+      await nextRender();
+
+      expect(getComputedStyle(dialog1.$.overlay.$.overlay).pointerEvents).to.equal('auto');
+    });
+
+    it('should allow closing the dialog on Escape press after opening notification', async () => {
+      dialog1.opened = true;
+      await nextRender();
+
+      // Show notification
+      dialog1.$.overlay.querySelector('#show').click();
+      await nextRender();
+
+      await sendKeys({ press: 'Escape' });
+
+      expect(dialog1.opened).to.be.false;
+    });
+  });
 });