From 638cbaa7db4921f9d0439e7552e96fe04f68aa3f Mon Sep 17 00:00:00 2001 From: web-padawan Date: Fri, 6 Dec 2024 12:33:29 +0200 Subject: [PATCH] fix: prevent notification from affecting overlay interactions --- .../overlay/src/vaadin-overlay-stack-mixin.js | 19 +++-- test/integration/not-animated-styles.js | 2 +- test/integration/notification-overlay.test.js | 84 +++++++++++++++++++ 3 files changed, 97 insertions(+), 8 deletions(-) diff --git a/packages/overlay/src/vaadin-overlay-stack-mixin.js b/packages/overlay/src/vaadin-overlay-stack-mixin.js index 73fd24d09f..ac1d56b3b9 100644 --- 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 index a43064789b..82117eeabd 100644 --- 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 index 1a17b622a3..117ee64189 100644 --- 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(''); + 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; + }); + }); });