feat(menu): add first version

[MathieuPuech]

What I did

1. Add menu implementation example as discussed in [Feature Request] lion/menu component #903

Need to add:

• role attributes
• a href demo
• tests
• typings

[changeset-bot]

⚠️ No Changeset found

Latest commit: 3bdb3bd

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[erikkroes]

To get the ball roling, this would be a starting point: https://www.w3.org/TR/wai-aria-practices-1.2/#menu

[tlouisse]

Hey,



Looks like a great start, thanks for the contribution 💪


For a side project I was working on, I also needed some menus (as they are really missing in Lion atm) and I created an outline for that.
 It has quite some varying demos:

See https://github.com/ing-bank/lion/tree/feat/manyMenus

As someone who worked on the listbox (and its subclasses like select and combobox) quite a lot, I thought it would be possible to give them all a common ancestor wrt keyboard navigation:

 I did a study on all aria components (or at least that I know of) that are related as well and have common denominators (so tree views and toolbars as well) 


Common denominators


What all components have in common:

• they have an active item
  • this can either be activedescendent (most modern solution, works for ‘1 layer deep’ menus) or roving tab index (needed for multi layer menus)
• They have (on or more) checked items.
• They are controlled via keyboard arrows + [enter] / [space]
• They are either horizontally or vertically oriented




Menus

Menus are based on the following assumptions:

• A menu is not necessarily an overlay (a menu bar is In page flow, but its submenus are overlays)
• A menu can be nested (one component, not a submenu component)

Work still required


This branch is by no means finished, so help here would be greatly appreciated 👍

• it needs unit tests
• it needs to be polished. There might be many small bugs, like the automatic focus doesn’t work as expected everywhere.
• although I followed all aria guidelines, it needs to be validated with screen readers



Since this branch has demos for toolbar, tree, etc. I think it would be good to start off with just the menu component
(although we should keep in mind those other use cases, so we don't need to refactor our fundament later).
Ideally, the behavior should also be shared within ListboxMixin, but for a first iteration this would not be needed.

About the api

I largely agreed with your api here and as discussed in the other issue (especially about nested menus, but made some changes on top:

1 level

  <button data-invoker>Open menu</button>
  <lion-menu>
    <div role="menuitem">Go to Definition</div>
    <div role="menuitem">Go to Type Definition</div>
  </lion-menu>

multi level

<nav>
  <button data-invoker>Open menu</button>
  <lion-menu>
    <div role="menuitem">Go to Definition</div>
    <div role="menuitem">Go to Type Definition</div>
    <div>
      <div role="menuitem" data-invoker>Peek</div>
      <lion-menu>
        <div role="menuitem">Peek Call Hierarchy</div>
        <div role="separator"></div>
        <div role="menuitem">Peek Definition</div>
      </lion-menu>
    </div>
    <div role="separator"></div>
    <div role="menuitem">Find all References</div>
  </lion-menu>
</nav>

• So i avoided the need for a slot=content to make it a bit nicer
• Also I put the invokers as siblings of the content (this is needed for a11y if you have multiple layers nested)

Everyone is free to comment on this api



Btw: please feel free to say when you don’t like it or that it’s better to go in a different direction

[MathieuPuech]

This PR is just a proposal, we can use your implementation if you are more advanced in the work.

There was some discussion about API here: #903

In this discussion, we agreed to use another attribute for specifying the types of nodes. The role attributes could be added in the component. We can argument about this if you want.

Using listbox for navigation is not very useful since it's not selecting anything but I see in your use cases that you can select things too in menu. It's a common use case for applications but I didn't see this in ARIA Authoring Practices.

We will need to adapt listbox to accept list with items that can't be selected (like the separators).

I suppose it's better without the slot=content.

I see you have full menubar. I suppose we can create a menubar package containg a menubar and menu component or it can be 2 packages.

[tlouisse]

Thanks. It would be awesome if you can elaborate on this of course 🙂
Feel free to contact us on how to start/determine the scope etc. etc. 💪

menubar

The menubar is behavior wise pretty close to the [role=menu] (except for role(menubar) and orientation I think), but we could indeed consider to make it a package. (toolbar and tree should definitely be their own packages, but here I'm fine with either one of the choices).

api for items

The api is debatable indeed. Normally we create an abstraction on top of the roles, so the suggestion in #903 would be consistent with that choice. (difference here with normal is that now they would be 1-to-1 mappings, but holding on to this principle would also be nice)
If we would be really strict, you could say that, since we apply them not on webcomponent hosts, the attributes should have a data- prefix in order to pass a w3c validator.
I would be fine with either on of the above options, but maybe some consumers would care about this w3c validator compliance.

We would at least need to support the following roles:

• menuitem
• menuitemradio
• menuitemcheckbox
• group
• separator

And - because the generic mixin (called InteractiveListMixin, name may be changed) supports trees/listboxes/toolbars etc - the roles below as well:

• treeitem
• option
• listitem (?)

bit more about overlay vs 'in page flow'

Biggest implication of not always making menu an overlay would be to make it in such a way that it behaves like a disclosure/collapsible by default (think of loading a menu inside of a drawer), but will act as an overlay as soon as you apply the OverlayMixin.
If we normalize apis between Collapsible and Overlay, this is easily achievable.
I already did all the groundwork for thsi in DisclosureMixin, but it would need unit tests and would be needed to be aligned with latest master :)

So again, feel free to contact us about that.

proposed funcionality for menu package

I would propose that we start with a menu package, that supports:

• 'in page flow' (or disclosure) menus
• overlay menus
• multi level menus
• animation hooks (should be part of Disclosure- and OverlayMixin)
• a11y (try to support functionality aria demos as suggested by @erikkroes)
• menuitemcheckbox/menuitemradio + groups (within one menu level)
• ...more?

[MathieuPuech]

The list of feature seems good to me :)

It's a lot of feature, I don't know if we need to add all of theses features from the beginning.

And I don't know if it will be easy to keep <div> for items if we have items with checkbox and radio. We may need a component for this?

For the overlay, menu are generally with it but if you think we can make it without easily too, that's fine. Material Design define menus like that:

Menus display a list of choices on temporary surfaces.

I don't know if "temporary surfaces" are always considered as in an overlay or not.

[tlouisse]

The list of feature seems good to me :)
It's a lot of feature, I don't know if we need to add all of theses features from the beginning.

