Skip to content
This repository has been archived by the owner on Mar 4, 2020. It is now read-only.

[RFC] ContextMenu Component #345

Closed
gopalgoel19 opened this issue Oct 10, 2018 · 4 comments · Fixed by #1696
Closed

[RFC] ContextMenu Component #345

gopalgoel19 opened this issue Oct 10, 2018 · 4 comments · Fixed by #1696
Labels
⚙️ enhancement New feature or request help wanted Extra attention is needed new component Component planned to be added to Stardust RFC vsts Paired with ticket in vsts

Comments

@gopalgoel19
Copy link
Member

A new ContextMenu component

Problem description

Building a new component ContextMenu.
getimage

Proposed solution

As of now, I am planning to build this component using Menu, Popup, Avatar etc.
Check progress at #340

Requesting For Comments on:

  1. What is the best approach to build this component?
  2. What all functionality should we extend to Menu, if any, to support ContextMenu?
@levithomason
Copy link
Member

I think this component is great and we should include it. The props I would imagine would be essentially the Menu's props initially. This way you can pass any items the context menu. As the Menu API grows to support more features, the Context Menu would also support those features.

@levithomason levithomason added ⚙️ enhancement New feature or request help wanted Extra attention is needed new component Component planned to be added to Stardust labels Oct 20, 2018
@gopalgoel19
Copy link
Member Author

Should we have a menu prop in Menu.Item to render a sub-menu? If yes, what is the difference between a ContextMenu component and a Menu component with sub-menus?

@pkumarie2011 pkumarie2011 added the vsts Paired with ticket in vsts label Oct 22, 2018
@jurokapsiar
Copy link
Contributor

I would like to discuss starting work on this component - it would solve some very common use cases. We have Menu Button prototype available, but that is far too much code for the consumers to copy. ARIA has a pattern for menu button, other libraries have it as well (https://material-ui.com/components/menus/) and we recently got a requirement for it internally as well. We will also use it heavily in combination with list item and tree item.

The API should include:

  • items shorthand (similar to Menu)
  • on ['click' | 'hover+click' | 'context']

@jurokapsiar
Copy link
Contributor

The alternative would be <Button menu={{ ... }} />, let's consider that as well. The important scenario is:

  • list item has a context menu button
  • by default, when navigating in the list, the focus goes to the list items
  • if the list item has context menu button, that would not be focusable, we would however allow users to press the context menu button (secondary action in screen readers) to open the menu
  • with that, the oncontext event handler would need to be on the focused list item and the menu would need to be controlled from outside

If the scenario above is doable using Button with menu prop, I am ok with it. If not, we would need a separate Context Menu component.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
⚙️ enhancement New feature or request help wanted Extra attention is needed new component Component planned to be added to Stardust RFC vsts Paired with ticket in vsts
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants