Implement Menu

[miguelcobain]

Opening the PR to gather initial feedback on the approach.
Currently opens/closes the popover with internal state.

@NullVoxPopuli is this initial approach what you expected?

Next step is yielding the Item and Separator components from Content.

I think a good first goal for the Menu would be an API like:

<Menu as |m|>
  <button type="button" {{m.trigger}}>Menu</button>

  <m.Content as |c|>
    <c.Item>Item 1</c.Item>
    <c.Item>Item 2</c.Item>
    <c.Separator />
    <c.Item>Item 3</c.Item>
  </m.Content>
</Menu>

Some questions I have:

• should we allow passing in an @isOpen arg, to control the open state of the Menu from the outside?
• if yes to the above, should we provide an accompanying @onToggle action?
• in the proposed API, we discussed items and separators as components. Could it make sense to have them as modifiers instead? That would probably more freedom to the caller, no? 🤔
• <ComponentSignature @module="components/menu" @name="Signature" /> probably won't work because we're importing Args type from another file, right? (i.e Args: PopoverSignature['Args'];).
• we should probably default @placement arg to 'bottom-start', since it's how most menus work like. Do you agree?

[NullVoxPopuli]

is this initial approach what you expected?

yes! this is looking fantastic!

should we allow passing in an @isopen arg, to control the open state of the Menu from the outside?

I can't think of a use case where we want to force a menu open 🤔
if one comes up, there is no reason we can't add it later.

if yes to the above, should we provide an accompanying @onToggle action?

n/a

in the proposed API, we discussed items and separators as components. Could it make sense to have them as modifiers instead? That would probably more freedom to the caller, no? 🤔

what would these modifiers do?

<ComponentSignature @module="components/menu" @name="Signature" /> probably won't work because we're importing Args type from another file, right? (i.e Args: PopoverSignature['Args'];).

I'm not sure -- if it ends up not working, we can just say "see Popover args" for now with a link -- the way I'm doing typedoc introspection is super weird, and I have an issue open with the maintainer to maybe one day have some help figuring stuff out 😅 github.com/TypeStrong/typedoc/issues/2528

we should probably default @placement arg to 'bottom-start', since it's how most menus work like. Do you agree?

I do, yes

[miguelcobain]

what would these modifiers do?

They would be used to "register" items and separators. Example:

<Menu as |m|>
  <button type="button" {{m.trigger}}>Menu</button>

  <m.Content as |c|>
    <div {{c.item}}>Item 1</div
    <div {{c.item}}>Item 2</div
    <div {{c.separator}}></c.Item>
    <div {{c.item}}>Item 3</div
  </m.Content>
</Menu>

item and separator modifiers would add role="menuitem" and role="separator" respectively.
Now that I'm reading aria docs, it feels like using a modifier would get pretty difficult really fast with things like managing aria-disabled="true" on the items.

Now that I think about it, the trigger should also have aria-expanded="true|false", which is tricky to manage with just a trigger modifier. :/

[NullVoxPopuli]

Ah yeah, more reason for components for the items and separators.

For the trigger modifier, it can still manage that aria attribute, 'cause i don't think we ever omit it?

[miguelcobain]

For the trigger, there's also aria-expanded and aria-controls.
A trigger component would make things easier.

[miguelcobain]

Another argument in favor is:

Any button that open a menu should have a role of button or, preferably, be a , and also have aria-haspopup="menu" (or true) set.

So, I think we shouldn't encourage the user to use any element at all, since it requires interactivity.
Plus, as mentioned before, a component allows us to manage more things for the user (aria-controls, aria-haspopup, aria-expanded, and probably other things in the future).

[miguelcobain]

@NullVoxPopuli this PR is ready for review. 🙏

[miguelcobain]

@NullVoxPopuli feedback was applied

[miguelcobain]

Dependent on CrowdStrike/ember-velcro#190