Fix nested dialogs (#1711)

* fix nested dialog focus

* fix nested dialog focus

* fix nested dialog focus

* prettier

* remove index.html

* fix tests

* prettier

docs/pages/resources/changelog.md

@@ -15,6 +15,7 @@ New versions of Shoelace are released as-needed and generally occur when a criti
 ## Next
 
 - Added the ability to call `form.checkValidity()` and it will use Shoelace's custom `checkValidity()` handler. [#1708]
+- Fixed a bug where nested dialogs were not properly trapping focus. [#1711]
 - Fixed a bug with form controls removing the custom validity handlers from the form. [#1708]
 - Fixed a bug in form control components that used a `form` property, but not an attribute. [#1707]
 - Fixed a bug with bundled components using CDN builds not having translations on initial connect [#1696]

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';

src/internal/tabbable.test.ts

@@ -1,9 +1,12 @@
-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 { clickOnElement } from './test.js';
 import { html } from 'lit';
 import { sendKeys } from '@web/test-runner-commands';
+import type { SlDialog } from '../shoelace.js';
+
+import '../../../dist/shoelace.js';
 
 async function holdShiftKey(callback: () => Promise<void>) {
   await sendKeys({ down: 'Shift' });
@@ -174,3 +177,43 @@ 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 = (): SlDialog => document.querySelector('#dialog-1')!;
+  const dialogTwo = (): SlDialog => document.querySelector('#dialog-2')!;
+
+  // lit-a11y doesn't like the "autofocus" attribute.
+  /* eslint-disable */
+  await fixture(html`
+    <div>
+      <sl-button id="open-dialog-1" @click=${() => dialogOne().show()}></sl-button>
+      <sl-dialog id="dialog-1" label="Dialog 1">
+        <sl-button @click=${() => dialogTwo().show()} id="open-dialog-2">Open Dialog 2</sl-button>
+        <sl-button slot="footer" variant="primary">Close</sl-button>
+      </sl-dialog>
+
+      <sl-dialog id="dialog-2" label="Dialog 2">
+        <sl-input id="focus-1" autofocus="" placeholder="I will have focus when the dialog is opened"></sl-input>
+        <sl-input id="focus-2" placeholder="Second input"></sl-input>
+        <sl-button slot="footer" variant="primary" class="close-2">Close</sl-button>
+      </sl-dialog>
+    </div>
+  `);
+  /* eslint-enable */
+
+  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')!);
+  // These clicks need a ~100ms timeout. I'm assuming for animation reasons?
+  await aTimeout(100);
+  await clickOnElement(document.querySelector('#open-dialog-2')!);
+  await aTimeout(100);
+
+  expect(activeElementsArray()).to.include(firstFocusedEl);
+
+  await sendKeys({ press: tabKey });
+  expect(activeElementsArray()).to.include(secondFocusedEl);
+});