From 5cb76857c7dacbb764574f46825285ecb7ba4203 Mon Sep 17 00:00:00 2001 From: konnorrogers Date: Thu, 9 Nov 2023 13:33:20 -0500 Subject: [PATCH] fix nested dialog focus --- src/internal/modal.ts | 2 ++ src/internal/tabbable.test.ts | 46 ++++++++++++++++++++++++++++++++++- 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/src/internal/modal.ts b/src/internal/modal.ts index 9c507d90..08ed3f59 100644 --- a/src/internal/modal.ts +++ b/src/internal/modal.ts @@ -63,11 +63,13 @@ export default class Modal { } private handleFocusIn = () => { + if (!this.isActive()) return this.checkFocus(); }; private handleKeyDown = (event: KeyboardEvent) => { if (event.key !== 'Tab' || this.isExternalActivated) return; + if (!this.isActive()) return if (event.shiftKey) { this.tabDirection = 'backward'; diff --git a/src/internal/tabbable.test.ts b/src/internal/tabbable.test.ts index c6705ffc..7b4ca8f0 100644 --- a/src/internal/tabbable.test.ts +++ b/src/internal/tabbable.test.ts @@ -1,9 +1,11 @@ -import { elementUpdated, expect, fixture } from '@open-wc/testing'; +import { aTimeout, elementUpdated, expect, fixture } from '@open-wc/testing'; import '../../dist/shoelace.js'; import { activeElements, getDeepestActiveElement } from './active-elements.js'; import { html } from 'lit'; import { sendKeys } from '@web/test-runner-commands'; +import { SlDialog } from '../../dist/shoelace.js'; +import { clickOnElement } from './test.js'; async function holdShiftKey(callback: () => Promise) { await sendKeys({ down: 'Shift' }); @@ -174,3 +176,45 @@ it('Should account for when focus is changed from outside sources (like clicking await holdShiftKey(async () => await sendKeys({ press: tabKey })); expect(activeElementsArray()).to.include(closeButton); }); + +// https://github.com/shoelace-style/shoelace/issues/1710 +it("Should respect nested modal instances", async () => { + const dialogOne = () => document.querySelector("#dialog-1") as SlDialog + const dialogTwo = () => document.querySelector("#dialog-2") as SlDialog + + await fixture(html` +
+ dialogOne().show()}> + + dialogTwo().show()} id="open-dialog-2">Open Dialog 2 + Close + + + + + + Close + +
+ `) + + const firstFocusedEl = document.querySelector("#focus-1") + const secondFocusedEl = document.querySelector("#focus-2") + + // So we can trigger auto-focus stuff + await clickOnElement(document.querySelector("#open-dialog-1") as Element) + // These clicks need a ~10ms timeout. Not sure why, if we don't do this, tests get flaky. + await aTimeout(100) + await clickOnElement(document.querySelector("#open-dialog-2") as Element) + await aTimeout(100) + + expect(activeElementsArray()).to.include(firstFocusedEl) + + await sendKeys({ press: tabKey }) + expect(activeElementsArray()).to.include(secondFocusedEl) +}) +