Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ContextMenu can only be used on the whole page #620

Closed
NyxCode opened this issue May 2, 2021 · 6 comments
Closed

ContextMenu can only be used on the whole page #620

NyxCode opened this issue May 2, 2021 · 6 comments

Comments

@NyxCode
Copy link

NyxCode commented May 2, 2021

Currently, ContextMenu can only be used on the whole page.
Sadly, this is pretty limiting - I'd like to use it only on an element on my page.

Could ContextMenu be split into a dumb sub-component which doesn't listen for events itself?
I would like to just give it x, y as props and handle everything else myself.

@metonym
Copy link
Collaborator

metonym commented May 6, 2021

I agree that there should be an option to use it on an arbitrary element.

I think that adding a target prop (HTMLElement) to ContextMenu would be a good approach. The default would still be the window.

@NyxCode
Copy link
Author

NyxCode commented May 6, 2021

I guess that would work for most cases, but I'm not sure that would be flexible enough.
At least for my usecase, I need to have a context menu for a lot of elements, and I wouldn't want a seperate ContextMenu for every one of them.

I think this would be a sane change for ContextMenu, but I still believe that something like ContextMenuInner (#622) would be great for more compilcated usecases.

@geoidesic
Copy link

geoidesic commented Jun 27, 2021

Should just be called Menu if it can only be used on the whole page. The whole point of adding the word "Context" before the word "Menu" in the name is, well.. for a menu that differs by context, i.e. different menu depending on which element you click on. Boggles the mind how we've arrived at a ContextMenu that is contextless! :)

@Trystan-SA
Copy link

I also don't understand why ContextMenu is indeed not contextualized based on what item you right-click. Please at least populate the passed on:open forwarded event with a reference to the DOM element we have the cursor over.

<ContextMenu on:open={(...e)=>{console.log(e); }}>
   <ContextMenuOption indented labelText="Copy"/>
   <ContextMenuOption indented kind="danger" labelText="Delete" />
</ContextMenu>

is currently returning this :
Capture d’écran 2021-11-17 194339

@metonym
Copy link
Collaborator

metonym commented Nov 18, 2021

In version 0.50, you can now pass custom elements as trigger targets to ContextMenu

Example: https://carbon-components-svelte.onrender.com/components/ContextMenu#custom-target

@Trystan-SA The event.target is passed to the on:open event detail as well.

<ContextMenu on:open={(e) => {
  console.log(e.detail); // HTMLElement
}}>
  ...
</ContextMenu>

@metonym metonym closed this as completed Mar 15, 2022
@geoidesic
Copy link

geoidesic commented Aug 23, 2023

@metonym Does this mean that this PR is no longer needed? #622

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants