-
-
Notifications
You must be signed in to change notification settings - Fork 263
Split ContextMenu into ContextMenuInner subcomponent #622
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
Conversation
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
geoidesic
commented
Jun 27, 2021
Any chance of resolving the conflicts?
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
commented
Jul 10, 2021
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
e7485c4 to
417102d
Compare
1a0a744 to
107b77d
Compare
9d66653 to
fb6719b
Compare
9f82cf8 to
f8cb660
Compare
9a1d380 to
831a3f0
Compare
234c1f9 to
e774974
Compare
See #620
This PR extracts most functionality of
ContextMenutoContextMenuInner, except event handling.The functionality of
ContextMenushould be unaffected by this.ContextMenuInnercan now be used when listening foron:contextmenuonwindowis 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