diff --git a/.changeset/gold-paws-juggle.md b/.changeset/gold-paws-juggle.md new file mode 100644 index 000000000..bcbe0cb6f --- /dev/null +++ b/.changeset/gold-paws-juggle.md @@ -0,0 +1,5 @@ +--- +"bits-ui": patch +--- + +fix: issue where you were unable to navigate to the previous menu from within a menu of the menubar via arrow keys 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 c4b3bc7fc..430da9247 100644 --- a/packages/bits-ui/src/lib/bits/menubar/menubar.svelte.ts +++ b/packages/bits-ui/src/lib/bits/menubar/menubar.svelte.ts @@ -62,6 +62,7 @@ class MenubarRootState { id: this.id, ref: this.ref, }); + this.rovingFocusGroup = useRovingFocus({ rootNodeId: this.id, candidateAttr: TRIGGER_ATTR, @@ -386,6 +387,7 @@ class MenubarContentState { "--bits-menubar-anchor-height": "var(--bits-floating-anchor-height)", }, onkeydown: this.onkeydown, + "data-menu-content": "", }) as const ); } diff --git a/packages/tests/src/tests/menubar/menubar.test.ts b/packages/tests/src/tests/menubar/menubar.test.ts index cba79e6b1..a63d23ab5 100644 --- a/packages/tests/src/tests/menubar/menubar.test.ts +++ b/packages/tests/src/tests/menubar/menubar.test.ts @@ -12,12 +12,17 @@ const kbd = getTestKbd(); /** * Helper function to reduce boilerplate in tests */ -function setup(props: Menubar.RootProps = {}, menuId: string = "1") { +function setup(props: Menubar.RootProps = {}) { const user = userEvent.setup(); const returned = render(MenubarTest, { ...props }); const { getByTestId } = returned; - const trigger = getByTestId(`${menuId}-trigger`); - return { user, ...returned, trigger }; + + 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`); + + return { user, ...returned, getTrigger, getContent, getSubTrigger, getSubContent }; } describe("menubar", () => { @@ -28,7 +33,8 @@ describe("menubar", () => { it.skip("should have bits data attrs", async () => { const menuId = "1"; - const { user, trigger, getByTestId, queryByTestId } = setup({}, menuId); + const { user, getTrigger, getByTestId, queryByTestId } = setup(); + const trigger = getTrigger(menuId); await user.click(trigger); await user.click(trigger); const content = queryByTestId("1-content"); @@ -65,28 +71,46 @@ describe("menubar", () => { }); it("should navigate triggers within the menubar using arrow keys", async () => { - const { user, trigger, getByTestId } = setup({}, "1"); + const { user, getTrigger } = setup(); + const trigger = getTrigger("1"); trigger.focus(); await user.keyboard(kbd.ARROW_RIGHT); - expect(getByTestId("2-trigger")).toHaveFocus(); + expect(getTrigger("2")).toHaveFocus(); await user.keyboard(kbd.ARROW_RIGHT); - expect(getByTestId("3-trigger")).toHaveFocus(); + expect(getTrigger("3")).toHaveFocus(); await user.keyboard(kbd.ARROW_RIGHT); - expect(getByTestId("4-trigger")).toHaveFocus(); + expect(getTrigger("4")).toHaveFocus(); await user.keyboard(kbd.ARROW_RIGHT); - expect(getByTestId("1-trigger")).toHaveFocus(); + expect(getTrigger("1")).toHaveFocus(); }); it("should respect the loop prop", async () => { - const { user, trigger, getByTestId } = setup({ loop: false }, "1"); + const { user, getTrigger } = setup({ loop: false }); + const trigger = getTrigger("1"); trigger.focus(); await user.keyboard(kbd.ARROW_RIGHT); - expect(getByTestId("2-trigger")).toHaveFocus(); + expect(getTrigger("2")).toHaveFocus(); + await user.keyboard(kbd.ARROW_RIGHT); + expect(getTrigger("3")).toHaveFocus(); await user.keyboard(kbd.ARROW_RIGHT); - expect(getByTestId("3-trigger")).toHaveFocus(); + expect(getTrigger("4")).toHaveFocus(); await user.keyboard(kbd.ARROW_RIGHT); - expect(getByTestId("4-trigger")).toHaveFocus(); + expect(getTrigger("4")).toHaveFocus(); + }); + + it("should navigate between menus when using the arrow keys and focus is within a menu", async () => { + const { user, getTrigger, getContent } = setup(); + const trigger = getTrigger("1"); + trigger.focus(); + await user.keyboard(kbd.ARROW_DOWN); + const content1 = getContent("1"); + expect(content1).toBeVisible(); await user.keyboard(kbd.ARROW_RIGHT); - expect(getByTestId("4-trigger")).toHaveFocus(); + const content2 = getContent("2"); + expect(content2).toBeVisible(); + expect(content1).not.toBeVisible(); + await user.keyboard(kbd.ARROW_LEFT); + expect(getContent("1")).toBeVisible(); + expect(content2).not.toBeVisible(); }); });