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

Split ContextMenu into ContextMenuInner subcomponent #622

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

NyxCode
Copy link

@NyxCode NyxCode commented May 3, 2021

See #620
This PR extracts most functionality of ContextMenu to ContextMenuInner, except event handling.
The functionality of ContextMenu should be unaffected by this.

ContextMenuInner can now be used when listening for on:contextmenu on window is undesirable. In the documentation, I added an example for binding the context menu to only a single element.

I'm pretty new to svelte & this library, so please thoroughly review my changes.

image

@vercel
Copy link

vercel bot commented May 3, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/carbon-svelte/carbon-components-svelte/HGdrjrhuNVnNwX63mSVKUHTABUVS
✅ Preview: https://carbon-compone-git-fork-nyxcode-split-context-menu-carbo-852ad5.vercel.app

@NyxCode NyxCode changed the title Split context menu Split context menu into ContextMenuInner subcomponent May 3, 2021
@NyxCode NyxCode changed the title Split context menu into ContextMenuInner subcomponent Split ContextMenu into ContextMenuInner subcomponent May 3, 2021
@geoidesic
Copy link

Any chance of resolving the conflicts?

@geoidesic
Copy link

geoidesic commented Jun 27, 2021

I tried just using the ContextMenuInner on it's own and I get this error, which I can't interpret:

proxy.js:15 [HMR][Svelte] Unrecoverable error in <ContextMenuOption>: next update will trigger a full reload
ContextMenuOption.svelte? [sm]:79 Uncaught (in promise) TypeError: Cannot read property 'subscribe' of undefined
    at instance (ContextMenuOption.svelte? [sm]:79)
    at init (index.mjs?v=cfb104fc:1660)
    at new ContextMenuOption (ContextMenuOption.svelte? [sm]:129)
    at createProxiedComponent (svelte-hooks.js:245)
    at new ProxyComponent (proxy.js:241)
    at new Proxy<ContextMenuOption> (proxy.js:341)
    at Array.create_default_slot (SceneThumb.svelte:1)
    at create_slot (index.mjs?v=cfb104fc:61)
    at create_fragment (ContextMenuInner.svelte? [sm]:37)
    at init (index.mjs?v=cfb104fc:1675)

@geoidesic
Copy link

I would consider calling it ElementContextMenu. As when it is used in isolation without the Global it is not Inner.

I also notice that if it is used without the Global, right-click outside of the context menu does not close it. This is undesirable. I had a go at fixing it but couldn't figure out how @NyxCode

@metonym metonym removed their request for review July 10, 2021 22:43
@metonym metonym force-pushed the master branch 2 times, most recently from e7485c4 to 417102d Compare November 8, 2023 03:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants