Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(next): Tabs ARIA attributes #702

Merged
merged 3 commits into from
Sep 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/rare-islands-divide.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"bits-ui": patch
---

fix: align `Tabs` component ARIA attribute with W3C spec
60 changes: 49 additions & 11 deletions packages/bits-ui/src/lib/bits/tabs/tabs.svelte.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -42,6 +44,10 @@ class TabsRootState {
disabled: TabsRootStateProps["disabled"];
rovingFocusGroup: UseRovingFocusReturn;
triggerIds = $state<string[]>([]);
// holds the trigger ID for each value to associate it with the content
valueToTriggerId = new SvelteMap<string, string>();
// holds the content ID for each value to associate it with the trigger
valueToContentId = new SvelteMap<string, string>();

constructor(props: TabsRootStateProps) {
this.#id = props.id;
Expand All @@ -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) => {
Expand Down Expand Up @@ -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;
Expand All @@ -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(() => {
Expand Down Expand Up @@ -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,
Expand All @@ -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;
Expand All @@ -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
);
Expand Down
10 changes: 5 additions & 5 deletions packages/bits-ui/src/lib/bits/utilities/portal/portal.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -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: ${
Expand All @@ -28,7 +28,7 @@
);
}

return target;
return localTarget;
}

let instance: any;
Expand Down
3 changes: 3 additions & 0 deletions packages/bits-ui/src/lib/internal/afterSleep.ts
Original file line number Diff line number Diff line change
@@ -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);
}
2 changes: 1 addition & 1 deletion packages/bits-ui/src/lib/internal/afterTick.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 6 additions & 0 deletions packages/bits-ui/src/lib/internal/arrays.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ export function arraysAreEqual<T extends Array<unknown>>(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;
Expand All @@ -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;
Expand Down
3 changes: 3 additions & 0 deletions packages/bits-ui/src/lib/internal/callbacks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ export function executeCallbacks<T extends unknown[]>(
};
}

/**
* A utility function that does nothing.
*/
export function noop() {
//
}
10 changes: 10 additions & 0 deletions packages/bits-ui/src/lib/internal/focus.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
10 changes: 10 additions & 0 deletions packages/bits-ui/src/lib/internal/getDirectionalKeys.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,30 @@ 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,
vertical: kbd.ARROW_DOWN,
}[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,
vertical: kbd.ARROW_UP,
}[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"
Expand Down
2 changes: 1 addition & 1 deletion packages/bits-ui/src/lib/internal/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export type WithChild<
SnippetProps extends Record<PropertyKey, unknown> = { _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<Props, "child" | "children"> & {
Expand Down
25 changes: 25 additions & 0 deletions packages/bits-ui/src/tests/tabs/tabs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
});
});