Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: prevent notification from affecting overlay interactions #8291

Merged
merged 1 commit into from
Dec 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 12 additions & 7 deletions packages/overlay/src/vaadin-overlay-stack-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
vursen marked this conversation as resolved.
Show resolved Hide resolved

/**
* 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
Expand Down Expand Up @@ -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';
}
});
Expand All @@ -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
Expand All @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion test/integration/not-animated-styles.js
Original file line number Diff line number Diff line change
Expand Up @@ -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]),
Expand Down
84 changes: 84 additions & 0 deletions test/integration/notification-overlay.test.js
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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;
});
});
});
Loading