[Menubar] Add Menubar primitive

[luisorbaiceta]

This PR is intended for the Menubar implementation. Closes #1133

• [x] Discuss API (Following the WAI-ARIA pattern for Menubar)
• [ ] Add integration tests
• [ ] Add stories for visual testing
• [ ] On right or left arrow press, if next focusable element is next tab it should open the tab and focus its trigger as described in WAI-ARIA

Depends on: #1565

Suggested API

<Menubar.Root>
  <Menubar.Menu>
    <Menubar.Trigger />
    <Menubar.Content>
      <Menubar.Item />
    </Menubar.Content>
  </Menubar.Menu>
  <Menubar.Menu>
    <Menubar.Trigger />
    <Menubar.Content>
      <Menubar.Item />
      <Menubar.SubMenu>
        <Menubar.Trigger />
        <Menubar.Content>
          <Menubar.Item />
        </Menubar.Content>
      </Menubar.SubMenu>
    </Menubar.Content>
  </Menubar.Menu>
</Menubar.Root>

For Menus and Submenus the Dropdown implementation will be used. Any additional behaviour will be added via its controlled api

[luisorbaiceta]

There are some test cases that should go in the DropdownMenu and Menu primitives. As I haven't seen test files for those, I'll first put them here till I can find some time to write tests for those components too

[jjenzz]

@luisorbaiceta Just a heads up that we favour chromatic or cypress tests these days over jest. I think the plan was to remove our jest tests eventually because of things like this (it's not a true representation of user's environment) so I wouldn't invest too much time there.

There is already a DropdownMenu cypress test file which covers a bunch of its requirements but feel free to include more there if needed.

[luisorbaiceta]

Good! I'll go with cypress then

[luisorbaiceta]

Right now I'm planning to implement the Right/Left arrow navigation by accessing the top level triggers from the Item element via the DropdownMenuItem controlled API.

There is one extra case that we didn't take into account though. When a top level menu trigger is focused, when hitting ArrowUp, this should open the menu and focus the last item. I wonder if this should be provided by the DropdownMenu too. Any thoughts?

[jjenzz]

I wonder if this should be provided by the DropdownMenu too. Any thoughts?

It does appear to be part of the DropdownMenu pattern here so I think it should be provided there but can be done as a separate PR.

[luisorbaiceta]

I'll send one tonight, if that's ok 😊

[jjenzz]

Just a heads up separately as well, it seems you're going down the route of making the Menubar for site navigation which is not the intention of the pattern we're going with. Ideally we would follow the example linked to in the ticket, so specifically for application menus.

For site navigation we have a HoverMenu component in the works which does not use menubar / menu / menuitem roles as they can hinder the user experience when used for site navigation. For example, most users expect that site navigation can be tabbed through, but menubar pattern requires arrow keys.

[luisorbaiceta]

I'll leave it up to the user then via the asChild prop, and default the Item to the DropdownMenuItem current default. Is that ok?

[jjenzz]

@luisorbaiceta Spot on 👍 thanks! 😍

[luisorbaiceta]

Hi everyone! I've already made a draft for keyboard navigation that could really use some feedback! Thanks in advance! 😊

[jjenzz]

@luisorbaiceta Just an FYI that I haven't forgotten about this. We're in the middle of a big push to try get Toast, Select and HoverMenu out the door so focus is on those at the moment but as soon as we've managed to get a release out i'll circle back onto this 🙏

[luisorbaiceta]

No problem! Good luck with those!! 😉

[luisorbaiceta]

Hi everyone! Now that Toast, Select and HoverMenu have been released, I would love to keep working on this 😊

[benoitgrelard]

Hi @luisorbaiceta,

We should be able to take some time to review your PR soon.

[benoitgrelard]

Hey @luisorbaiceta,

Sorry for the delay with this. We should have some time now to review and get this going. Would you mind rebasing your branch and making sure it is up to date with the API of the primitives you are rewrapping?

Thanks!

✌️

[luisorbaiceta]

I've noticed some changes in the Dropdown Menu components api so I will take some time to adapt everything that has been done so far

[luisorbaiceta]

I think I have reached a point in which I could use a review from the team. There are two upstream issues that affect the behavior of this primitive and prevents it from being fully compliant with the WAI-ARIA spec (#1565 #1564 )

Cheers!

[luisorbaiceta]

Hi everyone! A review would really help to push this forward 🙃

[benoitgrelard]

Hey @luisorbaiceta,

I just wanted to let you know that I have started reviewing this amongst some of the other contributions on the repo.

[Looky1173]

Hi, are there any updates on this pull request or the blocking issue #1308?

[andy-hook]

Hey @luisorbaiceta , I wanted to thank you personally for all of your hard work and effort on this and other changes to the project.

2022 was a super busy year for us and one of the bigger struggles was finding the time to provide feedback and reviews on this and other changes in a timely manner.



We did manage to find a gap in our schedule just before Christmas though and we made this work one our key goals to ship in the new year, your original API proposal and code was incredibly helpful in being able to release yesterday, thank you for the effort and patience.



Closing now as changes implemented in https://github.com/radix-ui/primitives/pull/1846,