Repository: melt-ui/melt-ui
Pull request #644 (2023-10-19 01:17:28)

Add toolbar tests
- Add tests for the `Toolbar` builder.
- Made our example accessible by adding `aria-label`s to the toolbar buttons

changeset-bot[bot]: ###  ⚠️  No Changeset found

Latest commit: 05a38b5df1ace23682d69e737bb19e929497a2dc

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. **If these changes should result in a version bump, you need to add a changeset.**

<details><summary>This PR includes no changesets</summary>

  When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

</details>

[Click here to learn what changesets are, and how to add one](https://github.com/changesets/changesets/blob/main/docs/adding-a-changeset.md).

[Click here if you're a maintainer who wants to add a changeset to this PR](https://github.com/huntabyte/melt-ui/new/toolbar-tests?filename=.changeset/serious-cows-lick.md&value=---%0A%22%40melt-ui%2Fsvelte%22%3A%20patch%0A---%0A%0AAdd%20toolbar%20tests%0A)


TGlide: @huntabyte merge conflicts :)
huntabyte: Check failing from unrelated `pagefind`!

Patch:
diff --git a/.changeset/empty-timers-enjoy.md b/.changeset/empty-timers-enjoy.md
new file mode 100644
index 00000000..a966d089
--- /dev/null
+++ b/.changeset/empty-timers-enjoy.md
@@ -0,0 +1,6 @@
+---
+"@melt-ui/svelte": patch
+---
+
+DropdownMenu: Implement `disableTriggerRefocus` prop
+
diff --git a/.changeset/serious-cows-lick.md b/.changeset/serious-cows-lick.md
new file mode 100644
index 00000000..9f91edb7
--- /dev/null
+++ b/.changeset/serious-cows-lick.md
@@ -0,0 +1,5 @@
+---
+"@melt-ui/svelte": patch
+---
+
+- Apply correct aria attributes depending on `type` of toolbar group
diff --git a/.eslintrc.cjs b/.eslintrc.cjs
index 27089285..7f3e30b3 100644
--- a/.eslintrc.cjs
+++ b/.eslintrc.cjs
@@ -57,6 +57,6 @@ module.exports = {
 	},
 	globals: {
 		NodeJS: true,
-		$$Generic: 'readable',
+		$$Generic: 'writable',
 	},
 };
diff --git a/src/docs/constants.ts b/src/docs/constants.ts
index aad2393a..12056c34 100644
--- a/src/docs/constants.ts
+++ b/src/docs/constants.ts
@@ -46,6 +46,12 @@ export const PROPS = {
 		description: DESCRIPTIONS.FORCE_VISIBLE(args.name ?? 'element'),
 		default: args.default ?? 'false',
 	}),
