From 08bf848b3284c1e4f4e74ae34b02874aaeb195d9 Mon Sep 17 00:00:00 2001 From: Serhii Kulykov Date: Thu, 21 Mar 2024 13:13:08 +0200 Subject: [PATCH] fix: set display none on overlay part to fix Safari 17.4 issue (#7246) --- integration/tests/overlay-focus-trap.test.js | 18 +++++++++++------- packages/dialog/test/dialog.common.js | 13 ++++++++----- packages/overlay/src/vaadin-overlay-styles.js | 3 ++- packages/overlay/test/focus-trap.common.js | 18 ++++++++---------- 4 files changed, 29 insertions(+), 23 deletions(-) diff --git a/integration/tests/overlay-focus-trap.test.js b/integration/tests/overlay-focus-trap.test.js index bf0ca27ac0..220c48a6a9 100644 --- a/integration/tests/overlay-focus-trap.test.js +++ b/integration/tests/overlay-focus-trap.test.js @@ -32,6 +32,8 @@ describe('focus-trap', () => { } }; overlay.requestContentUpdate(); + overlay.opened = true; + await oneEvent(overlay, 'vaadin-overlay-open'); focusableElements = getFocusableElements(overlay.$.overlay); }); @@ -49,10 +51,7 @@ describe('focus-trap', () => { expect(focusableElements[5]).to.equal(overlay.querySelector('vaadin-button')); }); - it('should focus focusable elements inside the content when focusTrap = true', async () => { - overlay.opened = true; - await oneEvent(overlay, 'vaadin-overlay-open'); - + it('should focus focusable elements inside the content when focusTrap = true', () => { // Tab for (let i = 0; i < focusableElements.length; i++) { const focusedIndex = getFocusedElementIndex(); @@ -88,10 +87,12 @@ describe('focus-trap', () => { } }; overlay.requestContentUpdate(); - focusableElements = getFocusableElements(overlay.$.overlay); }); - it('should properly detect multiple focusable elements inside shadow DOM', () => { + it('should properly detect multiple focusable elements inside shadow DOM', async () => { + overlay.opened = true; + await oneEvent(overlay, 'vaadin-overlay-open'); + focusableElements = getFocusableElements(overlay.$.overlay); expect(focusableElements.length).to.equal(4); const div = overlay.querySelector('div'); expect(focusableElements[1]).to.equal(div.shadowRoot.querySelector('input')); @@ -101,6 +102,7 @@ describe('focus-trap', () => { it('should not focus the overlay part if the content is already focused', async () => { overlay.opened = true; // Needs to happen after opened and before focus-trap loop is executed + focusableElements = getFocusableElements(overlay.$.overlay); focusableElements[1].focus(); await oneEvent(overlay, 'vaadin-overlay-open'); expect(getFocusedElementIndex()).not.to.equal(0); @@ -108,8 +110,9 @@ describe('focus-trap', () => { it('should focus first element with tabIndex=1', async () => { // It's an arguable behavior, probably overlay should be focused instead - focusableElements[1].tabIndex = 1; overlay.opened = true; + focusableElements = getFocusableElements(overlay.$.overlay); + focusableElements[1].tabIndex = 1; await oneEvent(overlay, 'vaadin-overlay-open'); const idx = getFocusedElementIndex(); expect(focusableElements[idx].tabIndex).to.equal(1); @@ -118,6 +121,7 @@ describe('focus-trap', () => { it('should focus focusable elements in shadow DOM on Tab and Shift Tab', async () => { overlay.opened = true; await oneEvent(overlay, 'vaadin-overlay-open'); + focusableElements = getFocusableElements(overlay.$.overlay); // Tab for (let i = 0; i < focusableElements.length; i++) { diff --git a/packages/dialog/test/dialog.common.js b/packages/dialog/test/dialog.common.js index 4f028b384d..3739484e04 100644 --- a/packages/dialog/test/dialog.common.js +++ b/packages/dialog/test/dialog.common.js @@ -145,10 +145,11 @@ describe('vaadin-dialog', () => { it('should restore opened state when added to the DOM', async () => { const parent = dialog.parentNode; dialog.remove(); - await aTimeout(0); + await nextRender(); expect(dialog.opened).to.be.false; parent.appendChild(dialog); + await nextRender(); expect(dialog.opened).to.be.true; }); @@ -202,11 +203,13 @@ describe('vaadin-dialog', () => { await nextRender(); }); - it('should not throw an exception if renderer is not present', () => { - const openDialog = () => { + it('should not throw an exception if renderer is not present', async () => { + try { dialog.opened = true; - }; - expect(openDialog).to.not.throw(); + await nextRender(); + } catch (e) { + expect.fail(`Error when opening dialog: ${e}`); + } }); it('should have min-width for content', async () => { diff --git a/packages/overlay/src/vaadin-overlay-styles.js b/packages/overlay/src/vaadin-overlay-styles.js index 19591981ee..193eeed36b 100644 --- a/packages/overlay/src/vaadin-overlay-styles.js +++ b/packages/overlay/src/vaadin-overlay-styles.js @@ -39,7 +39,8 @@ export const overlayStyles = css` } :host([hidden]), - :host(:not([opened]):not([closing])) { + :host(:not([opened]):not([closing])), + :host(:not([opened]):not([closing])) [part='overlay'] { display: none !important; } diff --git a/packages/overlay/test/focus-trap.common.js b/packages/overlay/test/focus-trap.common.js index 0c7d1badb4..c28954636d 100644 --- a/packages/overlay/test/focus-trap.common.js +++ b/packages/overlay/test/focus-trap.common.js @@ -28,6 +28,8 @@ describe('focus-trap', () => { await nextRender(); overlayPart = overlay.$.overlay; overlay.requestContentUpdate(); + overlay.opened = true; + await oneEvent(overlay, 'vaadin-overlay-open'); focusableElements = getFocusableElements(overlayPart); }); @@ -45,10 +47,7 @@ describe('focus-trap', () => { expect(focusableElements[4]).to.equal(overlay.querySelector('input')); }); - it('should focus focusable elements inside the content when focusTrap = true', async () => { - overlay.opened = true; - await oneEvent(overlay, 'vaadin-overlay-open'); - + it('should focus focusable elements inside the content when focusTrap = true', () => { // Tab for (let i = 0; i < focusableElements.length; i++) { const focusedIndex = getFocusedElementIndex(); @@ -68,10 +67,7 @@ describe('focus-trap', () => { expect(getFocusedElementIndex()).to.equal(focusableElements.length - 1); }); - it('should update focus sequence when focusing a random element', async () => { - overlay.opened = true; - await oneEvent(overlay, 'vaadin-overlay-open'); - + it('should update focus sequence when focusing a random element', () => { tabKeyDown(document.body); expect(getFocusedElementIndex()).to.equal(1); @@ -83,15 +79,16 @@ describe('focus-trap', () => { describe('empty', () => { beforeEach(async () => { - overlay = fixtureSync(''); + overlay = fixtureSync(''); await nextRender(); overlayPart = overlay.$.overlay; - focusableElements = getFocusableElements(overlayPart); }); it('should focus the overlay part when focusTrap = true', async () => { + overlay.focusTrap = true; overlay.opened = true; await oneEvent(overlay, 'vaadin-overlay-open'); + focusableElements = getFocusableElements(overlayPart); expect(focusableElements[0]).to.equal(overlayPart); expect(getFocusedElementIndex()).to.equal(0); }); @@ -100,6 +97,7 @@ describe('focus-trap', () => { overlay.focusTrap = false; overlay.opened = true; await oneEvent(overlay, 'vaadin-overlay-open'); + focusableElements = getFocusableElements(overlayPart); expect(getFocusedElementIndex()).to.equal(-1); }); });