Most of it is already there I think, but it needs to be polished, tested and rebased with master (which is also still some work). But let's find out together what makes sense for a first iteration. You can also do a proposal to change the scope.

And I don't know if it will be easy to keep <div> for items if we have items with checkbox and radio. We may need a component for this?

Currently they are already managed by the menu component. I think they should not be advanced childs like checkbox and radio, which are form elements. menuitemradio indicates one item can be selected and menutimcheckbox suggests multiple can be selected. LionMenu just keeps track of those states, the childs themselves only handle presentation.

For the overlay, menu are generally with it but if you think we can make it without easily too, that's fine. Material Design define menus like that:
Menus display a list of choices on temporary surfaces.
I don't know if "temporary surfaces" are always considered as in an overlay or not.

No clue what "temporary surfaces" means :) It suggests an overlay indeed, but then again a menubar is never an overlay. And also if you want to implement a drawer like in the Polymer docs (please make your screen width tablet/mobile size and open the drawer) you would place a 'disclosure menu' inside an overlay/drawer.

[MathieuPuech]

@tlouisse Is there anything I can do on the code you've started to do with menus?

I suppose this PR could be closed with your code.

[MathieuPuech]

@tlouisse Is there anything I can do on the code you've started to do with menus?

I suppose this PR could be closed with your code.

I miss the link of your branch in the long message you've posted. I will do PR targeting your branch if you are ok with that.

[tlouisse]

@tlouisse Is there anything I can do on the code you've started to do with menus?
I suppose this PR could be closed with your code.

I miss the link of your branch in the long message you've posted. I will do PR targeting your branch if you are ok with that.

@MathieuPuech sorry for the very late response. All due to the silly reason these github mails are automatically archived in
my mail app and it didn't send me the notifications.

Would you still be up to work on this?

If so, I could reserve some time on short notice to rebase my branch with latest master, do some cleanup and explain the main ideas on that branch (https://github.com/ing-bank/lion/tree/feat/manyMenus) 👍

[MathieuPuech]

Would you still be up to work on this?

Yes, I can work on this, let me know when it's rebased and what can I do to help

[tlouisse]

Would you still be up to work on this?

Yes, I can work on this, let me know when it's rebased and what can I do to help

@MathieuPuech I spent half a day on this last weekend to get back into it again. Hopefully I can push something next weekend.
Just letting you know it's on my radar 👍

[gerjanvangeest]

This PR still needs some attention, maybe we can bring it back to live and finalise it soon!
Anybody lending a hand, feel free 💪!