Add right click menu for multiple components#14640
Conversation
| function closeAllMenus() { | ||
| // Close both menus | ||
| alignmentMenuOpen.value = false | ||
| contextMenuTrigger.value?.close() | ||
| } |
There was a problem hiding this comment.
Should be both menus opened by once anytime?
We usually handle such things using Interactions - an example in https://github.com/enso-org/enso/blob/develop/app/gui/src/project-view/components/DropdownMenu.vue#L28-36
| /** Whether the menu is currently open */ | ||
| menuOpen: menuOpen as Ref<boolean>, | ||
| /** Two-way binding model for v-model:open that respects hover state */ | ||
| menuOpenModel, |
There was a problem hiding this comment.
It's not quite clear to me, when reading menuOpen may differ from reading menuOpenModel
There was a problem hiding this comment.
I needed menuOpenModel for getting the hover v click functionality working properly. We can look if we can simplify this, but I struggled.
155510a to
54c7ee6
Compare
| set: (open) => { | ||
| if (!open && menuOpenedByHover.value && menuHovering.value) return | ||
| menuOpen.value = open | ||
| if (!open) menuOpenedByHover.value = false | ||
| }, |
There was a problem hiding this comment.
I prefer to avoid this kind of logic in a computed's setter (conditionally setting, setting other values). It could be an independent function like trySetOpen, and then instead of v-model:open="menuOpenModel" the getter and setter can be provided separately like v-bind:open="menuOpen" @update:open="trySetOpen"
| * v-model:open="menuOpenModel" | ||
| * @pointerenter="handleMenuEnter" | ||
| * @pointerleave="handleMenuLeave" |
There was a problem hiding this comment.
Because this composable provides multiple properties to set on a component, and the relationship with the component will be the same everywhere the component is used, there's a pattern that might be applied here to simplify the API: The composable could expose an object containing all the properties that need to be connected to a dropdown menu, and then it could be used like <DropdownMenu v-bind="hoverMenuProps">. For example see
| @close="closeAllMenus" | ||
| /> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
DropdownMenu handles connecting a toggle button to a floating panel, and it seems like all of that same logic is needed here, is it possible to use that component to simplify this?
There was a problem hiding this comment.
I guess this was something I complained about in #14640 (comment) :) for me it was too much of "generalizations" (especially :deep rules), and I prefered another component for "side menus". Of course they may share some logic.
There was a problem hiding this comment.
Oh I see! Yeah, if it's not applicable without substantial customization, it's not worth reusing it.
40e0108 to
54c7ee6
Compare
Pull Request Description
Closes #14587
Important Notes
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
TypeScript,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
or the Snowflake database integration, a run of the Extra Tests has been scheduled.