fix(Menubar): Add onOpenChange to <Menubar.Menu/> and fix docs (#1324)

Co-authored-by: Aidan Bleser <[email protected]>
Co-authored-by: Aidan Bleser <[email protected]>
Co-authored-by: Hunter Johnston <[email protected]>

Author: 4 people

.changeset/unlucky-trainers-worry.md

@@ -0,0 +1,5 @@
+---
+"bits-ui": patch
+---
+
+fix(Menubar): Add `onOpenChange` prop to `<Menubar.Menu/>`

docs/src/lib/content/api-reference/menubar.api.ts

@@ -19,6 +19,7 @@ import type {
 	MenubarTriggerPropsWithoutHTML,
 } from "bits-ui";
 import {
+	childrenSnippet,
 	createApiSchema,
 	createBooleanProp,
 	createCSSVarSchema,
@@ -28,7 +29,7 @@ import {
 	withChildProps,
 } from "./helpers.js";
 import { menu as m } from "./menu.api.js";
-import { OnStringValueChangeProp } from "./extended-types/shared/index.js";
+import { OnOpenChangeProp, OnStringValueChangeProp } from "./extended-types/shared/index.js";
 import * as C from "$lib/content/constants.js";
 
 export const root = createApiSchema<MenubarRootPropsWithoutHTML>({
@@ -63,7 +64,12 @@ export const menu = createApiSchema<MenubarMenuPropsWithoutHTML>({
 			description:
 				"The value of this menu within the menubar, used to identify it when determining which menu is active.",
 		}),
-		...m.root.props,
+		onOpenChange: createFunctionProp({
+			definition: OnOpenChangeProp,
+			description: "A callback that is fired when the submenu's open state changes.",
+			stringDefinition: "(open: boolean) => void",
+		}),
+		children: childrenSnippet(),
 	},
 });
 

packages/bits-ui/src/lib/bits/menubar/components/menubar-menu.svelte

@@ -4,11 +4,13 @@
 	import { useMenubarMenu } from "../menubar.svelte.js";
 	import Menu from "$lib/bits/menu/components/menu.svelte";
 	import { useId } from "$lib/internal/use-id.js";
+	import { noop } from "$lib/internal/noop.js";
 
-	let { value = useId(), ...restProps }: MenubarMenuProps = $props();
+	let { value = useId(), onOpenChange = noop, ...restProps }: MenubarMenuProps = $props();
 
 	const menuState = useMenubarMenu({
 		value: box.with(() => value),
+		onOpenChange: box.with(() => onOpenChange),
 	});
 </script>
 

packages/bits-ui/src/lib/bits/menubar/menubar.svelte.ts

@@ -14,6 +14,7 @@ import type {
 	BitsFocusEvent,
 	BitsKeyboardEvent,
 	BitsPointerEvent,
+	OnChangeFn,
 	WithRefProps,
 } from "$lib/internal/types.js";
 import {
@@ -39,7 +40,7 @@ class MenubarRootState {
 	rovingFocusGroup: UseRovingFocusReturn;
 	wasOpenedByKeyboard = $state(false);
 	triggerIds = $state<string[]>([]);
-	valueToContentId = new Map<string, ReadableBox<string>>();
+	valueToChangeHandler = new Map<string, ReadableBox<OnChangeFn<boolean>>>();
 
 	constructor(readonly opts: MenubarRootStateProps) {
 		useRefById(opts);
@@ -74,31 +75,44 @@ class MenubarRootState {
 	 * @param contentId - the content id to associate with the value
 	 * @returns - a function to de-register the menu
 	 */
-	registerMenu(value: string, contentId: ReadableBox<string>) {
-		this.valueToContentId.set(value, contentId);
+	registerMenu(value: string, onOpenChange: ReadableBox<OnChangeFn<boolean>>) {
+		this.valueToChangeHandler.set(value, onOpenChange);
 
 		return () => {
-			this.valueToContentId.delete(value);
+			this.valueToChangeHandler.delete(value);
 		};
 	}
 
+	updateValue(value: string) {
+		const currValue = this.opts.value.current;
+		const currHandler = this.valueToChangeHandler.get(currValue)?.current;
+		const nextHandler = this.valueToChangeHandler.get(value)?.current;
+		this.opts.value.current = value;
+		if (currHandler && currValue !== value) {
+			currHandler(false);
+		}
+		if (nextHandler) {
+			nextHandler(true);
+		}
+	}
+
 	getTriggers() {
 		const node = this.opts.ref.current;
 		if (!node) return [];
 		return Array.from(node.querySelectorAll<HTMLButtonElement>(`[${MENUBAR_TRIGGER_ATTR}]`));
 	}
 
 	onMenuOpen(id: string, triggerId: string) {
-		this.opts.value.current = id;
+		this.updateValue(id);
 		this.rovingFocusGroup.setCurrentTabStopId(triggerId);
 	}
 
 	onMenuClose() {
-		this.opts.value.current = "";
+		this.updateValue("");
 	}
 
 	onMenuToggle(id: string) {
-		this.opts.value.current = this.opts.value.current ? "" : id;
+		this.updateValue(this.opts.value.current ? "" : id);
 	}
 
 	props = $derived.by(
@@ -113,6 +127,7 @@ class MenubarRootState {
 
 type MenubarMenuStateProps = ReadableBoxedValues<{
 	value: string;
+	onOpenChange: OnChangeFn<boolean>;
 }>;
 
 class MenubarMenuState {
@@ -135,10 +150,7 @@ class MenubarMenuState {
 		);
 
 		onMount(() => {
-			return this.root.registerMenu(
-				this.opts.value.current,
-				box.with(() => this.contentNode?.id ?? "")
-			);
+			return this.root.registerMenu(this.opts.value.current, opts.onOpenChange);
 		});
 	}
 

packages/bits-ui/src/lib/bits/menubar/types.ts

@@ -39,6 +39,11 @@ export type MenubarMenuPropsWithoutHTML = WithChildren<{
 	 * within the menubar.
 	 */
 	value?: string;
+
+	/**
+	 * A callback that is called when the menu is opened or closed.
+	 */
+	onOpenChange?: OnChangeFn<boolean>;
 }>;
 
 export type MenubarMenuProps = MenubarMenuPropsWithoutHTML;

tests/src/tests/menubar/menubar-test.svelte

@@ -1,17 +1,26 @@
+<script lang="ts" module>
+	export type MenubarTestProps = Menubar.RootProps & {
+		one?: Partial<MenubarMenuProps>;
+		two?: Partial<MenubarMenuProps>;
+		three?: Partial<MenubarMenuProps>;
+		four?: Partial<MenubarMenuProps>;
+	};
+</script>
+
 <script lang="ts">
 	import { Menubar } from "bits-ui";
-	import MenubarMenu from "./menubar-menu-test.svelte";
+	import MenubarMenu, { type MenubarMenuProps } from "./menubar-menu-test.svelte";
 
-	let restProps: Menubar.RootProps = $props();
+	let { one, two, three, four, ...restProps }: MenubarTestProps = $props();
 </script>
 
 <main>
 	<button data-testid="previous-button">previous button</button>
 	<Menubar.Root {...restProps} data-testid="root">
-		<MenubarMenu id="1" />
-		<MenubarMenu id="2" />
-		<MenubarMenu id="3" />
-		<MenubarMenu id="4" />
+		<MenubarMenu id="1" {...one} />
+		<MenubarMenu id="2" {...two} />
+		<MenubarMenu id="3" {...three} />
+		<MenubarMenu id="4" {...four} />
 	</Menubar.Root>
 	<button data-testid="next-button">next button</button>
 </main>

tests/src/tests/menubar/menubar.test.ts

@@ -1,25 +1,24 @@
 import { render, waitFor } from "@testing-library/svelte/svelte5";
 import { userEvent } from "@testing-library/user-event";
 import { axe } from "jest-axe";
-import { it } from "vitest";
-import type { Menubar } from "bits-ui";
+import { it, vi } from "vitest";
 import { getTestKbd } from "../utils.js";
-import MenubarTest from "./menubar-test.svelte";
+import MenubarTest, { type MenubarTestProps } from "./menubar-test.svelte";
 
 const kbd = getTestKbd();
 
 /**
  * Helper function to reduce boilerplate in tests
  */
-function setup(props: Menubar.RootProps = {}) {
+function setup(props: MenubarTestProps = {}) {
 	const user = userEvent.setup();
 	const returned = render(MenubarTest, { ...props });
-	const { getByTestId } = returned;
+	const { getByTestId, queryByTestId } = returned;
 
 	const getTrigger = (id: string) => getByTestId(`${id}-trigger`);
-	const getContent = (id: string) => getByTestId(`${id}-content`);
-	const getSubTrigger = (id: string) => getByTestId(`${id}-sub-trigger`);
-	const getSubContent = (id: string) => getByTestId(`${id}-sub-content`);
+	const getContent = (id: string) => queryByTestId(`${id}-content`);
+	const getSubTrigger = (id: string) => queryByTestId(`${id}-sub-trigger`);
+	const getSubContent = (id: string) => queryByTestId(`${id}-sub-content`);
 
 	return { user, ...returned, getTrigger, getContent, getSubTrigger, getSubContent };
 }
@@ -108,3 +107,33 @@ it("should close the menu and focus the previous tabbable element when `SHIFT+TA
 	await user.keyboard(kbd.SHIFT_TAB);
 	await waitFor(() => expect(previousButton).toHaveFocus());
 });
+
+it("should call the menus `onOpenChange` callback when the menu is opened or closed", async () => {
+	const callbacks = {
+		one: vi.fn(),
+		two: vi.fn(),
+		three: vi.fn(),
+		four: vi.fn(),
+	};
+	const { user, getTrigger, getContent } = setup({
+		one: { onOpenChange: callbacks.one },
+		two: { onOpenChange: callbacks.two },
+		three: { onOpenChange: callbacks.three },
+		four: { onOpenChange: callbacks.four },
+	});
+
+	for (const id of ["1", "2", "3", "4"]) {
+		// @ts-expect-error - sh
+		const callback = callbacks[Object.keys(callbacks)[Number(id) - 1]];
+
+		await user.click(getTrigger(id));
+		await waitFor(() => expect(getContent(id)).toBeVisible());
+		expect(callback).toHaveBeenCalledWith(true);
+		expect(callback).toHaveBeenCalledTimes(1);
+
+		await user.keyboard(kbd.ESCAPE);
+		await waitFor(() => expect(getContent(id)).not.toBeInTheDocument());
+		expect(callback).toHaveBeenCalledWith(false);
+		expect(callback).toHaveBeenCalledTimes(2);
+	}
+});