From a4a3ec621137c6001a354d3ad55aec3da6adba46 Mon Sep 17 00:00:00 2001 From: Hunter Johnston <64506580+huntabyte@users.noreply.github.com> Date: Fri, 27 Sep 2024 14:56:59 -0400 Subject: [PATCH] next: fix nav menu focus (#681) --- .../lib/bits/accordion/accordion.svelte.ts | 2 +- .../bits-ui/src/lib/bits/menu/menu.svelte.ts | 2 +- .../src/lib/bits/menubar/menubar.svelte.ts | 2 +- .../components/navigation-menu-item.svelte | 6 +- .../components/navigation-menu-link.svelte | 6 +- .../navigation-menu/navigation-menu.svelte.ts | 140 ++++++++++++------ .../bits/radio-group/radio-group.svelte.ts | 2 +- .../bits-ui/src/lib/bits/tabs/tabs.svelte.ts | 2 +- .../bits/toggle-group/toggle-group.svelte.ts | 2 +- .../src/lib/bits/toolbar/toolbar.svelte.ts | 2 +- .../src/lib/internal/useRovingFocus.svelte.ts | 23 ++- 11 files changed, 134 insertions(+), 55 deletions(-) diff --git a/packages/bits-ui/src/lib/bits/accordion/accordion.svelte.ts b/packages/bits-ui/src/lib/bits/accordion/accordion.svelte.ts index 99effd4b9..229678fdd 100644 --- a/packages/bits-ui/src/lib/bits/accordion/accordion.svelte.ts +++ b/packages/bits-ui/src/lib/bits/accordion/accordion.svelte.ts @@ -54,7 +54,7 @@ class AccordionBaseState { this.#loop = props.loop; this.rovingFocusGroup = useRovingFocus({ rootNodeId: this.#id, - candidateSelector: ACCORDION_TRIGGER_ATTR, + candidateAttr: ACCORDION_TRIGGER_ATTR, loop: this.#loop, orientation: this.orientation, }); 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 7b8e48ce0..50d6130d4 100644 --- a/packages/bits-ui/src/lib/bits/menu/menu.svelte.ts +++ b/packages/bits-ui/src/lib/bits/menu/menu.svelte.ts @@ -266,7 +266,7 @@ class MenuContentState { this.#handleTypeaheadSearch = useTypeahead().handleTypeaheadSearch; this.rovingFocusGroup = useRovingFocus({ rootNodeId: this.parentMenu.contentId, - candidateSelector: this.parentMenu.root.attrs.item, + candidateAttr: this.parentMenu.root.attrs.item, loop: this.#loop, orientation: box.with(() => "vertical"), }); 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 db76734bb..3e11d8ce6 100644 --- a/packages/bits-ui/src/lib/bits/menubar/menubar.svelte.ts +++ b/packages/bits-ui/src/lib/bits/menubar/menubar.svelte.ts @@ -51,7 +51,7 @@ class MenubarRootState { }); this.rovingFocusGroup = useRovingFocus({ rootNodeId: this.id, - candidateSelector: TRIGGER_ATTR, + candidateAttr: TRIGGER_ATTR, loop: this.loop, orientation: box.with(() => "horizontal"), currentTabStopId: this.currentTabStopId, diff --git a/packages/bits-ui/src/lib/bits/navigation-menu/components/navigation-menu-item.svelte b/packages/bits-ui/src/lib/bits/navigation-menu/components/navigation-menu-item.svelte index 09c8f6643..ea5257c44 100644 --- a/packages/bits-ui/src/lib/bits/navigation-menu/components/navigation-menu-item.svelte +++ b/packages/bits-ui/src/lib/bits/navigation-menu/components/navigation-menu-item.svelte @@ -8,14 +8,18 @@ let { id = useId(), value = useId(), + ref = $bindable(null), child, children, - ref = $bindable(), ...restProps }: ItemProps = $props(); const itemState = useNavigationMenuItem({ id: box.with(() => id), + ref: box.with( + () => ref, + (v) => (ref = v) + ), value: box.with(() => value), }); diff --git a/packages/bits-ui/src/lib/bits/navigation-menu/components/navigation-menu-link.svelte b/packages/bits-ui/src/lib/bits/navigation-menu/components/navigation-menu-link.svelte index 195e4ef93..8647ca474 100644 --- a/packages/bits-ui/src/lib/bits/navigation-menu/components/navigation-menu-link.svelte +++ b/packages/bits-ui/src/lib/bits/navigation-menu/components/navigation-menu-link.svelte @@ -8,7 +8,7 @@ let { id = useId(), - ref = $bindable(), + ref = $bindable(null), child, children, active = false, @@ -18,6 +18,10 @@ const linkState = useNavigationMenuLink({ id: box.with(() => id), + ref: box.with( + () => ref, + (v) => (ref = v) + ), active: box.with(() => active), onSelect: box.with(() => onSelect), }); diff --git a/packages/bits-ui/src/lib/bits/navigation-menu/navigation-menu.svelte.ts b/packages/bits-ui/src/lib/bits/navigation-menu/navigation-menu.svelte.ts index 1ad0b416b..9b0b8c0d4 100644 --- a/packages/bits-ui/src/lib/bits/navigation-menu/navigation-menu.svelte.ts +++ b/packages/bits-ui/src/lib/bits/navigation-menu/navigation-menu.svelte.ts @@ -18,9 +18,10 @@ import { kbd } from "$lib/internal/kbd.js"; import { useArrowNavigation } from "$lib/internal/useArrowNavigation.js"; import { boxAutoReset } from "$lib/internal/boxAutoReset.svelte.js"; import { useRefById } from "$lib/internal/useRefById.svelte.js"; -import type { ElementRef } from "$lib/internal/types.js"; +import type { ElementRef, WithRefProps } from "$lib/internal/types.js"; import { afterTick } from "$lib/internal/afterTick.js"; import { noop } from "$lib/internal/callbacks.js"; +import { useRovingFocus } from "$lib/internal/useRovingFocus.svelte.js"; const [setNavigationMenuRootContext] = createContext("NavigationMenu.Root"); @@ -29,9 +30,15 @@ const [setNavigationMenuMenuContext, getNavigationMenuMenuContext] = createConte NavigationMenuMenuState | NavigationMenuSubState >("NavigationMenu.Root or NavigationMenu.Sub"); +const [setNavigationMenuListContext, getNavigationMenuListContext] = + createContext("NavigationMenu.List"); + const [setNavigationMenuItemContext, getNavigationMenuItemContext] = createContext("NavigationMenu.Item"); +const [setNavigationMenuContentContext, getNavigationMenuContentContext] = + createContext("NavigationMenu.Content"); + const ROOT_ATTR = "data-navigation-menu-root"; const SUB_ATTR = "data-navigation-menu-sub"; const ITEM_ATTR = "data-navigation-menu-item"; @@ -255,10 +262,6 @@ class NavigationMenuMenuState { return new NavigationMenuListState(props, this); } - createItem(props: NavigationMenuItemStateProps) { - return new NavigationMenuItemState(props, this); - } - createIndicator(props: NavigationMenuIndicatorStateProps) { return new NavigationMenuIndicatorState(props, this); } @@ -344,10 +347,6 @@ class NavigationMenuSubState { return new NavigationMenuListState(props, this); } - createItem(props: NavigationMenuItemStateProps) { - return new NavigationMenuItemState(props, this); - } - createIndicator(props: NavigationMenuIndicatorStateProps) { return new NavigationMenuIndicatorState(props, this); } @@ -372,22 +371,30 @@ type NavigationMenuListStateProps = ReadableBoxedValues<{ }>; class NavigationMenuListState { - id: NavigationMenuListStateProps["id"]; - listRef: NavigationMenuListStateProps["ref"]; + #id: NavigationMenuListStateProps["id"]; + #ref: NavigationMenuListStateProps["ref"]; indicatorTrackRef: NavigationMenuListStateProps["indicatorTrackRef"]; indicatorTrackId = box(useId()); + rovingFocusGroup: ReturnType; constructor( props: NavigationMenuListStateProps, private menu: NavigationMenuMenuState | NavigationMenuSubState ) { - this.id = props.id; - this.listRef = props.ref; + this.#id = props.id; + this.#ref = props.ref; this.indicatorTrackRef = props.indicatorTrackRef; + this.rovingFocusGroup = useRovingFocus({ + rootNodeId: this.#id, + candidateAttr: TRIGGER_ATTR, + candidateSelector: `:is([${TRIGGER_ATTR}], [data-list-link]):not([data-disabled])`, + loop: box.with(() => false), + orientation: this.menu.orientation, + }); useRefById({ - id: this.id, - ref: this.listRef, + id: this.#id, + ref: this.#ref, }); useRefById({ @@ -396,6 +403,7 @@ class NavigationMenuListState { onRefChange: (node) => { this.menu.indicatorTrackNode = node; }, + condition: () => Boolean(this.menu.root.value.current), }); } @@ -409,22 +417,29 @@ class NavigationMenuListState { }) as const ); + createItem(props: NavigationMenuItemStateProps) { + return new NavigationMenuItemState(props, this, this.menu); + } + props = $derived.by( () => ({ + id: this.#id.current, "data-orientation": getDataOrientation(this.menu.orientation.current), [LIST_ATTR]: "", }) as const ); } -type NavigationMenuItemStateProps = ReadableBoxedValues<{ - id: string; - value: string; -}>; +type NavigationMenuItemStateProps = WithRefProps< + ReadableBoxedValues<{ + value: string; + }> +>; class NavigationMenuItemState { id: NavigationMenuItemStateProps["id"]; + #ref: NavigationMenuItemStateProps["ref"]; value: NavigationMenuItemStateProps["value"]; contentNode = $state(null); triggerNode = $state(null); @@ -434,14 +449,23 @@ class NavigationMenuItemState { restoreContentTabOrder = noop; wasEscapeClose = $state(false); menu: NavigationMenuMenuState | NavigationMenuSubState; + list: NavigationMenuListState; constructor( props: NavigationMenuItemStateProps, + list: NavigationMenuListState, menu: NavigationMenuMenuState | NavigationMenuSubState ) { this.id = props.id; + this.#ref = props.ref; this.value = props.value; this.menu = menu; + this.list = list; + + useRefById({ + id: this.id, + ref: this.#ref, + }); } #handleContentEntry = (side: "start" | "end" = "start") => { @@ -487,7 +511,7 @@ class NavigationMenuItemState { } createLink(props: NavigationMenuLinkStateProps) { - return new NavigationMenuLinkState(props); + return new NavigationMenuLinkState(props, this); } } @@ -501,7 +525,8 @@ type NavigationMenuTriggerStateProps = ReadableBoxedValues<{ }>; class NavigationMenuTriggerState { - id: NavigationMenuTriggerStateProps["id"]; + #id: NavigationMenuTriggerStateProps["id"]; + #ref: NavigationMenuTriggerStateProps["ref"]; focusProxyMounted: NavigationMenuTriggerStateProps["focusProxyMounted"]; menu: NavigationMenuMenuState | NavigationMenuSubState; item: NavigationMenuItemState; @@ -509,19 +534,18 @@ class NavigationMenuTriggerState { hasPointerMoveOpened = boxAutoReset(false, 150); wasClickClose = $state(false); open = $derived.by(() => this.item.value.current === this.menu.value.current); - triggerRef: NavigationMenuTriggerStateProps["ref"]; constructor(props: NavigationMenuTriggerStateProps, item: NavigationMenuItemState) { - this.id = props.id; - this.triggerRef = props.ref; + this.#id = props.id; + this.#ref = props.ref; this.item = item; this.menu = item.menu; this.disabled = props.disabled; this.focusProxyMounted = props.focusProxyMounted; useRefById({ - id: this.id, - ref: this.triggerRef, + id: this.#id, + ref: this.#ref, onRefChange: (node) => { this.item.triggerNode = node; }, @@ -537,9 +561,9 @@ class NavigationMenuTriggerState { }); $effect(() => { - this.menu.registerTrigger(this.triggerRef); + this.menu.registerTrigger(this.#ref); return () => { - this.menu.deRegisterTrigger(this.triggerRef); + this.menu.deRegisterTrigger(this.#ref); }; }); } @@ -585,16 +609,19 @@ class NavigationMenuTriggerState { horizontal: kbd.ARROW_DOWN, vertical: verticalEntryKey, }[this.menu.orientation.current]; + if (this.open && e.key === entryKey) { this.item.onEntryKeydown(); e.preventDefault(); + return; } + this.item.list.rovingFocusGroup.handleKeydown(this.#ref.current, e); }; props = $derived.by( () => ({ - id: this.id.current, + id: this.#id.current, disabled: getDisabled(this.disabled.current), "data-disabled": getDataDisabled(this.disabled.current), "data-state": getDataOpenClosed(this.open), @@ -630,21 +657,36 @@ class NavigationMenuTriggerState { ); } -type NavigationMenuLinkStateProps = ReadableBoxedValues<{ - id: string; - active: boolean; - onSelect: (e: Event) => void; -}>; +type NavigationMenuLinkStateProps = WithRefProps & + ReadableBoxedValues<{ + active: boolean; + onSelect: (e: Event) => void; + }>; class NavigationMenuLinkState { - id: NavigationMenuItemState["id"]; + #id: NavigationMenuItemState["id"]; + #ref: NavigationMenuLinkStateProps["ref"]; active: NavigationMenuLinkStateProps["active"]; onSelect: NavigationMenuLinkStateProps["onSelect"]; + content?: NavigationMenuContentState; + item: NavigationMenuItemState; - constructor(props: NavigationMenuLinkStateProps) { - this.id = props.id; + constructor( + props: NavigationMenuLinkStateProps, + item: NavigationMenuItemState, + content?: NavigationMenuContentState + ) { + this.#id = props.id; + this.#ref = props.ref; this.active = props.active; this.onSelect = props.onSelect; + this.content = content; + this.item = item; + + useRefById({ + id: this.#id, + ref: this.#ref, + }); } #onclick = (e: MouseEvent) => { @@ -660,14 +702,20 @@ class NavigationMenuLinkState { } }; + #onkeydown = (e: KeyboardEvent) => { + this.item.list.rovingFocusGroup.handleKeydown(this.#ref.current, e); + }; + props = $derived.by( () => ({ - id: this.id.current, + id: this.#id.current, "data-active": this.active.current ? "" : undefined, "aria-current": this.active.current ? "page" : undefined, + "data-list-link": this.content ? undefined : "", onclick: this.#onclick, onfocus: (_: FocusEvent) => {}, + onkeydown: this.content ? undefined : this.#onkeydown, }) as const ); } @@ -915,6 +963,10 @@ class NavigationMenuContentState { newSelectedElement?.focus(); }; + createLink(props: NavigationMenuLinkStateProps) { + return new NavigationMenuLinkState(props, this.item, this); + } + props = $derived.by( () => ({ @@ -1055,12 +1107,12 @@ export function useNavigationMenuSub(props: NavigationMenuSubStateProps) { export function useNavigationMenuList(props: NavigationMenuListStateProps) { const menuState = getNavigationMenuMenuContext(); - return menuState.createList(props); + return setNavigationMenuListContext(menuState.createList(props)); } export function useNavigationMenuItem(props: NavigationMenuItemStateProps) { - const menuState = getNavigationMenuMenuContext(); - return setNavigationMenuItemContext(menuState.createItem(props)); + const listState = getNavigationMenuListContext(); + return setNavigationMenuItemContext(listState.createItem(props)); } export function useNavigationMenuTrigger(props: NavigationMenuTriggerStateProps) { @@ -1068,7 +1120,7 @@ export function useNavigationMenuTrigger(props: NavigationMenuTriggerStateProps) } export function useNavigationMenuContent(props: NavigationMenuContentStateProps) { - return getNavigationMenuItemContext().createContent(props); + return setNavigationMenuContentContext(getNavigationMenuItemContext().createContent(props)); } export function useNavigationMenuViewport(props: NavigationMenuViewportStateProps) { @@ -1080,6 +1132,10 @@ export function useNavigationMenuIndicator(props: NavigationMenuIndicatorStatePr } export function useNavigationMenuLink(props: NavigationMenuLinkStateProps) { + const content = getNavigationMenuContentContext(null); + if (content) { + return content.createLink(props); + } return getNavigationMenuItemContext().createLink(props); } diff --git a/packages/bits-ui/src/lib/bits/radio-group/radio-group.svelte.ts b/packages/bits-ui/src/lib/bits/radio-group/radio-group.svelte.ts index 586fa20c0..743de2983 100644 --- a/packages/bits-ui/src/lib/bits/radio-group/radio-group.svelte.ts +++ b/packages/bits-ui/src/lib/bits/radio-group/radio-group.svelte.ts @@ -42,7 +42,7 @@ class RadioGroupRootState { this.#ref = props.ref; this.rovingFocusGroup = useRovingFocus({ rootNodeId: this.#id, - candidateSelector: RADIO_GROUP_ITEM_ATTR, + candidateAttr: RADIO_GROUP_ITEM_ATTR, loop: this.loop, orientation: this.orientation, }); 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 695dc3f54..7d4c15bb4 100644 --- a/packages/bits-ui/src/lib/bits/tabs/tabs.svelte.ts +++ b/packages/bits-ui/src/lib/bits/tabs/tabs.svelte.ts @@ -58,7 +58,7 @@ class TabsRootState { }); this.rovingFocusGroup = useRovingFocus({ - candidateSelector: TRIGGER_ATTR, + candidateAttr: TRIGGER_ATTR, rootNodeId: this.#id, loop: this.loop, orientation: this.orientation, diff --git a/packages/bits-ui/src/lib/bits/toggle-group/toggle-group.svelte.ts b/packages/bits-ui/src/lib/bits/toggle-group/toggle-group.svelte.ts index cf5de2a04..5263c8781 100644 --- a/packages/bits-ui/src/lib/bits/toggle-group/toggle-group.svelte.ts +++ b/packages/bits-ui/src/lib/bits/toggle-group/toggle-group.svelte.ts @@ -43,7 +43,7 @@ class ToggleGroupBaseState { this.loop = props.loop; this.orientation = props.orientation; this.rovingFocusGroup = useRovingFocus({ - candidateSelector: ITEM_ATTR, + candidateAttr: ITEM_ATTR, rootNodeId: this.id, loop: this.loop, orientation: this.orientation, diff --git a/packages/bits-ui/src/lib/bits/toolbar/toolbar.svelte.ts b/packages/bits-ui/src/lib/bits/toolbar/toolbar.svelte.ts index 5ea67c478..f7c5d8ee9 100644 --- a/packages/bits-ui/src/lib/bits/toolbar/toolbar.svelte.ts +++ b/packages/bits-ui/src/lib/bits/toolbar/toolbar.svelte.ts @@ -51,7 +51,7 @@ class ToolbarRootState { orientation: this.orientation, loop: this.#loop, rootNodeId: this.#id, - candidateSelector: ITEM_ATTR, + candidateAttr: ITEM_ATTR, }); } diff --git a/packages/bits-ui/src/lib/internal/useRovingFocus.svelte.ts b/packages/bits-ui/src/lib/internal/useRovingFocus.svelte.ts index 1c4c63926..c818032cf 100644 --- a/packages/bits-ui/src/lib/internal/useRovingFocus.svelte.ts +++ b/packages/bits-ui/src/lib/internal/useRovingFocus.svelte.ts @@ -9,7 +9,13 @@ type UseRovingFocusProps = { /** * The selector used to find the focusable candidates. */ - candidateSelector: string; + candidateAttr: string; + + /** + * Custom candidate selector + */ + candidateSelector?: string; + /** * The id of the root node */ @@ -48,9 +54,18 @@ export function useRovingFocus(props: UseRovingFocusProps) { if (!isBrowser) return []; const node = document.getElementById(props.rootNodeId.current); if (!node) return []; - return Array.from( - node.querySelectorAll(`[${props.candidateSelector}]:not([data-disabled])`) - ); + + if (props.candidateSelector) { + const candidates = Array.from( + node.querySelectorAll(props.candidateSelector) + ); + return candidates; + } else { + const candidates = Array.from( + node.querySelectorAll(`[${props.candidateAttr}]:not([data-disabled])`) + ); + return candidates; + } } function focusFirstCandidate() {