diff --git a/.changeset/twelve-bees-dance.md b/.changeset/twelve-bees-dance.md new file mode 100644 index 000000000..ed664ff90 --- /dev/null +++ b/.changeset/twelve-bees-dance.md @@ -0,0 +1,5 @@ +--- +"bits-ui": patch +--- + +fix: allow tabbing out of menus to previous and next tabbable item in the dom diff --git a/packages/bits-ui/package.json b/packages/bits-ui/package.json index bc01505ef..299611611 100644 --- a/packages/bits-ui/package.json +++ b/packages/bits-ui/package.json @@ -52,7 +52,8 @@ "@internationalized/date": "^3.5.6", "esm-env": "^1.1.2", "runed": "^0.23.2", - "svelte-toolbelt": "^0.7.1" + "svelte-toolbelt": "^0.7.1", + "tabbable": "^6.2.0" }, "peerDependencies": { "svelte": "^5.11.0" diff --git a/packages/bits-ui/src/lib/bits/menu/menu.svelte.ts b/packages/bits-ui/src/lib/bits/menu/menu.svelte.ts index 75fda2b11..8daaa6b32 100644 --- a/packages/bits-ui/src/lib/bits/menu/menu.svelte.ts +++ b/packages/bits-ui/src/lib/bits/menu/menu.svelte.ts @@ -36,6 +36,8 @@ import { import type { Direction } from "$lib/shared/index.js"; import { isPointerInGraceArea, makeHullFromElements } from "$lib/internal/polygon.js"; import { IsUsingKeyboard } from "$lib/index.js"; +import { getTabbableFrom } from "$lib/internal/tabbable.js"; +import { FocusScopeContext } from "../utilities/focus-scope/use-focus-scope.svelte.js"; export const CONTEXT_MENU_TRIGGER_ATTR = "data-context-menu-trigger"; @@ -63,6 +65,7 @@ export const MenuOpenEvent = new CustomEventDispatcher("bitsmenuopen", { class MenuRootState { isUsingKeyboard = new IsUsingKeyboard(); + ignoreCloseAutoFocus = $state(false); constructor(readonly opts: MenuRootStateProps) {} @@ -83,7 +86,7 @@ class MenuMenuState { constructor( readonly opts: MenuMenuStateProps, readonly root: MenuRootState, - readonly parentMenu?: MenuMenuState + readonly parentMenu: MenuMenuState | null ) { if (parentMenu) { watch( @@ -196,8 +199,48 @@ class MenuContentState { this.#pointerGraceIntent = intent; } + handleTabKeyDown(e: BitsKeyboardEvent) { + /** + * We locate the root `menu`'s trigger by going up the tree until + * we find a menu that has no parent. This will allow us to focus the next + * tabbable element before/after the root trigger. + */ + let rootMenu = this.parentMenu; + while (rootMenu.parentMenu !== null) { + rootMenu = rootMenu.parentMenu; + } + // if for some unforeseen reason the root menu has no trigger, we bail + if (!rootMenu.triggerNode) return; + + // cancel default tab behavior + e.preventDefault(); + + // find the next/previous tabbable + const nodeToFocus = getTabbableFrom(rootMenu.triggerNode, e.shiftKey ? "prev" : "next"); + if (nodeToFocus) { + /** + * We set a flag to ignore the `onCloseAutoFocus` event handler + * as well as the fallbacks inside the focus scope to prevent + * race conditions causing focus to fall back to the body even + * though we're trying to focus the next tabbable element. + */ + this.parentMenu.root.ignoreCloseAutoFocus = true; + rootMenu.onClose(); + nodeToFocus.focus(); + afterTick(() => { + this.parentMenu.root.ignoreCloseAutoFocus = false; + }); + } else { + document.body.focus(); + } + } + onkeydown(e: BitsKeyboardEvent) { if (e.defaultPrevented) return; + if (e.key === kbd.TAB) { + this.handleTabKeyDown(e); + return; + } const target = e.target; const currentTarget = e.currentTarget; @@ -206,6 +249,7 @@ class MenuContentState { const isKeydownInside = target.closest(`[${this.parentMenu.root.getAttr("content")}]`)?.id === this.parentMenu.contentId.current; + const isModifierKey = e.ctrlKey || e.altKey || e.metaKey; const isCharacterKey = e.key.length === 1; @@ -218,8 +262,6 @@ class MenuContentState { const candidateNodes = this.#getCandidateNodes(); if (isKeydownInside) { - // menus do not respect the tab key - if (e.key === kbd.TAB) e.preventDefault(); if (!isModifierKey && isCharacterKey) { this.#handleTypeaheadSearch(e.key, candidateNodes); } @@ -979,6 +1021,7 @@ class ContextMenuTriggerState { "data-disabled": getDataDisabled(this.opts.disabled.current), "data-state": getDataOpenClosed(this.parentMenu.opts.open.current), [CONTEXT_MENU_TRIGGER_ATTR]: "", + tabindex: -1, // onpointerdown: this.onpointerdown, onpointermove: this.onpointermove, @@ -992,11 +1035,17 @@ class ContextMenuTriggerState { type MenuItemCombinedProps = MenuItemSharedStateProps & MenuItemStateProps; export function useMenuRoot(props: MenuRootStateProps) { - return MenuRootContext.set(new MenuRootState(props)); + const root = new MenuRootState(props); + FocusScopeContext.set({ + get ignoreCloseAutoFocus() { + return root.ignoreCloseAutoFocus; + }, + }); + return MenuRootContext.set(root); } export function useMenuMenu(root: MenuRootState, props: MenuMenuStateProps) { - return MenuMenuContext.set(new MenuMenuState(props, root)); + return MenuMenuContext.set(new MenuMenuState(props, root, null)); } export function useMenuSubmenu(props: MenuMenuStateProps) { diff --git a/packages/bits-ui/src/lib/bits/menubar/menubar.svelte.ts b/packages/bits-ui/src/lib/bits/menubar/menubar.svelte.ts index fca777c6f..c4bdf5f1f 100644 --- a/packages/bits-ui/src/lib/bits/menubar/menubar.svelte.ts +++ b/packages/bits-ui/src/lib/bits/menubar/menubar.svelte.ts @@ -1,13 +1,5 @@ -import { - type ReadableBox, - afterTick, - box, - onDestroyEffect, - onMountEffect, - useRefById, -} from "svelte-toolbelt"; -import { untrack } from "svelte"; -import { Context } from "runed"; +import { type ReadableBox, afterTick, box, useRefById } from "svelte-toolbelt"; +import { Context, watch } from "runed"; import type { InteractOutsideBehaviorType } from "../utilities/dismissible-layer/types.js"; import type { ReadableBoxedValues, WritableBoxedValues } from "$lib/internal/box.svelte.js"; import { @@ -24,6 +16,11 @@ import type { BitsPointerEvent, WithRefProps, } from "$lib/internal/types.js"; +import { + FocusScopeContext, + type FocusScopeContextValue, +} from "../utilities/focus-scope/use-focus-scope.svelte.js"; +import { onMount } from "svelte"; const MENUBAR_ROOT_ATTR = "data-menubar-root"; const MENUBAR_TRIGGER_ATTR = "data-menubar-trigger"; @@ -40,19 +37,11 @@ type MenubarRootStateProps = WithRefProps< class MenubarRootState { rovingFocusGroup: UseRovingFocusReturn; - currentTabStopId = box(null); wasOpenedByKeyboard = $state(false); triggerIds = $state([]); valueToContentId = new Map>(); constructor(readonly opts: MenubarRootStateProps) { - this.onMenuClose = this.onMenuClose.bind(this); - this.onMenuOpen = this.onMenuOpen.bind(this); - this.onMenuToggle = this.onMenuToggle.bind(this); - - this.registerTrigger = this.registerTrigger.bind(this); - this.deRegisterTrigger = this.deRegisterTrigger.bind(this); - useRefById(opts); this.rovingFocusGroup = useRovingFocus({ @@ -60,16 +49,37 @@ class MenubarRootState { candidateAttr: MENUBAR_TRIGGER_ATTR, loop: this.opts.loop, orientation: box.with(() => "horizontal"), - currentTabStopId: this.currentTabStopId, }); + + this.onMenuClose = this.onMenuClose.bind(this); + this.onMenuOpen = this.onMenuOpen.bind(this); + this.onMenuToggle = this.onMenuToggle.bind(this); + this.registerTrigger = this.registerTrigger.bind(this); } + /** + * @param id - the id of the trigger to register + * @returns - a function to de-register the trigger + */ registerTrigger(id: string) { this.triggerIds.push(id); + + return () => { + this.triggerIds = this.triggerIds.filter((triggerId) => triggerId !== id); + }; } - deRegisterTrigger(id: string) { - this.triggerIds = this.triggerIds.filter((triggerId) => triggerId !== id); + /** + * @param value - the value of the menu to register + * @param contentId - the content id to associate with the value + * @returns - a function to de-register the menu + */ + registerMenu(value: string, contentId: ReadableBox) { + this.valueToContentId.set(value, contentId); + + return () => { + this.valueToContentId.delete(value); + }; } getTriggers() { @@ -78,9 +88,9 @@ class MenubarRootState { return Array.from(node.querySelectorAll(`[${MENUBAR_TRIGGER_ATTR}]`)); } - onMenuOpen(id: string) { + onMenuOpen(id: string, triggerId: string) { this.opts.value.current = id; - this.currentTabStopId.current = id; + this.rovingFocusGroup.setCurrentTabStopId(triggerId); } onMenuClose() { @@ -115,29 +125,30 @@ class MenubarMenuState { readonly opts: MenubarMenuStateProps, readonly root: MenubarRootState ) { - $effect(() => { - if (!this.open) { - untrack(() => { + watch( + () => this.open, + () => { + if (!this.open) { this.wasOpenedByKeyboard = false; - }); + } } - }); + ); - onMountEffect(() => { - this.root.valueToContentId.set( + onMount(() => { + return this.root.registerMenu( this.opts.value.current, box.with(() => this.contentNode?.id ?? "") ); }); - - onDestroyEffect(() => { - this.root.valueToContentId.delete(this.opts.value.current); - }); } getTriggerNode() { return this.triggerNode; } + + openMenu() { + this.root.onMenuOpen(this.opts.value.current, this.triggerNode?.id ?? ""); + } } type MenubarTriggerStateProps = WithRefProps< @@ -170,12 +181,8 @@ class MenubarTriggerState { }, }); - onMountEffect(() => { - this.root.registerTrigger(opts.id.current); - }); - - onDestroyEffect(() => { - this.root.deRegisterTrigger(opts.id.current); + onMount(() => { + return this.root.registerTrigger(opts.id.current); }); $effect(() => { @@ -193,25 +200,26 @@ class MenubarTriggerState { if (!this.menu.open) { e.preventDefault(); } - this.root.onMenuOpen(this.menu.opts.value.current); + this.menu.openMenu(); } } onpointerenter(_: BitsPointerEvent) { const isMenubarOpen = Boolean(this.root.opts.value.current); if (isMenubarOpen && !this.menu.open) { - this.root.onMenuOpen(this.menu.opts.value.current); + this.menu.openMenu(); this.menu.getTriggerNode()?.focus(); } } onkeydown(e: BitsKeyboardEvent) { if (this.opts.disabled.current) return; + if (e.key === kbd.TAB) return; if (e.key === kbd.ENTER || e.key === kbd.SPACE) { this.root.onMenuToggle(this.menu.opts.value.current); } if (e.key === kbd.ARROW_DOWN) { - this.root.onMenuOpen(this.menu.opts.value.current); + this.menu.openMenu(); } // prevent keydown from scrolling window / first focused item // from inadvertently closing the menu @@ -245,7 +253,7 @@ class MenubarTriggerState { "data-disabled": getDataDisabled(this.opts.disabled.current), "data-menu-value": this.menu.opts.value.current, disabled: this.opts.disabled.current ? true : undefined, - tabIndex: this.#tabIndex, + tabindex: this.#tabIndex, [MENUBAR_TRIGGER_ATTR]: "", onpointerdown: this.onpointerdown, onpointerenter: this.onpointerenter, @@ -265,12 +273,14 @@ type MenubarContentStateProps = WithRefProps< class MenubarContentState { root: MenubarRootState; hasInteractedOutside = $state(false); + focusScopeContext: FocusScopeContextValue; constructor( readonly opts: MenubarContentStateProps, readonly menu: MenubarMenuState ) { this.root = menu.root; + this.focusScopeContext = FocusScopeContext.get(); this.onCloseAutoFocus = this.onCloseAutoFocus.bind(this); this.onFocusOutside = this.onFocusOutside.bind(this); @@ -288,8 +298,11 @@ class MenubarContentState { } onCloseAutoFocus(e: Event) { - const menubarOpen = Boolean(this.root.opts.value.current); - if (!menubarOpen && !this.hasInteractedOutside) { + if ( + !this.root.opts.value.current && + !this.hasInteractedOutside && + !this.focusScopeContext.ignoreCloseAutoFocus + ) { this.menu.getTriggerNode()?.focus(); } @@ -330,16 +343,20 @@ class MenubarContentState { if (isKeydownInsideSubMenu && isPrevKey) return; const items = this.root.getTriggers().filter((trigger) => !trigger.disabled); - let candidateValues = items.map((item) => item.getAttribute("data-menu-value")!); - if (isPrevKey) candidateValues.reverse(); + let candidates = items.map((item) => ({ + value: item.getAttribute("data-menu-value")!, + triggerId: item.id ?? "", + })); + if (isPrevKey) candidates.reverse(); + const candidateValues = candidates.map(({ value }) => value); const currentIndex = candidateValues.indexOf(this.menu.opts.value.current); - candidateValues = this.root.opts.loop.current - ? wrapArray(candidateValues, currentIndex + 1) - : candidateValues.slice(currentIndex + 1); - const [nextValue] = candidateValues; - if (nextValue) this.root.onMenuOpen(nextValue); + candidates = this.root.opts.loop.current + ? wrapArray(candidates, currentIndex + 1) + : candidates.slice(currentIndex + 1); + const [nextValue] = candidates; + if (nextValue) this.menu.root.onMenuOpen(nextValue.value, nextValue.triggerId); } props = $derived.by( diff --git a/packages/bits-ui/src/lib/bits/utilities/focus-scope/use-focus-scope.svelte.ts b/packages/bits-ui/src/lib/bits/utilities/focus-scope/use-focus-scope.svelte.ts index 7f1c84f74..dfed48e5b 100644 --- a/packages/bits-ui/src/lib/bits/utilities/focus-scope/use-focus-scope.svelte.ts +++ b/packages/bits-ui/src/lib/bits/utilities/focus-scope/use-focus-scope.svelte.ts @@ -1,5 +1,5 @@ import { afterSleep, afterTick, box, executeCallbacks, useRefById } from "svelte-toolbelt"; -import { watch } from "runed"; +import { Context, watch } from "runed"; import { on } from "svelte/events"; import { createFocusScopeAPI, @@ -21,6 +21,12 @@ const AutoFocusOnDestroyEvent = new CustomEventDispatcher("focusScope.autoFocusO cancelable: true, }); +export type FocusScopeContextValue = { + ignoreCloseAutoFocus: boolean; +}; + +export const FocusScopeContext = new Context("FocusScope"); + type UseFocusScopeProps = ReadableBoxedValues<{ /** * ID of the focus scope container node. @@ -75,6 +81,7 @@ export function useFocusScope({ const focusScopeStack = createFocusScopeStack(); const focusScope = createFocusScopeAPI(); const ref = box(null); + const ctx = FocusScopeContext.getOr({ ignoreCloseAutoFocus: false }); useRefById({ id, @@ -93,12 +100,15 @@ export function useFocusScope({ if (container.contains(target)) { lastFocusedElement = target; } else { + if (ctx.ignoreCloseAutoFocus) return; focus(lastFocusedElement, { select: true }); } }; const handleFocusOut = (event: FocusEvent) => { - if (focusScope.paused || !container) return; + if (focusScope.paused || !container || ctx.ignoreCloseAutoFocus) { + return; + } const relatedTarget = event.relatedTarget; if (!isHTMLElement(relatedTarget)) return; // A `focusout` event with a `null` `relatedTarget` will happen in at least two cases: @@ -116,7 +126,9 @@ export function useFocusScope({ // If the focus has moved to an actual legitimate element (`relatedTarget !== null`) // that is outside the container, we move focus to the last valid focused element inside. - if (!container.contains(relatedTarget)) focus(lastFocusedElement, { select: true }); + if (!container.contains(relatedTarget)) { + focus(lastFocusedElement, { select: true }); + } }; // When the focused element gets removed from the DOM, browsers move focus @@ -147,11 +159,11 @@ export function useFocusScope({ watch([() => forceMount.current, () => ref.current], ([forceMount, container]) => { if (forceMount) return; const prevFocusedElement = document.activeElement as HTMLElement | null; - handleMount(container, prevFocusedElement); + handleOpen(container, prevFocusedElement); return () => { if (!container) return; - handleDestroy(prevFocusedElement); + handleClose(prevFocusedElement); }; }); @@ -160,16 +172,16 @@ export function useFocusScope({ ([forceMount, container]) => { if (!forceMount) return; const prevFocusedElement = document.activeElement as HTMLElement | null; - handleMount(container, prevFocusedElement); + handleOpen(container, prevFocusedElement); return () => { if (!container) return; - handleDestroy(prevFocusedElement); + handleClose(prevFocusedElement); }; } ); - function handleMount(container: HTMLElement | null, prevFocusedElement: HTMLElement | null) { + function handleOpen(container: HTMLElement | null, prevFocusedElement: HTMLElement | null) { if (!container) container = document.getElementById(id.current); if (!container) return; focusScopeStack.add(focusScope); @@ -192,12 +204,13 @@ export function useFocusScope({ } } - function handleDestroy(prevFocusedElement: HTMLElement | null) { + function handleClose(prevFocusedElement: HTMLElement | null) { const destroyEvent = AutoFocusOnDestroyEvent.createEvent(); onCloseAutoFocus.current(destroyEvent); + const shouldIgnore = ctx.ignoreCloseAutoFocus; afterSleep(0, () => { - if (!destroyEvent.defaultPrevented && prevFocusedElement) { + if (!destroyEvent.defaultPrevented && prevFocusedElement && !shouldIgnore) { focus(prevFocusedElement ?? document.body, { select: true }); } focusScopeStack.remove(focusScope); diff --git a/packages/bits-ui/src/lib/internal/dom.ts b/packages/bits-ui/src/lib/internal/dom.ts index 97fdbb528..e81614b58 100644 --- a/packages/bits-ui/src/lib/internal/dom.ts +++ b/packages/bits-ui/src/lib/internal/dom.ts @@ -1,3 +1,17 @@ +export function getDocument(element?: Element | null) { + return element?.ownerDocument ?? document; +} + +export function activeElement(doc: Document) { + let activeElement = doc.activeElement; + + while (activeElement?.shadowRoot?.activeElement != null) { + activeElement = activeElement.shadowRoot.activeElement; + } + + return activeElement; +} + export function getFirstNonCommentChild(element: HTMLElement | null) { if (!element) return null; for (const child of element.childNodes) { diff --git a/packages/bits-ui/src/lib/internal/tabbable.ts b/packages/bits-ui/src/lib/internal/tabbable.ts new file mode 100644 index 000000000..c984a3e7c --- /dev/null +++ b/packages/bits-ui/src/lib/internal/tabbable.ts @@ -0,0 +1,76 @@ +import { focusable, isFocusable, isTabbable, tabbable } from "tabbable"; +import { activeElement, getDocument } from "./dom.js"; + +function getTabbableOptions() { + return { + getShadowRoot: true, + displayCheck: + // JSDOM does not support the `tabbable` library. To solve this we can + // check if `ResizeObserver` is a real function (not polyfilled), which + // determines if the current environment is JSDOM-like. + typeof ResizeObserver === "function" && + ResizeObserver.toString().includes("[native code]") + ? "full" + : "none", + } as const; +} + +function getTabbableIn(container: HTMLElement, direction: "next" | "prev") { + const allTabbable = tabbable(container, getTabbableOptions()); + + if (direction === "prev") { + allTabbable.reverse(); + } + + const activeEl = activeElement(getDocument(container)) as HTMLElement; + + const activeIndex = allTabbable.indexOf(activeEl); + const nextTabbableElements = allTabbable.slice(activeIndex + 1); + return nextTabbableElements[0]; +} + +/** + * Gets all tabbable elements in the body and finds the next/previous tabbable element + * from the `currentNode` based on the `direction` provided. + * @param currentNode - the node we want to get the next/previous tabbable from + */ +export function getTabbableFrom(currentNode: HTMLElement, direction: "next" | "prev") { + if (!isTabbable(currentNode, getTabbableOptions())) { + return getTabbableFromFocusable(currentNode, direction); + } + const allTabbable = tabbable(getDocument(currentNode).body, getTabbableOptions()); + if (direction === "prev") allTabbable.reverse(); + const activeIndex = allTabbable.indexOf(currentNode); + if (activeIndex === -1) return document.body; + const nextTabbableElements = allTabbable.slice(activeIndex + 1); + return nextTabbableElements[0]; +} + +export function getTabbableFromFocusable(currentNode: HTMLElement, direction: "next" | "prev") { + if (!isFocusable(currentNode, getTabbableOptions())) return document.body; + + // find all focusable nodes, since some elements may be focusable but not tabbable + // such as context menu triggers + const allFocusable = focusable(getDocument(currentNode).body, getTabbableOptions()); + + // find index of current node among focusable siblings + if (direction === "prev") allFocusable.reverse(); + const activeIndex = allFocusable.indexOf(currentNode); + if (activeIndex === -1) return document.body; + + const nextFocusableElements = allFocusable.slice(activeIndex + 1); + + // find the next focusable node that is also tabbable + return ( + nextFocusableElements.find((node) => isTabbable(node, getTabbableOptions())) ?? + document.body + ); +} + +export function getNextTabbable() { + return getTabbableIn(document.body, "next"); +} + +export function getPreviousTabbable() { + return getTabbableIn(document.body, "prev"); +} diff --git a/packages/bits-ui/src/lib/internal/use-roving-focus.svelte.ts b/packages/bits-ui/src/lib/internal/use-roving-focus.svelte.ts index 4093c3b94..3b6614859 100644 --- a/packages/bits-ui/src/lib/internal/use-roving-focus.svelte.ts +++ b/packages/bits-ui/src/lib/internal/use-roving-focus.svelte.ts @@ -1,4 +1,4 @@ -import { type ReadableBox, type WritableBox, box } from "svelte-toolbelt"; +import { type ReadableBox, box } from "svelte-toolbelt"; import { getElemDirection } from "./locale.js"; import { getDirectionalKeys } from "./get-directional-keys.js"; import { kbd } from "./kbd.js"; @@ -36,19 +36,12 @@ type UseRovingFocusProps = { * A callback function called when a candidate is focused. */ onCandidateFocus?: (node: HTMLElement) => void; - - /** - * The current tab stop id. - */ - currentTabStopId?: WritableBox; }; export type UseRovingFocusReturn = ReturnType; export function useRovingFocus(props: UseRovingFocusProps) { - const currentTabStopId: WritableBox = props.currentTabStopId - ? props.currentTabStopId - : box(null); + const currentTabStopId = box(null); function getCandidateNodes() { if (!isBrowser) return []; @@ -88,7 +81,6 @@ export function useRovingFocus(props: UseRovingFocusProps) { const currentIndex = items.indexOf(node); const dir = getElemDirection(rootNode); const { nextKey, prevKey } = getDirectionalKeys(dir, props.orientation.current); - const loop = props.loop.current; const keyToIndex = { @@ -126,6 +118,7 @@ export function useRovingFocus(props: UseRovingFocusProps) { function getTabIndex(node: HTMLElement | null | undefined) { const items = getCandidateNodes(); const anyActive = currentTabStopId.current !== null; + if (node && !anyActive && items[0] === node) { currentTabStopId.current = node.id; return 0; diff --git a/packages/tests/src/tests/context-menu/context-menu-force-mount-test.svelte b/packages/tests/src/tests/context-menu/context-menu-force-mount-test.svelte index c08ccd65c..4cf212166 100644 --- a/packages/tests/src/tests/context-menu/context-menu-force-mount-test.svelte +++ b/packages/tests/src/tests/context-menu/context-menu-force-mount-test.svelte @@ -103,6 +103,7 @@
outside
+
- + diff --git a/packages/tests/src/tests/context-menu/context-menu-test.svelte b/packages/tests/src/tests/context-menu/context-menu-test.svelte index 21d67b21e..aa1a5e9fc 100644 --- a/packages/tests/src/tests/context-menu/context-menu-test.svelte +++ b/packages/tests/src/tests/context-menu/context-menu-test.svelte @@ -26,6 +26,7 @@
outside
+
+ diff --git a/packages/tests/src/tests/context-menu/context-menu.test.ts b/packages/tests/src/tests/context-menu/context-menu.test.ts index 10227ac9a..8eb06bffe 100644 --- a/packages/tests/src/tests/context-menu/context-menu.test.ts +++ b/packages/tests/src/tests/context-menu/context-menu.test.ts @@ -1,7 +1,6 @@ import { cleanup, render, screen, waitFor } from "@testing-library/svelte/svelte5"; import { axe } from "jest-axe"; import { describe, it } from "vitest"; -import type { Component } from "svelte"; import { getTestKbd, setupUserEvents } from "../utils.js"; import ContextMenuTest from "./context-menu-test.svelte"; import type { ContextMenuTestProps } from "./context-menu-test.svelte"; @@ -10,15 +9,17 @@ import ContextMenuForceMountTest from "./context-menu-force-mount-test.svelte"; const kbd = getTestKbd(); +type ContextMenuSetupProps = (ContextMenuTestProps | ContextMenuForceMountTestProps) & { + component?: typeof ContextMenuTest | typeof ContextMenuForceMountTest; +}; + /** * Helper function to reduce boilerplate in tests */ -function setup( - props: ContextMenuTestProps | ContextMenuForceMountTestProps = {}, - component: Component = ContextMenuTest -) { +function setup(props: ContextMenuSetupProps = {}) { + const { component = ContextMenuTest, ...rest } = props; const user = setupUserEvents(); - const returned = render(component, { ...props }); + const returned = render(component, { ...rest }); const trigger = returned.getByTestId("trigger"); function getContent() { return returned.queryByTestId("content"); @@ -36,7 +37,7 @@ function setup( }; } -async function open(props: ContextMenuTestProps = {}) { +async function open(props: ContextMenuSetupProps = {}) { const { getByTestId, queryByTestId, user, trigger, getContent, ...returned } = setup(props); expect(getContent()).toBeNull(); @@ -311,18 +312,16 @@ describe("context menu", () => { }); it("should forceMount the content when `forceMount` is true", async () => { - const { getByTestId } = setup({}, ContextMenuForceMountTest); + const { getByTestId } = setup({ component: ContextMenuForceMountTest }); expect(getByTestId("content")).toBeVisible(); }); it("should forceMount the content when `forceMount` is true and the `open` snippet prop is used to conditionally render the content", async () => { - const { queryByTestId, getByTestId, user, trigger } = setup( - { - withOpenCheck: true, - }, - ContextMenuForceMountTest - ); + const { queryByTestId, getByTestId, user, trigger } = setup({ + withOpenCheck: true, + component: ContextMenuForceMountTest, + }); expect(queryByTestId("content")).toBeNull(); await user.pointer([{ target: trigger }, { keys: "[MouseRight]", target: trigger }]); @@ -330,4 +329,32 @@ describe("context menu", () => { const content = getByTestId("content"); expect(content).toBeVisible(); }); + + it.each([ContextMenuTest, ContextMenuForceMountTest])( + "should close the menu and focus the next tabbable element when `TAB` is pressed while the menu is open", + async (component) => { + const { user, getByTestId, queryByTestId } = await open({ + component, + withOpenCheck: true, + }); + const nextButton = getByTestId("next-button"); + await user.keyboard(kbd.TAB); + await waitFor(() => expect(nextButton).toHaveFocus()); + expect(queryByTestId("content")).toBeNull(); + } + ); + + it.each([ContextMenuTest, ContextMenuForceMountTest])( + "should close the menu and focus the previous tabbable element when `SHIFT+TAB` is pressed while the menu is open", + async (component) => { + const { user, getByTestId, queryByTestId } = await open({ + component, + withOpenCheck: true, + }); + const previousButton = getByTestId("previous-button"); + await user.keyboard(kbd.SHIFT_TAB); + await waitFor(() => expect(previousButton).toHaveFocus()); + expect(queryByTestId("content")).toBeNull(); + } + ); }); diff --git a/packages/tests/src/tests/dropdown-menu/dropdown-menu-force-mount-test.svelte b/packages/tests/src/tests/dropdown-menu/dropdown-menu-force-mount-test.svelte index 46ea98c58..97e413c54 100644 --- a/packages/tests/src/tests/dropdown-menu/dropdown-menu-force-mount-test.svelte +++ b/packages/tests/src/tests/dropdown-menu/dropdown-menu-force-mount-test.svelte @@ -106,6 +106,7 @@
outside
+
open @@ -128,6 +129,7 @@
+ diff --git a/packages/tests/src/tests/dropdown-menu/dropdown-menu-test.svelte b/packages/tests/src/tests/dropdown-menu/dropdown-menu-test.svelte index 7d6ebb73c..5552b2559 100644 --- a/packages/tests/src/tests/dropdown-menu/dropdown-menu-test.svelte +++ b/packages/tests/src/tests/dropdown-menu/dropdown-menu-test.svelte @@ -28,6 +28,7 @@
outside
+
open @@ -97,6 +98,7 @@
+ diff --git a/packages/tests/src/tests/dropdown-menu/dropdown-menu.test.ts b/packages/tests/src/tests/dropdown-menu/dropdown-menu.test.ts index 3fd271fa1..ebb28742b 100644 --- a/packages/tests/src/tests/dropdown-menu/dropdown-menu.test.ts +++ b/packages/tests/src/tests/dropdown-menu/dropdown-menu.test.ts @@ -1,7 +1,7 @@ import { render, screen, waitFor } from "@testing-library/svelte/svelte5"; import { axe } from "jest-axe"; import { describe, it } from "vitest"; -import { type Component, tick } from "svelte"; +import { tick } from "svelte"; import { getTestKbd, setupUserEvents, sleep } from "../utils.js"; import DropdownMenuTest from "./dropdown-menu-test.svelte"; import type { DropdownMenuTestProps } from "./dropdown-menu-test.svelte"; @@ -11,15 +11,17 @@ import DropdownMenuForceMountTest from "./dropdown-menu-force-mount-test.svelte" const kbd = getTestKbd(); const OPEN_KEYS = [kbd.ENTER, kbd.ARROW_DOWN, kbd.SPACE]; +type DropdownMenuSetupProps = (DropdownMenuTestProps | DropdownMenuForceMountTestProps) & { + component?: typeof DropdownMenuTest | typeof DropdownMenuForceMountTest; +}; + /** * Helper function to reduce boilerplate in tests */ -function setup( - props: DropdownMenuTestProps | DropdownMenuForceMountTestProps = {}, - component: Component = DropdownMenuTest -) { +function setup(props: DropdownMenuSetupProps = {}) { + const { component: comp = DropdownMenuTest, ...rest } = props; const user = setupUserEvents(); - const { getByTestId, queryByTestId } = render(component, { ...props }); + const { getByTestId, queryByTestId } = render(comp, { ...rest }); const trigger = getByTestId("trigger"); return { getByTestId, @@ -29,7 +31,7 @@ function setup( }; } -async function openWithPointer(props: DropdownMenuTestProps = {}) { +async function openWithPointer(props: DropdownMenuSetupProps = {}) { const { getByTestId, queryByTestId, user, trigger } = setup(props); const content = queryByTestId("content"); expect(content).toBeNull(); @@ -39,7 +41,7 @@ async function openWithPointer(props: DropdownMenuTestProps = {}) { return { getByTestId, queryByTestId, user, trigger }; } -async function openWithKbd(props: DropdownMenuTestProps = {}, key: string = kbd.ENTER) { +async function openWithKbd(props: DropdownMenuSetupProps = {}, key: string = kbd.ENTER) { const { getByTestId, queryByTestId, user, trigger } = setup(props); const content = queryByTestId("content"); expect(content).toBeNull(); @@ -342,18 +344,16 @@ describe("dropdown menu", () => { }); it("should forceMount the content when `forceMount` is true", async () => { - const { getByTestId } = setup({}, DropdownMenuForceMountTest); + const { getByTestId } = setup({ component: DropdownMenuForceMountTest }); expect(getByTestId("content")).toBeVisible(); }); it("should forceMount the content when `forceMount` is true and the `open` snippet prop is used to conditionally render the content", async () => { - const { queryByTestId, getByTestId, user, trigger } = setup( - { - withOpenCheck: true, - }, - DropdownMenuForceMountTest - ); + const { queryByTestId, getByTestId, user, trigger } = setup({ + withOpenCheck: true, + component: DropdownMenuForceMountTest, + }); expect(queryByTestId("content")).toBeNull(); await user.click(trigger); @@ -361,4 +361,35 @@ describe("dropdown menu", () => { const content = getByTestId("content"); expect(content).toBeVisible(); }); + + it.each([DropdownMenuTest, DropdownMenuForceMountTest])( + "should close the menu and focus the next tabbable element when `TAB` is pressed while the menu is open", + async (component) => { + const { trigger, user, getByTestId, queryByTestId } = await openWithKbd({ + component, + withOpenCheck: true, + }); + const nextButton = getByTestId("next-button"); + await user.keyboard(kbd.TAB); + await waitFor(() => expect(nextButton).toHaveFocus()); + + expect(queryByTestId("content")).toBeNull(); + await user.click(trigger); + await waitFor(() => expect(queryByTestId("content")).not.toBeNull()); + } + ); + + it.each([DropdownMenuTest, DropdownMenuForceMountTest])( + "should close the menu and focus the previous tabbable element when `SHIFT+TAB` is pressed while the menu is open", + async (component) => { + const { user, getByTestId, queryByTestId } = await openWithKbd({ + component, + withOpenCheck: true, + }); + const previousButton = getByTestId("previous-button"); + await user.keyboard(kbd.SHIFT_TAB); + await waitFor(() => expect(previousButton).toHaveFocus()); + expect(queryByTestId("content")).toBeNull(); + } + ); }); diff --git a/packages/tests/src/tests/menubar/menubar-test.svelte b/packages/tests/src/tests/menubar/menubar-test.svelte index 511d7a364..44fde5101 100644 --- a/packages/tests/src/tests/menubar/menubar-test.svelte +++ b/packages/tests/src/tests/menubar/menubar-test.svelte @@ -6,10 +6,12 @@
+ +
diff --git a/packages/tests/src/tests/menubar/menubar.test.ts b/packages/tests/src/tests/menubar/menubar.test.ts index a63d23ab5..1fd0c296d 100644 --- a/packages/tests/src/tests/menubar/menubar.test.ts +++ b/packages/tests/src/tests/menubar/menubar.test.ts @@ -1,8 +1,7 @@ -import { render, screen, waitFor } from "@testing-library/svelte/svelte5"; +import { render, waitFor } from "@testing-library/svelte/svelte5"; import { userEvent } from "@testing-library/user-event"; import { axe } from "jest-axe"; import { describe, it } from "vitest"; -import { tick } from "svelte"; import type { Menubar } from "bits-ui"; import { getTestKbd } from "../utils.js"; import MenubarTest from "./menubar-test.svelte"; @@ -31,45 +30,6 @@ describe("menubar", () => { expect(await axe(container)).toHaveNoViolations(); }); - it.skip("should have bits data attrs", async () => { - const menuId = "1"; - const { user, getTrigger, getByTestId, queryByTestId } = setup(); - const trigger = getTrigger(menuId); - await user.click(trigger); - await user.click(trigger); - const content = queryByTestId("1-content"); - await tick(); - await waitFor(() => expect(content).not.toBeNull()); - - const root = getByTestId("root"); - expect(root).toHaveAttribute("data-menubar-root"); - - const parts = [ - "content", - "trigger", - "group", - "group-heading", - "separator", - "sub-trigger", - "item", - "checkbox-item", - "radio-group", - "radio-item", - "checkbox-indicator", - ]; - const mappedParts = parts.map((part) => `${menuId}-${part}`); - - for (const part in parts) { - const el = screen.getByTestId(mappedParts[part] as string); - expect(el).toHaveAttribute(`data-menu-${parts[part]}`); - } - - await user.click(getByTestId("1-sub-trigger")); - - const subContent = getByTestId("1-sub-content"); - expect(subContent).toHaveAttribute(`data-menu-sub-content`); - }); - it("should navigate triggers within the menubar using arrow keys", async () => { const { user, getTrigger } = setup(); const trigger = getTrigger("1"); @@ -84,7 +44,7 @@ describe("menubar", () => { expect(getTrigger("1")).toHaveFocus(); }); - it("should respect the loop prop", async () => { + it("should respect `loop: false`", async () => { const { user, getTrigger } = setup({ loop: false }); const trigger = getTrigger("1"); trigger.focus(); @@ -113,4 +73,29 @@ describe("menubar", () => { expect(getContent("1")).toBeVisible(); expect(content2).not.toBeVisible(); }); + + it("should close the menu and focus the next tabbable element when `TAB` is pressed while the menu is open", async () => { + const { user, getTrigger, getContent, getByTestId } = setup(); + const trigger = getTrigger("1"); + trigger.focus(); + await user.keyboard(kbd.ARROW_DOWN); + const content1 = getContent("1"); + await waitFor(() => expect(content1).toBeVisible()); + + const nextButton = getByTestId("next-button"); + await user.keyboard(kbd.TAB); + await waitFor(() => expect(nextButton).toHaveFocus()); + }); + + it("should close the menu and focus the previous tabbable element when `SHIFT+TAB` is pressed while the menu is open", async () => { + const { user, getTrigger, getContent, getByTestId } = setup(); + const trigger = getTrigger("1"); + trigger.focus(); + await user.keyboard(kbd.ARROW_DOWN); + const content1 = getContent("1"); + expect(content1).toBeVisible(); + const previousButton = getByTestId("previous-button"); + await user.keyboard(kbd.SHIFT_TAB); + await waitFor(() => expect(previousButton).toHaveFocus()); + }); }); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 922a701b9..ab604dd9a 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -83,6 +83,9 @@ importers: svelte-toolbelt: specifier: ^0.7.1 version: 0.7.1(svelte@5.19.4) + tabbable: + specifier: ^6.2.0 + version: 6.2.0 devDependencies: '@sveltejs/kit': specifier: ^2.16.1 diff --git a/sites/docs/src/lib/components/demos/menubar-demo.svelte b/sites/docs/src/lib/components/demos/menubar-demo.svelte index 385f9fee5..38bc6408f 100644 --- a/sites/docs/src/lib/components/demos/menubar-demo.svelte +++ b/sites/docs/src/lib/components/demos/menubar-demo.svelte @@ -69,6 +69,7 @@ File @@ -120,6 +121,7 @@ Edit @@ -197,6 +199,7 @@ View @@ -252,6 +255,7 @@ Profiles