From 25a23441da2f7f00a4b0a45fd18cbcdb6eb3663e Mon Sep 17 00:00:00 2001 From: Hunter Johnston <64506580+huntabyte@users.noreply.github.com> Date: Sun, 29 Sep 2024 18:43:53 -0400 Subject: [PATCH] fix(next): `Tabs` ARIA attributes (#702) --- .changeset/rare-islands-divide.md | 5 ++ .../bits-ui/src/lib/bits/tabs/tabs.svelte.ts | 60 +++++++++++++++---- .../lib/bits/utilities/portal/portal.svelte | 10 ++-- .../bits-ui/src/lib/internal/afterSleep.ts | 3 + .../bits-ui/src/lib/internal/afterTick.ts | 2 +- packages/bits-ui/src/lib/internal/arrays.ts | 6 ++ .../bits-ui/src/lib/internal/callbacks.ts | 3 + packages/bits-ui/src/lib/internal/focus.ts | 10 ++++ .../src/lib/internal/getDirectionalKeys.ts | 10 ++++ packages/bits-ui/src/lib/internal/types.ts | 2 +- packages/bits-ui/src/tests/tabs/tabs.test.ts | 25 ++++++++ 11 files changed, 118 insertions(+), 18 deletions(-) create mode 100644 .changeset/rare-islands-divide.md diff --git a/.changeset/rare-islands-divide.md b/.changeset/rare-islands-divide.md new file mode 100644 index 000000000..3b15f8da7 --- /dev/null +++ b/.changeset/rare-islands-divide.md @@ -0,0 +1,5 @@ +--- +"bits-ui": patch +--- + +fix: align `Tabs` component ARIA attribute with W3C spec diff --git a/packages/bits-ui/src/lib/bits/tabs/tabs.svelte.ts b/packages/bits-ui/src/lib/bits/tabs/tabs.svelte.ts index 7d4c15bb4..cb7083158 100644 --- a/packages/bits-ui/src/lib/bits/tabs/tabs.svelte.ts +++ b/packages/bits-ui/src/lib/bits/tabs/tabs.svelte.ts @@ -1,7 +1,9 @@ import { untrack } from "svelte"; +import { SvelteMap } from "svelte/reactivity"; import type { TabsActivationMode } from "./types.js"; import { getAriaOrientation, + getAriaSelected, getDataDisabled, getDataOrientation, getDisabled, @@ -42,6 +44,10 @@ class TabsRootState { disabled: TabsRootStateProps["disabled"]; rovingFocusGroup: UseRovingFocusReturn; triggerIds = $state([]); + // holds the trigger ID for each value to associate it with the content + valueToTriggerId = new SvelteMap(); + // holds the content ID for each value to associate it with the trigger + valueToContentId = new SvelteMap(); constructor(props: TabsRootStateProps) { this.#id = props.id; @@ -65,12 +71,24 @@ class TabsRootState { }); } - registerTrigger = (id: string) => { + registerTrigger = (id: string, value: string) => { this.triggerIds.push(id); + this.valueToTriggerId.set(value, id); + + // returns the deregister function + return () => { + this.triggerIds = this.triggerIds.filter((triggerId) => triggerId !== id); + this.valueToTriggerId.delete(value); + }; }; - deRegisterTrigger = (id: string) => { - this.triggerIds = this.triggerIds.filter((triggerId) => triggerId !== id); + registerContent = (id: string, value: string) => { + this.valueToContentId.set(value, id); + + // returns the deregister function + return () => { + this.valueToContentId.delete(value); + }; }; setValue = (v: string) => { @@ -155,6 +173,7 @@ class TabsTriggerState { #isActive = $derived.by(() => this.#root.value.current === this.#value.current); #isDisabled = $derived.by(() => this.#disabled.current || this.#root.disabled.current); #tabIndex = $state(0); + #ariaControls = $derived.by(() => this.#root.valueToContentId.get(this.#value.current)); constructor(props: TabsTriggerStateProps, root: TabsRootState) { this.#root = root; @@ -169,15 +188,16 @@ class TabsTriggerState { }); $effect(() => { - // we want to track the value + // we want to track the value & id const id = this.#id.current; - // on mount register the trigger - untrack(() => this.#root.registerTrigger(id)); - - return () => { - // deregister on ID change or unmount - this.#root.deRegisterTrigger(id); - }; + const value = this.#value.current; + + untrack(() => { + const deregister = this.#root.registerTrigger(id, value); + return () => { + deregister(); + }; + }); }); $effect(() => { @@ -223,6 +243,8 @@ class TabsTriggerState { "data-value": this.#value.current, "data-orientation": getDataOrientation(this.#root.orientation.current), "data-disabled": getDataDisabled(this.#disabled.current), + "aria-selected": getAriaSelected(this.#isActive), + "aria-controls": this.#ariaControls, [TRIGGER_ATTR]: "", disabled: getDisabled(this.#disabled.current), tabindex: this.#tabIndex, @@ -249,6 +271,7 @@ class TabsContentState { #ref: TabsContentStateProps["ref"]; #value: TabsContentStateProps["value"]; #isActive = $derived.by(() => this.#root.value.current === this.#value.current); + #ariaLabelledBy = $derived.by(() => this.#root.valueToTriggerId.get(this.#value.current)); constructor(props: TabsContentStateProps, root: TabsRootState) { this.#root = root; @@ -260,16 +283,31 @@ class TabsContentState { id: this.#id, ref: this.#ref, }); + + $effect(() => { + // we want to track the value & id + const id = this.#id.current; + const value = this.#value.current; + + untrack(() => { + const deregister = this.#root.registerContent(id, value); + return () => { + deregister(); + }; + }); + }); } props = $derived.by( () => ({ + id: this.#id.current, role: "tabpanel", hidden: getHidden(!this.#isActive), tabindex: 0, "data-value": this.#value.current, "data-state": getTabDataState(this.#isActive), + "aria-labelledby": this.#ariaLabelledBy, [CONTENT_ATTR]: "", }) as const ); diff --git a/packages/bits-ui/src/lib/bits/utilities/portal/portal.svelte b/packages/bits-ui/src/lib/bits/utilities/portal/portal.svelte index 9a277779c..840771663 100644 --- a/packages/bits-ui/src/lib/bits/utilities/portal/portal.svelte +++ b/packages/bits-ui/src/lib/bits/utilities/portal/portal.svelte @@ -12,14 +12,14 @@ function getTarget() { if (!isBrowser || disabled) return null; - let target: HTMLElement | null | DocumentFragment | Element = null; + let localTarget: HTMLElement | null | DocumentFragment | Element = null; if (typeof to === "string") { - target = document.querySelector(to); - if (target === null) { + localTarget = document.querySelector(to); + if (localTarget === null) { throw new Error(`Target element "${to}" not found.`); } } else if (to instanceof HTMLElement || to instanceof DocumentFragment) { - target = to; + localTarget = to; } else { throw new TypeError( `Unknown portal target type: ${ @@ -28,7 +28,7 @@ ); } - return target; + return localTarget; } let instance: any; diff --git a/packages/bits-ui/src/lib/internal/afterSleep.ts b/packages/bits-ui/src/lib/internal/afterSleep.ts index af05a4fea..56b980f4c 100644 --- a/packages/bits-ui/src/lib/internal/afterSleep.ts +++ b/packages/bits-ui/src/lib/internal/afterSleep.ts @@ -1,3 +1,6 @@ +/** + * A utility function that executes a callback after a specified number of milliseconds. + */ export function afterSleep(ms: number, cb: () => void) { setTimeout(cb, ms); } diff --git a/packages/bits-ui/src/lib/internal/afterTick.ts b/packages/bits-ui/src/lib/internal/afterTick.ts index c55a33bc1..cc9c2a599 100644 --- a/packages/bits-ui/src/lib/internal/afterTick.ts +++ b/packages/bits-ui/src/lib/internal/afterTick.ts @@ -2,7 +2,7 @@ import { tick } from "svelte"; import type { AnyFn } from "./types.js"; /** - * Calls the provided callback after the current tick. + * A utility function that executes a callback after the current tick. */ export function afterTick(cb: AnyFn) { tick().then(cb); diff --git a/packages/bits-ui/src/lib/internal/arrays.ts b/packages/bits-ui/src/lib/internal/arrays.ts index 2519c2a87..87f75c91f 100644 --- a/packages/bits-ui/src/lib/internal/arrays.ts +++ b/packages/bits-ui/src/lib/internal/arrays.ts @@ -9,6 +9,9 @@ export function arraysAreEqual>(arr1: T, arr2: T): bool return arr1.every((value, index) => isEqual(value, arr2[index])); } +/** + * A utility function that compares two values for equality. + */ function isEqual(a: unknown, b: unknown): boolean { if (Number.isNaN(a as number) && Number.isNaN(b as number)) { return true; @@ -25,6 +28,9 @@ function isEqual(a: unknown, b: unknown): boolean { return Object.is(a, b); } +/** + * A utility function that compares two values for deep equality. + */ function isDeepEqual(a: unknown, b: unknown): boolean { if (typeof a !== "object" || typeof b !== "object" || a === null || b === null) { return false; diff --git a/packages/bits-ui/src/lib/internal/callbacks.ts b/packages/bits-ui/src/lib/internal/callbacks.ts index a93a3af99..eb2c3f368 100644 --- a/packages/bits-ui/src/lib/internal/callbacks.ts +++ b/packages/bits-ui/src/lib/internal/callbacks.ts @@ -16,6 +16,9 @@ export function executeCallbacks( }; } +/** + * A utility function that does nothing. + */ export function noop() { // } diff --git a/packages/bits-ui/src/lib/internal/focus.ts b/packages/bits-ui/src/lib/internal/focus.ts index dcaa56bdb..4ac39a0f6 100644 --- a/packages/bits-ui/src/lib/internal/focus.ts +++ b/packages/bits-ui/src/lib/internal/focus.ts @@ -18,6 +18,9 @@ export function handleCalendarInitialFocus(calendar: HTMLElement) { if (firstDay) return focusWithoutScroll(firstDay); } +/** + * A utility function that focuses an element without scrolling. + */ export function focusWithoutScroll(element: HTMLElement) { const scrollPosition = { x: window.pageXOffset || document.documentElement.scrollLeft, @@ -27,6 +30,9 @@ export function focusWithoutScroll(element: HTMLElement) { window.scrollTo(scrollPosition.x, scrollPosition.y); } +/** + * A utility function that focuses an element. + */ export function focus(element?: FocusableTarget | null, { select = false } = {}) { if (!(element && element.focus)) return; const previouslyFocusedElement = document.activeElement; @@ -92,6 +98,10 @@ export function getTabbableCandidates(container: HTMLElement) { return nodes; } +/** + * A utility function that returns the first and last elements within a container that are + * visible and focusable. + */ export function getTabbableEdges(container: HTMLElement) { const candidates = getTabbableCandidates(container); const first = findVisible(candidates, container); diff --git a/packages/bits-ui/src/lib/internal/getDirectionalKeys.ts b/packages/bits-ui/src/lib/internal/getDirectionalKeys.ts index e5a71f6fa..b8465d2b9 100644 --- a/packages/bits-ui/src/lib/internal/getDirectionalKeys.ts +++ b/packages/bits-ui/src/lib/internal/getDirectionalKeys.ts @@ -6,6 +6,9 @@ export const LAST_KEYS = [kbd.ARROW_UP, kbd.PAGE_DOWN, kbd.END]; export const FIRST_LAST_KEYS = [...FIRST_KEYS, ...LAST_KEYS]; export const SELECTION_KEYS = [kbd.SPACE, kbd.ENTER]; +/** + * A utility function that returns the next key based on the direction and orientation. + */ export function getNextKey(dir: Direction = "ltr", orientation: Orientation = "horizontal") { return { horizontal: dir === "rtl" ? kbd.ARROW_LEFT : kbd.ARROW_RIGHT, @@ -13,6 +16,9 @@ export function getNextKey(dir: Direction = "ltr", orientation: Orientation = "h }[orientation]; } +/** + * A utility function that returns the previous key based on the direction and orientation. + */ export function getPrevKey(dir: Direction = "ltr", orientation: Orientation = "horizontal") { return { horizontal: dir === "rtl" ? kbd.ARROW_RIGHT : kbd.ARROW_LEFT, @@ -20,6 +26,10 @@ export function getPrevKey(dir: Direction = "ltr", orientation: Orientation = "h }[orientation]; } +/** + * A utility function that returns the next and previous keys based on the direction + * and orientation. + */ export function getDirectionalKeys( dir: Direction = "ltr", orientation: Orientation = "horizontal" diff --git a/packages/bits-ui/src/lib/internal/types.ts b/packages/bits-ui/src/lib/internal/types.ts index 39b8bede6..633b2bb06 100644 --- a/packages/bits-ui/src/lib/internal/types.ts +++ b/packages/bits-ui/src/lib/internal/types.ts @@ -19,7 +19,7 @@ export type WithChild< SnippetProps extends Record = { _default: never }, /** * The underlying DOM element being rendered. You can bind to this prop to - * programatically interact with the element. + * programmatically interact with the element. */ Ref = HTMLElement, > = Omit & { diff --git a/packages/bits-ui/src/tests/tabs/tabs.test.ts b/packages/bits-ui/src/tests/tabs/tabs.test.ts index 5d365e084..15f18e6be 100644 --- a/packages/bits-ui/src/tests/tabs/tabs.test.ts +++ b/packages/bits-ui/src/tests/tabs/tabs.test.ts @@ -224,4 +224,29 @@ describe("tabs", () => { expect(content1).not.toBeVisible(); expect(content3).toBeVisible(); }); + + it("should apply appropriate `aria-controls` and `aria-labelledby` attributes to the `Tabs.Trigger` and `Tabs.Content` components", async () => { + const { getByTestId } = setup(); + const triggers = [ + getByTestId("trigger-1"), + getByTestId("trigger-2"), + getByTestId("trigger-3"), + ]; + + const contents = [ + getByTestId("content-1"), + getByTestId("content-2"), + getByTestId("content-3"), + ]; + + for (let i = 0; i < triggers.length; i++) { + const trigger = triggers[i]!; + const content = contents[i]!; + + expect(content).toHaveAttribute("role", "tabpanel"); + expect(trigger).toHaveAttribute("role", "tab"); + expect(trigger.getAttribute("aria-controls")).toBe(content.id); + expect(content.getAttribute("aria-labelledby")).toBe(trigger.id); + } + }); });