From 04721fad71ae3ac2ef7c549b830627f177175b49 Mon Sep 17 00:00:00 2001 From: konnorrogers Date: Tue, 22 Aug 2023 17:13:27 -0400 Subject: [PATCH] fix: internal logic for tabbable checks slotted elements --- src/internal/active-elements.ts | 22 +++++ src/internal/modal.ts | 21 ++++- src/internal/tabbable.test.ts | 148 ++++++++++++++++++++++++++++++++ src/internal/tabbable.ts | 43 ++++++++-- 4 files changed, 226 insertions(+), 8 deletions(-) create mode 100644 src/internal/active-elements.ts create mode 100644 src/internal/tabbable.test.ts diff --git a/src/internal/active-elements.ts b/src/internal/active-elements.ts new file mode 100644 index 00000000..7153c320 --- /dev/null +++ b/src/internal/active-elements.ts @@ -0,0 +1,22 @@ +/** + * Use a generator so we can iterate and possibly break early. + * @example + * // to operate like a regular array. This gets kinda nullify generator benefits. + * const allActiveElements = [...activeElements()] + * + * // Earlier return + * for (const activeElement of activeElements()) { + * if () { + * break; // Break the loop, dont need to iterate over the whole array or store an array in memory! + * } + * } + */ +export function* activeElements (activeElement: Element | null = document.activeElement): Generator { + if (activeElement == null) return + + yield activeElement + + if ("shadowRoot" in activeElement && activeElement.shadowRoot && activeElement.shadowRoot.mode !== "closed") { + yield* activeElements(activeElement.shadowRoot.activeElement) + } +} diff --git a/src/internal/modal.ts b/src/internal/modal.ts index 86bc866c..ec3f6946 100644 --- a/src/internal/modal.ts +++ b/src/internal/modal.ts @@ -1,4 +1,5 @@ import { getTabbableElements } from './tabbable.js'; +import { activeElements } from './active-elements.js' let activeModals: HTMLElement[] = []; @@ -55,6 +56,21 @@ export default class Modal { return getTabbableElements(this.element).findIndex(el => el === this.currentFocus); } + /** + * Checks if the `startElement` is already focused. This is important if the modal already + * has an existing focused prior to the first tab key. + */ + startElementAlreadyFocused (startElement: HTMLElement) { + for (const activeElement of activeElements()) { + if (startElement === activeElement) { + // + return true + } + } + + return false + } + handleKeyDown = (event: KeyboardEvent) => { if (event.key !== 'Tab') return; @@ -68,7 +84,10 @@ export default class Modal { const tabbableElements = getTabbableElements(this.element); const start = tabbableElements[0]; - let focusIndex = this.currentFocusIndex; + + // Sometimes we programmatically focus the first element in a modal. + // Lets make sure the start element isn't already focused. + let focusIndex = this.startElementAlreadyFocused(start) ? 0 : this.currentFocusIndex; if (focusIndex === -1) { this.currentFocus = start; diff --git a/src/internal/tabbable.test.ts b/src/internal/tabbable.test.ts new file mode 100644 index 00000000..4d9b202a --- /dev/null +++ b/src/internal/tabbable.test.ts @@ -0,0 +1,148 @@ +import { elementUpdated, expect, fixture, aTimeout } from '@open-wc/testing'; + +import "../../dist/shoelace.js" +import { html } from 'lit'; +import type { SlDrawer } from '../../dist/shoelace.js'; +import { sendKeys } from '@web/test-runner-commands'; + +function getActiveElements (el: null | Element = document.activeElement) { + const elements: Element[] = [] + + function walk (el: null | Element) { + if (el == null) { + return + } + + elements.push(el) + + if ("shadowRoot" in el && el.shadowRoot && el.shadowRoot.mode !== "closed") { + walk(el.shadowRoot.activeElement) + } + } + + walk(el) + + return elements +} + +function getDeepestActiveElement (el: null | Element = document.activeElement) { + const activeElements = getActiveElements(el) + + return activeElements[activeElements.length - 1] +} + +async function holdShiftKey (callback: () => Promise) { + await sendKeys({ down: "Shift" }) + await callback() + await sendKeys({ up: "Shift" }) +} + +window.customElements.define("tab-test-1", class extends HTMLElement { + connectedCallback () { + this.attachShadow({ mode: "open" }) + this.shadowRoot!.innerHTML = ` + + + + + + + + ` + } +}) + + +it("Should allow tabbing to slotted elements", async () => { + const el = await fixture(html` + +
+ Focus 1 +
+ +
+ + Focus 3 +
+ +
+
Focus 6
+ +
+ + `) + + const drawer = el.shadowRoot!.querySelector("sl-drawer") as unknown as SlDrawer + + await drawer.show() + + await elementUpdated(drawer) + + const focusZero = drawer.shadowRoot!.querySelector("[role='dialog']") + const focusOne = el.querySelector("#focus-1") + const focusTwo = drawer.shadowRoot!.querySelector("[part~='close-button']") + const focusThree = el.querySelector("#focus-3") + const focusFour = el.querySelector("#focus-4") + const focusFive = el.querySelector("#focus-5") + const focusSix = el.querySelector("#focus-6") + + // When we open drawer, we should be focused on the panel to start. + expect(getDeepestActiveElement()).to.equal(focusZero) + + await sendKeys({ press: "Tab" }) + expect(getActiveElements()).to.include(focusOne) + + // When we hit the key we should go to the "close button" on the drawer + await sendKeys({ press: "Tab" }) + expect(getActiveElements()).to.include(focusTwo) + + await sendKeys({ press: "Tab" }) + expect(getActiveElements()).to.include(focusThree) + + await sendKeys({ press: "Tab" }) + expect(getActiveElements()).to.include(focusFour) + + await sendKeys({ press: "Tab" }) + expect(getActiveElements()).to.include(focusFive) + + await sendKeys({ press: "Tab" }) + expect(getActiveElements()).to.include(focusSix) + + // Now we should loop back to #panel + await sendKeys({ press: "Tab" }) + expect(getActiveElements()).to.include(focusZero) + + // Now we should loop back to #panel + await sendKeys({ press: "Tab" }) + expect(getActiveElements()).to.include(focusOne) + + // Let's reset and try from starting point 0 and go backwards. + await holdShiftKey(async () => await sendKeys({ press: "Tab" })) + expect(getActiveElements()).to.include(focusZero) + + await holdShiftKey(async () => await sendKeys({ press: "Tab" })) + expect(getActiveElements()).to.include(focusSix) + + await holdShiftKey(async () => await sendKeys({ press: "Tab" })) + expect(getActiveElements()).to.include(focusFive) + + await holdShiftKey(async () => await sendKeys({ press: "Tab" })) + expect(getActiveElements()).to.include(focusFour) + + await holdShiftKey(async () => await sendKeys({ press: "Tab" })) + expect(getActiveElements()).to.include(focusThree) + + await holdShiftKey(async () => await sendKeys({ press: "Tab" })) + expect(getActiveElements()).to.include(focusTwo) + + await holdShiftKey(async () => await sendKeys({ press: "Tab" })) + expect(getActiveElements()).to.include(focusOne) + + await holdShiftKey(async () => await sendKeys({ press: "Tab" })) + expect(getActiveElements()).to.include(focusZero) + + await holdShiftKey(async () => await sendKeys({ press: "Tab" })) + expect(getActiveElements()).to.include(focusSix) +}) diff --git a/src/internal/tabbable.ts b/src/internal/tabbable.ts index 4925e2ab..1c426294 100644 --- a/src/internal/tabbable.ts +++ b/src/internal/tabbable.ts @@ -70,10 +70,35 @@ export function getTabbableBoundary(root: HTMLElement | ShadowRoot) { export function getTabbableElements(root: HTMLElement | ShadowRoot) { const allElements: HTMLElement[] = []; + const tabbableElements: HTMLElement[] = []; function walk(el: HTMLElement | ShadowRoot) { if (el instanceof Element) { - allElements.push(el); + // if the element has "inert" we can just no-op it. + if (el.hasAttribute("inert")) { + return + } + + if (!allElements.includes(el)) { + allElements.push(el); + } + + if (!tabbableElements.includes(el) && isTabbable(el)) { + tabbableElements.push(el) + } + + /** + * This looks funky. Basically a slots children will always be picked up *if* they're within the `root` element. + * However, there is an edge case if the `root` is wrapped by another shadowDOM, it won't grab the children. + * This fixes that fun edge case. + */ + const slotElementsOutsideRootElement = (el: HTMLSlotElement) => (el.getRootNode({ composed: true }) as ShadowRoot | null)?.host !== root + + if (el instanceof HTMLSlotElement && slotElementsOutsideRootElement(el)) { + el.assignedElements({ flatten: true }).forEach((el: HTMLElement) => { + walk(el) + }) + } if (el.shadowRoot !== null && el.shadowRoot.mode === 'open') { walk(el.shadowRoot); @@ -86,10 +111,14 @@ export function getTabbableElements(root: HTMLElement | ShadowRoot) { // Collect all elements including the root walk(root); - return allElements.filter(isTabbable).sort((a, b) => { - // Make sure we sort by tabindex. - const aTabindex = Number(a.getAttribute('tabindex')) || 0; - const bTabindex = Number(b.getAttribute('tabindex')) || 0; - return bTabindex - aTabindex; - }); + return tabbableElements + + // Is this worth having? Most sorts will always add increased overhead. And positive tabindexes shouldn't really be used. + // So is it worth being right? Or fast? + // return allElements.filter(isTabbable).sort((a, b) => { + // // Make sure we sort by tabindex. + // const aTabindex = Number(a.getAttribute('tabindex')) || 0; + // const bTabindex = Number(b.getAttribute('tabindex')) || 0; + // return bTabindex - aTabindex; + // }); }