+	DISABLE_TRIGGER_REFOCUS: (args: PropArgs = {}): Prop => ({
+		name: 'disableTriggerRefocus',
+		type: 'boolean',
+		description: `Whether or not to refocus the trigger when the ${args.name ?? 'element'} closes`,
+		default: args.default ?? 'false',
+	}),
 	DISABLED: (args: PropArgs = {}): Prop => ({
 		name: 'disabled',
 		type: 'boolean',
diff --git a/src/docs/data/builders/menu.ts b/src/docs/data/builders/menu.ts
index e4662aee..25029686 100644
--- a/src/docs/data/builders/menu.ts
+++ b/src/docs/data/builders/menu.ts
@@ -23,6 +23,7 @@ export const menuBuilderProps = [
 	PROPS.DEFAULT_OPEN,
 	PROPS.OPEN,
 	PROPS.ON_OPEN_CHANGE,
+	PROPS.DISABLE_TRIGGER_REFOCUS
 ];
 
 export const menuBuilderOptions = [
diff --git a/src/docs/previews/toolbar/main/tailwind/index.svelte b/src/docs/previews/toolbar/main/tailwind/index.svelte
index aa5fee7d..03dc7895 100644
--- a/src/docs/previews/toolbar/main/tailwind/index.svelte
+++ b/src/docs/previews/toolbar/main/tailwind/index.svelte
@@ -30,25 +30,37 @@
 	class="flex min-w-max items-center gap-4 rounded-md bg-white px-3 py-3 text-neutral-700 shadow-sm lg:w-[35rem]"
 >
 	<div class="flex items-center gap-1" use:melt={$fontGroup}>
-		<button class="item" use:melt={$fontItem('bold')}>
+		<button class="item" aria-label="bold" use:melt={$fontItem('bold')}>
 			<Bold class="square-5" />
 		</button>
-		<button class="item" use:melt={$fontItem('italic')}>
+		<button class="item" aria-label="italic" use:melt={$fontItem('italic')}>
 			<Italic class="square-5" />
 		</button>
-		<button class="item" use:melt={$fontItem('strikethrough')}>
+		<button
+			class="item"
+			aria-label="strikethrough"
+			use:melt={$fontItem('strikethrough')}
+		>
 			<Strikethrough class="square-5" />
 		</button>
 	</div>
 	<div class="separator" use:melt={$separator} />
 	<div class="flex items-center gap-1" use:melt={$alignGroup}>
-		<button class="item" use:melt={$alignItem('left')}>
+		<button class="item" aria-label="align left" use:melt={$alignItem('left')}>
 			<AlignLeft class="square-5" />
 		</button>
-		<button class="item" use:melt={$alignItem('center')}>
+		<button
+			class="item"
+			aria-label="align center"
+			use:melt={$alignItem('center')}
+		>
 			<AlignCenter class="square-5" />
 		</button>
-		<button class="item" use:melt={$alignItem('right')}>
+		<button
+			class="item"
+			aria-label="align-right"
+			use:melt={$alignItem('right')}
+		>
 			<AlignRight class="square-5" />
 		</button>
 	</div>
diff --git a/src/lib/builders/dropdown-menu/create.ts b/src/lib/builders/dropdown-menu/create.ts
index 800f7b05..aa4f12f2 100644
--- a/src/lib/builders/dropdown-menu/create.ts
+++ b/src/lib/builders/dropdown-menu/create.ts
@@ -48,7 +48,7 @@ export function createDropdownMenu(props?: CreateDropdownMenuProps) {
 		rootActiveTrigger,
 		nextFocusable,
 		prevFocusable,
-		disableTriggerRefocus: true,
+		disableTriggerRefocus: props?.disableTriggerRefocus ?? false,
 		selector: 'dropdown-menu',
 		removeScroll: true,
 	});
diff --git a/src/lib/builders/menu/create.ts b/src/lib/builders/menu/create.ts
index 74370b4a..904e43cc 100644
--- a/src/lib/builders/menu/create.ts
+++ b/src/lib/builders/menu/create.ts
@@ -547,7 +547,6 @@ export function createMenuBuilder(opts: _MenuBuilderOptions) {
 
 						if (e.defaultPrevented) {
 							if (!isHTMLElement(itemEl)) return;
-
 							handleRovingFocus(itemEl);
 							return;
 						}
@@ -871,7 +870,6 @@ export function createMenuBuilder(opts: _MenuBuilderOptions) {
 							if (!isHTMLElement(menuEl)) return;
 
 							const firstItem = getMenuItems(menuEl)[0];
-
 							handleRovingFocus(firstItem);
 						}
 					}),
@@ -883,7 +881,6 @@ export function createMenuBuilder(opts: _MenuBuilderOptions) {
 
 						const triggerEl = e.currentTarget;
 						if (!isHTMLElement(triggerEl)) return;
-
 						handleRovingFocus(triggerEl);
 
 						const openTimer = get(subOpenTimer);
@@ -1102,7 +1099,7 @@ export function createMenuBuilder(opts: _MenuBuilderOptions) {
 				unsubs.push(removeScroll());
 			}
 
-			if (!$rootOpen && $rootActiveTrigger) {
+			if (!$rootOpen && $rootActiveTrigger && opts.disableTriggerRefocus === false) {
 				handleRovingFocus($rootActiveTrigger);
 			}
 
@@ -1121,6 +1118,9 @@ export function createMenuBuilder(opts: _MenuBuilderOptions) {
 					handleRovingFocus(menuItems[0]);
 				} else if ($rootActiveTrigger) {
 					// Focus on active trigger trigger
+					if (opts.disableTriggerRefocus) {
+						return;
+					}
 					handleRovingFocus($rootActiveTrigger);
 				} else {
 					if (opts.disableTriggerRefocus) {
@@ -1217,7 +1217,6 @@ export function createMenuBuilder(opts: _MenuBuilderOptions) {
 
 		const parentMenuEl = getParentMenu(target);
 		if (!parentMenuEl) return;
-
 		handleRovingFocus(parentMenuEl);
 	}
 
@@ -1465,7 +1464,6 @@ export function handleMenuNavigation(e: KeyboardEvent, loop?: boolean) {
 		default:
 			return;
 	}
-
 	handleRovingFocus(candidateNodes[nextIndex]);
 }
 
diff --git a/src/lib/builders/menu/types.ts b/src/lib/builders/menu/types.ts
index 07cf35c9..e4bdbeed 100644
--- a/src/lib/builders/menu/types.ts
+++ b/src/lib/builders/menu/types.ts
@@ -99,6 +99,13 @@ export type _CreateMenuProps = {
 	 * @default true
 	 */
 	typeahead?: boolean;
+
+	/**
+	 * Whether to disable the functionality that the trigger element is refocused when the menu is closed.
+	 *
+	 * @type {boolean}
+	 */
+	disableTriggerRefocus?: boolean;
 };
 
 export type _CreateSubmenuProps = Pick<_CreateMenuProps, 'arrowSize' | 'positioning'> & {
diff --git a/src/lib/builders/toolbar/create.ts b/src/lib/builders/toolbar/create.ts
index 2a26db3a..eae0ca67 100644
--- a/src/lib/builders/toolbar/create.ts
+++ b/src/lib/builders/toolbar/create.ts
@@ -130,6 +130,9 @@ export const createToolbar = (props?: CreateToolbarProps) => {
 					const disabled = $disabled || argDisabled;
 
 					const pressed = Array.isArray($value) ? $value.includes(itemValue) : $value === itemValue;
+
+					const isSingle = $type === 'single';
+					const isMultiple = $type === 'multiple';
 					return {
 						disabled: disabledAttr(disabled),
 						pressed,
@@ -137,9 +140,10 @@ export const createToolbar = (props?: CreateToolbarProps) => {
 						'data-disabled': disabledAttr(disabled),
 						'data-value': itemValue,
 						'data-state': pressed ? 'on' : 'off',
-						'aria-pressed': pressed,
+						'aria-checked': isSingle ? pressed : undefined,
+						'aria-pressed': isMultiple ? pressed : undefined,
 						type: 'button',
-						role: $type === 'single' ? 'radio' : undefined,
+						role: isSingle ? 'radio' : undefined,
 						'data-melt-toolbar-item': '',
 					} as const;
 				};


Test patch:
diff --git a/src/tests/toolbar/Toolbar.spec.ts b/src/tests/toolbar/Toolbar.spec.ts
new file mode 100644
index 00000000..9a00ef09
--- /dev/null
+++ b/src/tests/toolbar/Toolbar.spec.ts
@@ -0,0 +1,294 @@
+import { kbd } from '$lib/internal/helpers/keyboard.js';
+import { render } from '@testing-library/svelte';
+import userEvent from '@testing-library/user-event';
+import { axe } from 'jest-axe';
+import { describe } from 'vitest';
+import ToolbarTest from './ToolbarTest.svelte';
+import { writable } from 'svelte/store';
+import { tick } from 'svelte';
+
+const group1Items = ['item-1', 'item-2', 'item-3'];
+const group2Items = ['item-4', 'item-5', 'item-6'];
+const tbItems = [...group1Items, ...group2Items];
+
+const tbLinks = ['link-1'];
+const tbButtons = ['button-1'];
+
+const allToolbarItems = [...tbItems, ...tbLinks, ...tbButtons];
+
+describe('Toolbar', () => {
+	test('has no accessibility violations', async () => {
+		const { container } = render(ToolbarTest);
+		expect(await axe(container)).toHaveNoViolations();
+	});
+
+	test("tab enters toolbar but doesn't navigate between items", async () => {
+		const user = userEvent.setup();
+		const { getByTestId } = render(ToolbarTest);
+
+		await user.tab();
+		expect(getByTestId('item-1')).toHaveFocus();
+		await user.tab();
+		expect(getByTestId('item-2')).not.toHaveFocus();
+		await user.tab({ shift: true });
+		expect(getByTestId('item-1')).toHaveFocus();
+		await user.tab({ shift: true });
+		expect(getByTestId('item-1')).not.toHaveFocus();
+	});
+
+	test('arrow keys navigate between items', async () => {
+		const user = userEvent.setup();
+		const { getByTestId } = render(ToolbarTest);
+
+		await user.tab();
+
+		const reversedItems = [...allToolbarItems].reverse();
+
+		for (const item of allToolbarItems) {
+			expect(getByTestId(item)).toHaveFocus();
+			await user.keyboard(`{${kbd.ARROW_RIGHT}}`);
+		}
+
+		for (const item of reversedItems) {
+			expect(getByTestId(item)).toHaveFocus();
+			await user.keyboard(`{${kbd.ARROW_LEFT}}`);
+		}
+	});
+
+	test('home and end keys focus first and last items - horizontal', async () => {
+		const user = userEvent.setup();
+		const { getByTestId } = render(ToolbarTest);
+
+		await user.tab();
+		const firstItem = getByTestId('item-1');
+		const lastItem = getByTestId('button-1');
+		expect(firstItem).toHaveFocus();
+		await user.keyboard(`{${kbd.END}}`);
+		expect(lastItem).toHaveFocus();
+		await user.keyboard(`{${kbd.HOME}}`);
+		expect(firstItem).toHaveFocus();
+	});
+
+	test('home and end keys focus first and last items - vertical', async () => {
+		const user = userEvent.setup();
+		const { getByTestId } = render(ToolbarTest, {
+			orientation: 'vertical',
+		});
+
+		await user.tab();
+		const firstItem = getByTestId('item-1');
+		const lastItem = getByTestId('button-1');
+		expect(firstItem).toHaveFocus();
+		await user.keyboard(`{${kbd.END}}`);
+		expect(lastItem).toHaveFocus();
+		await user.keyboard(`{${kbd.HOME}}`);
+		expect(firstItem).toHaveFocus();
+	});
+
+	test('items can be selected with keyboard', async () => {
+		const user = userEvent.setup();
+		const { getByTestId } = render(ToolbarTest);
+
+		for (const item of group1Items) {
+			const curr = getByTestId(item);
+			curr.focus();
+			expect(curr).toHaveFocus();
+			await user.keyboard(`{${kbd.SPACE}}`);
+			expect(curr).toHaveAttribute('aria-pressed', 'true');
+			expect(curr).toHaveAttribute('data-state', 'on');
+		}
+
+		for (const item of group2Items) {
+			const curr = getByTestId(item);
+			curr.focus();
+			expect(curr).toHaveFocus();
+			await user.keyboard(`{${kbd.SPACE}}`);
+			expect(curr).toHaveAttribute('aria-checked', 'true');
+			expect(curr).toHaveAttribute('data-state', 'on');
+		}
+	});
+
+	test('items can be selected with mouse', async () => {
+		const user = userEvent.setup();
+		const { getByTestId } = render(ToolbarTest);
+
+		for (const item of group1Items) {
+			const curr = getByTestId(item);
+			await user.click(curr);
+			expect(curr).toHaveAttribute('aria-pressed', 'true');
+			expect(curr).toHaveAttribute('data-state', 'on');
+		}
+
+		for (const item of group2Items) {
+			const curr = getByTestId(item);
+			await user.click(curr);
+			expect(curr).toHaveAttribute('aria-checked', 'true');
+			expect(curr).toHaveAttribute('data-state', 'on');
+		}
+	});
+
+	test('multiple items can be selected in `multiple` groups', async () => {
+		const user = userEvent.setup();
+		const { getByTestId } = render(ToolbarTest);
+
+		for (const item of group1Items) {
+			const curr = getByTestId(item);
+			await user.click(curr);
+		}
+
+		const group1 = getByTestId('group-1');
+		const group1OnItems = group1.querySelectorAll<HTMLElement>('[data-state="on"]');
+		expect(group1OnItems.length).toBe(group1Items.length);
+	});
+
+	test('multiple items cannot be selected in `single` groups', async () => {
+		const user = userEvent.setup();
+		const { getByTestId } = render(ToolbarTest);
+
+		const group2Items = ['item-4', 'item-5', 'item-6'];
+
+		for (const item of group2Items) {
+			const curr = getByTestId(item);
+			await user.click(curr);
+		}
+
+		const group2 = getByTestId('group-2');
+		const group2OnItems = group2.querySelectorAll<HTMLElement>('[data-state="on"]');
+		expect(group2OnItems.length).toBe(1);
+	});
+
+	test('orientation changes keyboard nav', async () => {
+		const user = userEvent.setup();
+		const { getByTestId } = render(ToolbarTest, {
+			orientation: 'vertical',
+		});
+
+		await user.tab();
+
+		const reversedItems = [...allToolbarItems].reverse();
+
+		for (const item of allToolbarItems) {
+			expect(getByTestId(item)).toHaveFocus();
+			await user.keyboard(`{${kbd.ARROW_DOWN}}`);
+		}
+
+		for (const item of reversedItems) {
+			expect(getByTestId(item)).toHaveFocus();
+			await user.keyboard(`{${kbd.ARROW_UP}}`);
+		}
+	});
+
+	test('default value sets initial pressed/checked items', async () => {
+		const { getByTestId } = render(ToolbarTest, {
+			orientation: 'vertical',
+			toolbarGroup1Props: {
+				defaultValue: ['item-1', 'item-2'],
+			},
+			toolbarGroup2Props: {
+				defaultValue: 'item-5',
+			},
+		});
+
+		const group1 = getByTestId('group-1');
+		const group1OnItems = group1.querySelectorAll<HTMLElement>('[data-state="on"]');
+		expect(group1OnItems.length).toBe(2);
+		expect(group1OnItems[0]).toHaveAttribute('data-testid', 'item-1');
+		expect(group1OnItems[1]).toHaveAttribute('data-testid', 'item-2');
+
+		const group2 = getByTestId('group-2');
+		const group2OnItems = group2.querySelectorAll<HTMLElement>('[data-state="on"]');
+		expect(group2OnItems.length).toBe(1);
+		expect(group2OnItems[0]).toHaveAttribute('data-testid', 'item-5');
+	});
+
+	test('onValueChange function can override value', async () => {
+		const user = userEvent.setup();
+		const { getByTestId } = render(ToolbarTest, {
+			orientation: 'vertical',
+			toolbarGroup1Props: {
+				onValueChange: () => {
+					return ['item-1'];
+				},
+			},
+			toolbarGroup2Props: {
+				onValueChange: () => {
+					return 'item-5';
+				},
+			},
+		});
+
+		const item1 = getByTestId('item-1');
+		const item2 = getByTestId('item-2');
+		await user.click(item2);
+		expect(item2).not.toHaveAttribute('data-state', 'on');
+		expect(item1).toHaveAttribute('data-state', 'on');
+
+		const item5 = getByTestId('item-5');
+		const item6 = getByTestId('item-6');
+		await user.click(item6);
+		expect(item6).not.toHaveAttribute('data-state', 'on');
+		expect(item5).toHaveAttribute('data-state', 'on');
+	});
+
+	test('value prop overrides default value', async () => {
+		const valueStore1 = writable(['item-1', 'item-2']);
+		const valueStore2 = writable('item-5');
+		const { getByTestId } = render(ToolbarTest, {
+			orientation: 'vertical',
+			toolbarGroup1Props: {
+				defaultValue: ['item-3'],
+				value: valueStore1,
+			},
+			toolbarGroup2Props: {
+				defaultValue: 'item-6',
+				value: valueStore2,
+			},
+		});
+
+		const group1 = getByTestId('group-1');
+		const group1OnItems = group1.querySelectorAll<HTMLElement>('[data-state="on"]');
+		expect(group1OnItems.length).toBe(2);
+		expect(group1OnItems[0]).toHaveAttribute('data-testid', 'item-1');
+		expect(group1OnItems[1]).toHaveAttribute('data-testid', 'item-2');
+
+		const group2 = getByTestId('group-2');
+		const group2OnItems = group2.querySelectorAll<HTMLElement>('[data-state="on"]');
+		expect(group2OnItems.length).toBe(1);
+		expect(group2OnItems[0]).toHaveAttribute('data-testid', 'item-5');
+	});
+
+	test('can be programmatically updated using value store', async () => {
+		const valueStore1 = writable<string[]>([]);
+		const valueStore2 = writable<string>(undefined);
+		const { getByTestId } = render(ToolbarTest, {
+			orientation: 'vertical',
+			toolbarGroup1Props: {
+				value: valueStore1,
+			},
+			toolbarGroup2Props: {
+				value: valueStore2,
+			},
+		});
+
+		const group1 = getByTestId('group-1');
+		const group1OnItems = group1.querySelectorAll<HTMLElement>('[data-state="on"]');
+		expect(group1OnItems.length).toBe(0);
+
+		const group2 = getByTestId('group-2');
+		const group2OnItems = group2.querySelectorAll<HTMLElement>('[data-state="on"]');
+		expect(group2OnItems.length).toBe(0);
+
+		valueStore1.update(() => ['item-1', 'item-2']);
+		await tick();
+		const group1OnItems2 = group1.querySelectorAll<HTMLElement>('[data-state="on"]');
+		expect(group1OnItems2.length).toBe(2);
+		expect(group1OnItems2[0]).toHaveAttribute('data-testid', 'item-1');
+		expect(group1OnItems2[1]).toHaveAttribute('data-testid', 'item-2');
+
+		valueStore2.update(() => 'item-5');
+		await tick();
+		const group2OnItems2 = group2.querySelectorAll<HTMLElement>('[data-state="on"]');
+		expect(group2OnItems2.length).toBe(1);
+		expect(group2OnItems2[0]).toHaveAttribute('data-testid', 'item-5');
+	});
+});
diff --git a/src/tests/toolbar/ToolbarTest.svelte b/src/tests/toolbar/ToolbarTest.svelte
new file mode 100644
index 00000000..0af7dd03
--- /dev/null
+++ b/src/tests/toolbar/ToolbarTest.svelte
@@ -0,0 +1,62 @@
+<script lang="ts">
+	import {
+		createToolbar,
+		melt,
+		type CreateToolbarProps,
+		type CreateToolbarGroupProps,
+	} from '$lib/index.js';
+
+	export let loop: CreateToolbarProps['loop'] = undefined;
+	export let orientation: CreateToolbarProps['orientation'] = undefined;
+
+	export let toolbarGroup1Props: CreateToolbarGroupProps<'multiple'> = {
+		type: 'multiple',
+		defaultValue: undefined,
+		disabled: undefined,
+		onValueChange: undefined,
+		value: undefined,
+	};
+
+	export let toolbarGroup2Props: CreateToolbarGroupProps = {
+		type: 'single',
+		defaultValue: undefined,
+		disabled: undefined,
+		onValueChange: undefined,
+		value: undefined,
+	};
+
+	const {
+		elements: { root, button, link, separator },
+		builders: { createToolbarGroup },
+	} = createToolbar({
+		loop,
+		orientation,
+	});
+
+	const {
+		elements: { group: fontGroup, item: fontItem },
+	} = createToolbarGroup(toolbarGroup1Props);
+
+	const {
+		elements: { group: alignGroup, item: alignItem },
+	} = createToolbarGroup(toolbarGroup2Props);
+</script>
+
+<main>
+	<div use:melt={$root} data-testid="root">
+		<div use:melt={$fontGroup} data-testid="group-1">
+			<button use:melt={$fontItem('item-1')} data-testid="item-1"> item-1 </button>
+			<button use:melt={$fontItem('item-2')} data-testid="item-2"> item-2 </button>
+			<button use:melt={$fontItem('item-3')} data-testid="item-3"> item-3 </button>
+		</div>
+		<div use:melt={$separator} />
+		<div use:melt={$alignGroup} data-testid="group-2">
+			<button use:melt={$alignItem('item-4')} data-testid="item-4"> item-4 </button>
+			<button use:melt={$alignItem('item-5')} data-testid="item-5"> item-5 </button>
+			<button use:melt={$alignItem('item-6')} data-testid="item-6"> item-6</button>
+		</div>
+		<div use:melt={$separator} />
+		<a href="/" use:melt={$link} data-testid="link-1"> link-1 </a>
+		<button use:melt={$button} data-testid="button-1">button-1</button>
+	</div>
+</main>