Skip to content

Commit 63018cc

Browse files
authored
fix: prevent notification from affecting overlay interactions (#8291)
1 parent 9d24214 commit 63018cc

File tree

3 files changed

+97
-8
lines changed

3 files changed

+97
-8
lines changed

packages/overlay/src/vaadin-overlay-stack-mixin.js

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,20 @@ const getAttachedInstances = () =>
1313
.filter((el) => el instanceof HTMLElement && el._hasOverlayStackMixin && !el.hasAttribute('closing'))
1414
.sort((a, b) => a.__zIndex - b.__zIndex || 0);
1515

16+
/**
17+
* Returns all attached overlay instances excluding notification container,
18+
* which only needs to be in the stack for zIndex but not pointer-events.
19+
* @private
20+
*/
21+
const getOverlayInstances = () => getAttachedInstances().filter((el) => el.$.overlay);
22+
1623
/**
1724
* Returns true if the overlay is the last one in the opened overlays stack.
1825
* @param {HTMLElement} overlay
1926
* @return {boolean}
2027
* @protected
2128
*/
22-
export const isLastOverlay = (overlay) => overlay === getAttachedInstances().pop();
29+
export const isLastOverlay = (overlay) => overlay === getOverlayInstances().pop();
2330

2431
/**
2532
* @polymerMixin
@@ -68,8 +75,8 @@ export const OverlayStackMixin = (superClass) =>
6875
}
6976

7077
// Disable pointer events in other attached overlays
71-
getAttachedInstances().forEach((el) => {
72-
if (el !== this && el.$.overlay) {
78+
getOverlayInstances().forEach((el) => {
79+
if (el !== this) {
7380
el.$.overlay.style.pointerEvents = 'none';
7481
}
7582
});
@@ -84,7 +91,7 @@ export const OverlayStackMixin = (superClass) =>
8491
}
8592

8693
// Restore pointer events in the previous overlay(s)
87-
const instances = getAttachedInstances();
94+
const instances = getOverlayInstances();
8895

8996
let el;
9097
// Use instances.pop() to ensure the reverse order
@@ -93,9 +100,7 @@ export const OverlayStackMixin = (superClass) =>
93100
// Skip the current instance
94101
continue;
95102
}
96-
if (el.$.overlay) {
97-
el.$.overlay.style.removeProperty('pointer-events');
98-
}
103+
el.$.overlay.style.removeProperty('pointer-events');
99104
if (!el.modeless) {
100105
// Stop after the last modal
101106
break;

test/integration/not-animated-styles.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { css, registerStyles } from '@vaadin/vaadin-themable-mixin/vaadin-themab
22

33
// Disable animations for all overlays
44
registerStyles(
5-
'vaadin-*-overlay',
5+
'vaadin-*-overlay vaadin-notification-card',
66
css`
77
:host([opening]),
88
:host([closing]),

test/integration/notification-overlay.test.js

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
import { expect } from '@vaadin/chai-plugins';
22
import { fixtureSync, nextRender } from '@vaadin/testing-helpers';
3+
import { sendKeys } from '@web/test-runner-commands';
4+
import './not-animated-styles.js';
5+
import '@vaadin/dialog';
36
import '@vaadin/notification';
47
import '@vaadin/popover';
58
import '@vaadin/tooltip';
@@ -61,4 +64,85 @@ describe('notification and overlays', () => {
6164
expect(popoverZIndex).to.be.above(notificationZIndex);
6265
});
6366
});
67+
68+
describe('notification and dialog', () => {
69+
let dialog1, dialog2, notification;
70+
71+
beforeEach(async () => {
72+
dialog1 = fixtureSync('<vaadin-dialog></vaadin-dialog>');
73+
await nextRender();
74+
75+
dialog1.renderer = (root) => {
76+
if (!root.firstChild) {
77+
notification = document.createElement('vaadin-notification');
78+
notification.renderer = (root) => {
79+
root.textContent = 'Hello!';
80+
};
81+
82+
dialog2 = document.createElement('vaadin-dialog');
83+
dialog2.renderer = (root2) => {
84+
if (!root2.firstChild) {
85+
const close = document.createElement('button');
86+
close.textContent = 'Close and show notification';
87+
close.addEventListener('click', () => {
88+
console.log('close and show');
89+
notification.opened = true;
90+
dialog2.opened = false;
91+
});
92+
root2.appendChild(close);
93+
}
94+
};
95+
96+
const open = document.createElement('button');
97+
open.setAttribute('id', 'open');
98+
open.textContent = 'Open dialog 2';
99+
open.addEventListener('click', () => {
100+
dialog2.opened = true;
101+
});
102+
103+
const show = document.createElement('button');
104+
show.setAttribute('id', 'show');
105+
show.textContent = 'Show notification';
106+
show.addEventListener('click', () => {
107+
notification.opened = true;
108+
});
109+
110+
root.append(notification, dialog2, open, show);
111+
}
112+
};
113+
});
114+
115+
afterEach(() => {
116+
notification.opened = false;
117+
});
118+
119+
it('should remove pointer-events when closing dialog and opening notification', async () => {
120+
dialog1.opened = true;
121+
await nextRender();
122+
123+
// Open dialog 2
124+
dialog1.$.overlay.querySelector('#open').click();
125+
await nextRender();
126+
expect(getComputedStyle(dialog1.$.overlay.$.overlay).pointerEvents).to.equal('none');
127+
128+
// Close dialog 2 and show notification
129+
dialog2.$.overlay.querySelector('button').click();
130+
await nextRender();
131+
132+
expect(getComputedStyle(dialog1.$.overlay.$.overlay).pointerEvents).to.equal('auto');
133+
});
134+
135+
it('should allow closing the dialog on Escape press after opening notification', async () => {
136+
dialog1.opened = true;
137+
await nextRender();
138+
139+
// Show notification
140+
dialog1.$.overlay.querySelector('#show').click();
141+
await nextRender();
142+
143+
await sendKeys({ press: 'Escape' });
144+
145+
expect(dialog1.opened).to.be.false;
146+
});
147+
});
64148
});

0 commit comments

Comments
 (0)