revert(focus-trap): drop focus-trap options causing inconsistent focus behavior

src/components/modal/modal.e2e.ts

@@ -1,8 +1,8 @@
-import { E2EPage, newE2EPage } from "@stencil/core/testing";
+import { newE2EPage } from "@stencil/core/testing";
 import { focusable, renders, slots, hidden, t9n } from "../../tests/commonTests";
 import { html } from "../../../support/formatting";
 import { CSS, SLOTS, DURATIONS } from "./resources";
-import { isElementFocused, newProgrammaticE2EPage, skipAnimations, waitForAnimationFrame } from "../../tests/utils";
+import { newProgrammaticE2EPage, skipAnimations } from "../../tests/utils";
 
 describe("calcite-modal properties", () => {
   it("renders", () => renders("calcite-modal", { display: "flex", visible: false }));
@@ -303,131 +303,83 @@ describe("opening and closing behavior", () => {
 
 describe("calcite-modal accessibility checks", () => {
   it("traps focus within the modal when open", async () => {
-    const button1Id = "button1";
-    const button2Id = "button2";
     const page = await newE2EPage();
     await page.setContent(
-      html`<calcite-modal>
+      `<calcite-modal>
         <div slot="content">
-          <button id="${button1Id}">Focus1</button>
-          <button id="${button2Id}">Focus2</button>
+          <button class="btn-1">Focus1</button>
+          <button class="btn-2">Focus1</button>
         </div>
       </calcite-modal>`
     );
-    await skipAnimations(page);
-    await page.waitForChanges();
     const modal = await page.find("calcite-modal");
-    modal.setProperty("open", true);
+    let $button1;
+    let $button2;
+    let $close;
+    await page.$eval(".btn-1", (elm) => ($button1 = elm));
+    await page.$eval(".btn-2", (elm) => ($button2 = elm));
+    await page.$eval("calcite-modal", (elm) => {
+      $close = elm.shadowRoot.querySelector(".close");
+    });
+    await modal.setProperty("open", true);
     await page.waitForChanges();
-
-    expect(await isElementFocused(page, `.${CSS.close}`, { shadowed: true })).toBe(true);
+    expect(document.activeElement).toEqual($close);
     await page.keyboard.press("Tab");
-    expect(await isElementFocused(page, `#${button1Id}`)).toBe(true);
+    expect(document.activeElement).toEqual($button1);
     await page.keyboard.press("Tab");
-    expect(await isElementFocused(page, `#${button2Id}`)).toBe(true);
-
+    expect(document.activeElement).toEqual($button2);
     await page.keyboard.press("Tab");
-    expect(await isElementFocused(page, `.${CSS.close}`, { shadowed: true })).toBe(true);
+    expect(document.activeElement).toEqual($close);
     await page.keyboard.down("Shift");
     await page.keyboard.press("Tab");
-    expect(await isElementFocused(page, `#${button2Id}`)).toBe(true);
-
+    expect(document.activeElement).toEqual($button2);
     await page.keyboard.press("Tab");
-    expect(await isElementFocused(page, `#${button1Id}`)).toBe(true);
+    expect(document.activeElement).toEqual($button1);
   });
 
   it("restores focus to previously focused element when closed", async () => {
-    const initiallyFocusedId = "initially-focused";
-    const initiallyFocusedIdSelector = `#${initiallyFocusedId}`;
     const page = await newE2EPage();
-    await page.setContent(
-      html`
-        <button id="${initiallyFocusedId}">Focus</button>
-        <calcite-modal></calcite-modal>
-      `
-    );
-    await skipAnimations(page);
+    await page.setContent(`<button>Focus</button><calcite-modal></calcite-modal>`);
     const modal = await page.find("calcite-modal");
-    await page.$eval(initiallyFocusedIdSelector, (button: HTMLButtonElement) => {
-      button.focus();
+    let $button;
+    await page.$eval("button", (elm: any) => {
+      $button = elm;
+      $button.focus();
     });
     await modal.setProperty("open", true);
     await page.waitForChanges();
     await modal.setProperty("open", false);
     await page.waitForChanges();
-    expect(await isElementFocused(page, initiallyFocusedIdSelector)).toBe(true);
+    expect(document.activeElement).toEqual($button);
   });
 
   it("traps focus within the modal when open and disabled close button", async () => {
-    const button1Id = "button1";
-    const button2Id = "button2";
     const page = await newE2EPage();
     await page.setContent(
-      html`<calcite-modal close-button-disabled>
+      `<calcite-modal close-button-disabled>
         <div slot="content">
-          <button id="${button1Id}">Focus1</button>
-          <button id="${button2Id}">Focus2</button>
+          <button class="btn-1">Focus1</button>
+          <button class="btn-2">Focus1</button>
         </div>
       </calcite-modal>`
     );
-    await skipAnimations(page);
     const modal = await page.find("calcite-modal");
+    let $button1;
+    let $button2;
+    await page.$eval(".btn-1", (elm) => ($button1 = elm));
+    await page.$eval(".btn-2", (elm) => ($button2 = elm));
 
     await modal.setProperty("open", true);
     await page.waitForChanges();
-    expect(await isElementFocused(page, `#${button1Id}`)).toBe(true);
-
     await page.keyboard.press("Tab");
-    expect(await isElementFocused(page, `#${button2Id}`)).toBe(true);
-
+    expect(document.activeElement).toEqual($button1);
     await page.keyboard.press("Tab");
-    expect(await isElementFocused(page, `#${button1Id}`)).toBe(true);
+    expect(document.activeElement).toEqual($button2);
     await page.keyboard.down("Shift");
     await page.keyboard.press("Tab");
-    expect(await isElementFocused(page, `#${button2Id}`)).toBe(true);
+    expect(document.activeElement).toEqual($button2);
     await page.keyboard.press("Tab");
-    expect(await isElementFocused(page, `#${button1Id}`)).toBe(true);
-  });
-
-  it("traps focus within the modal when user clicks inside the modal", async () => {
-    const page = await newE2EPage();
-    await page.setContent(
-      html`
-        <calcite-modal aria-labelledby="modal-title" close-button-disabled open>
-          <div slot="header" id="modal-title">Modal title</div>
-          <div slot="content">
-            <p>Modal content.</p>
-            <calcite-button icon-start="plus" id="plus" round></calcite-button>
-          </div>
-          <calcite-button slot="back" color="neutral" width="full">Back</calcite-button>
-          <calcite-button slot="secondary" width="full" appearance="outline">Cancel</calcite-button>
-          <calcite-button slot="primary" width="full">Save</calcite-button>
-        </calcite-modal>
-      `
-    );
-    await skipAnimations(page);
-
-    const contentEl = await page.find(`div[slot="content"]`);
-    const backButtonEl = await page.find(`calcite-button[slot="back"]`);
-
-    await page.keyboard.press("Tab");
-    expect(await isElementFocused(page, `#plus`)).toBe(true);
-
-    await page.keyboard.press("Tab");
-    expect(await isElementFocused(page, `[slot="back"]`)).toBe(true);
-
-    await page.keyboard.press("Tab");
-    expect(await isElementFocused(page, `[slot="secondary"]`)).toBe(true);
-
-    await page.keyboard.press("Tab");
-    expect(await isElementFocused(page, `[slot="primary"]`)).toBe(true);
-
-    await contentEl.click();
-    await page.waitForChanges();
-    await backButtonEl.click();
-    await page.waitForChanges();
-
-    expect(await isElementFocused(page, `[slot="back"]`)).toBe(true);
+    expect(document.activeElement).toEqual($button1);
   });
 
   describe("setFocus", () => {

src/components/popover/popover.e2e.ts

@@ -734,48 +734,4 @@ describe("calcite-popover", () => {
         shadowFocusTargetSelector: `.${CSS.closeButton}`
       }));
   });
-
-  it("should focus input element in the page with popover when user click", async () => {
-    // refer to https://github.com/Esri/calcite-components/issues/5993 for context
-    const page = await newE2EPage();
-    await page.setContent(html` <calcite-shell content-behind>
-        <calcite-shell-panel slot="panel-end">
-          <calcite-label>
-            value
-            <calcite-input-number value="0" min="0" max="10" id="shell-input"></calcite-input-number>
-          </calcite-label>
-          <calcite-button id="button">open popover</calcite-button>
-        </calcite-shell-panel>
-      </calcite-shell>
-
-      <calcite-popover reference-element="button">
-        <calcite-panel heading="popover panel header" closable="true" style="height: 400px">
-          <calcite-input-number value="5" min="0" max="10" id="popover-input"></calcite-input-number>
-        </calcite-panel>
-      </calcite-popover>`);
-
-    const popover = await page.find("calcite-popover");
-    expect(await popover.getProperty("open")).toBe(false);
-
-    const referenceElement = await page.find("calcite-button#button");
-    await referenceElement.click();
-    await page.waitForChanges();
-    expect(await popover.getProperty("open")).toBe(true);
-
-    const inputElInPopover = await page.find("calcite-input-number#popover-input");
-    await inputElInPopover.click();
-    expect(await page.evaluate(() => document.activeElement.id)).toBe("popover-input");
-
-    await page.keyboard.press("Backspace");
-    await page.keyboard.type("12345");
-    expect(await inputElInPopover.getProperty("value")).toBe("12345");
-
-    const inputElInShell = await page.find("calcite-input-number#shell-input");
-    await inputElInShell.click();
-    expect(await page.evaluate(() => document.activeElement.id)).toBe("shell-input");
-
-    await page.keyboard.press("Backspace");
-    await page.keyboard.type("12345");
-    expect(await inputElInShell.getProperty("value")).toBe("12345");
-  });
 });

src/tests/utils.ts

@@ -271,35 +271,3 @@ export async function skipAnimations(page: E2EPage): Promise<void> {
     content: `:root { --calcite-duration-factor: 0; }`
   });
 }
-
-interface MatchesFocusedElementOptions {
-  /**
-   * Set this to true when the focused element is expected to reside in the shadow DOM
-   */
-  shadowed: boolean;
-}
-
-/**
- * This util helps determine if a selector matches the currently focused element.
- *
- * @param page – the E2E page
- * @param selector – selector of element to match
- * @param options - options to customize the utility behavior
- */
-export async function isElementFocused(
-  page: E2EPage,
-  selector: string,
-  options?: MatchesFocusedElementOptions
-): Promise<boolean> {
-  const shadowed = options?.shadowed;
-
-  return page.evaluate(
-    (selector: string, shadowed: boolean): boolean => {
-      const targetDoc = shadowed ? document.activeElement?.shadowRoot : document;
-
-      return !!targetDoc?.activeElement?.matches(selector);
-    },
-    selector,
-    shadowed
-  );
-}

src/utils/focusTrapComponent.spec.ts

@@ -12,6 +12,7 @@ describe("focusTrapComponent", () => {
 
     connectFocusTrap(fakeComponent);
 
+    expect(fakeComponent.focusTrapEl.tabIndex).toBe(-1);
     expect(fakeComponent.focusTrap).toBeDefined();
     expect(fakeComponent.focusTrap.active).toBe(false);
 
@@ -33,4 +34,14 @@ describe("focusTrapComponent", () => {
     deactivateFocusTrap(fakeComponent);
     expect(deactivateSpy).toHaveBeenCalledTimes(1);
   });
+
+  it("focusTrapEl with tabIndex`", () => {
+    const fakeComponent = {} as any;
+    fakeComponent.focusTrapEl = document.createElement("div");
+    fakeComponent.focusTrapEl.tabIndex = 0;
+
+    connectFocusTrap(fakeComponent);
+    expect(fakeComponent.focusTrapEl.tabIndex).toBe(0);
+    expect(fakeComponent.focusTrap).toBeDefined();
+  });
 });

src/utils/focusTrapComponent.ts

@@ -42,11 +42,12 @@ export function connectFocusTrap(component: FocusTrapComponent): void {
     return;
   }
 
+  if (focusTrapEl.tabIndex == null) {
+    focusTrapEl.tabIndex = -1;
+  }
+
   const focusTrapOptions: FocusTrapOptions = {
-    allowOutsideClick: true,
-    clickOutsideDeactivates: (event: MouseEvent | TouchEvent) => {
-      return !event.composedPath().find((el) => (el as HTMLElement) === focusTrapEl);
-    },
+    clickOutsideDeactivates: true,
     document: focusTrapEl.ownerDocument,
     escapeDeactivates: false,
     fallbackFocus: focusTrapEl,