feat(EditorDragHandle): proxy nested / nestedOptions props and emit hover event#5960
Conversation
…d 'nestedOptions', omit the same from button props
📝 WalkthroughWalkthroughAdded a Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
commit: |
solidprinciples
changed the title
solidprinciples
changed the title
solidprinciples
changed the title
solidprinciples
changed the title
|
@benjamincanac - noticed a few bugs/things that didn't quite align with documentation. Here's a small concise PR for the fixes:
😄 first contribution, long time user of nuxt ui ❤️ - custom middleware seemed simple enough, and opens doors for flexibility. If its not robust enough, I can revert it. |
solidprinciples
changed the title
benjamincanac
left a comment
benjamincanac
left a comment
There was a problem hiding this comment.
When building the Editor I did try to store the current node and send it inside the nodeChange even like you did but for a reason I never understood this breaks the drag handle completely, I even tried to pass it inside default slot props but didn't work either. My guess is the node.toJSON() object is just too big.
You can try in the playground and see it no longer displays when hovering elements.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/runtime/components/EditorDragHandle.vue`:
- Around line 42-44: The emit was renamed from nodeChange to nodeChanged,
breaking consumers who used `@node-change`; update the API docs and changelog to
document this breaking change and provide a migration note (replace `@node-change`
with `@node-changed`), and ensure the type/interface EditorDragHandleEmits and any
usages of nodeChange in EditorDragHandle.vue and elsewhere are updated to
nodeChanged so templates and listeners reflect the new event name.
|
@solidprinciples We can't introduce a breaking change like this unfortunately, what are you trying to solve here? The |
|
Looking to be able to track currently selected node by the handle upon hover. |
|
What about adding a new diff --git a/src/runtime/components/EditorDragHandle.vue b/src/runtime/components/EditorDragHandle.vue
index 2c1824881..862cc47b9 100644
--- a/src/runtime/components/EditorDragHandle.vue
+++ b/src/runtime/components/EditorDragHandle.vue
@@ -40,6 +40,10 @@ export interface EditorDragHandleSlots {
export interface EditorDragHandleEmits {
nodeChange: [{ node: JSONContent, pos: number }]
+ /**
+ * Emitted when the hovered node changes.
+ */
+ hover: [{ node: JSONContent, pos: number }]
}
</script>
@@ -114,6 +118,13 @@ const currentNodePos = ref<number | null>()
function onNodeChange({ pos }: { pos: number }) {
currentNodePos.value = pos
+
+ if (!props.editor || pos < 0) return
+
+ const node = props.editor.state.doc.nodeAt(pos)
+ if (node) {
+ emit('hover', { node: node.toJSON(), pos })
+ }
}
function onClick() { |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/runtime/components/EditorDragHandle.vue (1)
154-164: Default handle no longer triggersonClick.After removing the root
@click, the fallback<UButton>isn’t wired toonClick, sonodeChangewill never fire for the default slot. Please bind the handler in the fallback content to avoid a regression.🛠️ Suggested fix
<UButton v-bind="{ ...buttonProps, icon: props.icon || appConfig.ui.icons.drag, ...$attrs }" data-slot="handle" :class="ui.handle({ class: [props.ui?.handle, props.class] })" :ui="transformUI(ui, props.ui)" + `@click`="onClick" />
|
@benjamincanac hmm. same issue - I've at least diagnosed it down to this:
Haven't seen anything on tiptap's repo :( |
|
@benjamincanac - https://github.com/solidprinciples/tiptap-draghandle-state-update-bug/tree/bug/drag-handle Tiptap bug - same thing happens in their extension demo. Investigating to see feasibility of fixing upstream. |
|
@benjamincanac fixed in tip-tap - small few-liner in ueberdosis/tiptap#7472 👍 - seems like the DragHandle is re-rendering and re-applying visibility: hidden in these cases. |
|
I already submitted such a fix back in November 😬 ueberdosis/tiptap#7252 But I don't see how it's relevant to this issue 🤔 |
In testing for my use case outside of Nuxt - directly in Tiptap, which this feature depends on - I ran into an issue where the drag handles would disappear when consuming state extracted from @nodeChange externally (i.e. what @hover is now doing). Persisting the value to a ref worked fine, but actually using that ref in the template or elsewhere caused rendering to break. This behavior reproduced consistently in Tiptap’s own extension demo, independent of Nuxt, which is what led me to dig further. That surfaced an upstream Tiptap issue related to the DragHandle re-rendering and re-applying visibility: hidden during external state updates. I was able to isolate, diagnose, and fix it in a minimal repro, and the same fix can be picked up here if needed. tl;dr: without the Tiptap fix, |
|
From a relevancy standpoint, the changes in this PR are small, closely related, and all part of making the documented behavior function correctly. ( Happy to remove middleware changes as that's more of a feature than a fix and very barebones at that. All in this would then just add/fix:
|
solidprinciples
changed the title
solidprinciples
changed the title
| :on-node-change="onNodeChange" | ||
| data-slot="root" | ||
| :class="ui.root({ class: [props.ui?.root, props.class] })" | ||
| @click="onClick" |
There was a problem hiding this comment.
It didn't seem like this did anything - since we pass to the <UButton /> below, unless I am missing something. Otherwise nearing ready for review @benjamincanac
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/runtime/components/EditorDragHandle.vue`:
- Around line 41-43: The public API was changed by adding a new emit named
"hover" to EditorDragHandleEmits in EditorDragHandle.vue; update the public docs
and changelog to document this new event, including its signature (hover: [{
node: JSONContent, pos: number }]), when it is emitted, expected payload shape,
and any behavioral notes or migration guidance for consumers; ensure README/API
reference that lists component emits and the changelog entry for this release
mention the new hover event and its purpose so consumers are aware of the
behavior change.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/runtime/components/EditorDragHandle.vue (1)
154-164: Default UButton missing@clickbinding —nodeChangewon't emit on click.The slot exposes
onClickvia:on-click, but the defaultUButtondoesn't bind a click handler. Since the root-level@clickwas removed (per AI summary), clicking the default drag handle button won't callonClick, breaking thenodeChangeemission.🐛 Proposed fix to bind the click handler on the default button
<UButton v-bind="{ ...buttonProps, icon: props.icon || appConfig.ui.icons.drag, ...$attrs }" + `@click`="onClick" data-slot="handle" :class="ui.handle({ class: [props.ui?.handle, props.class] })" :ui="transformUI(ui, props.ui)" />
benjamincanac
left a comment
benjamincanac
left a comment
There was a problem hiding this comment.
@solidprinciples Let me know if you still have work to do on this, otherwise looks good!
@benjamincanac - tested, all good from my end. do I can wake up and commit the changelog part using the prompt from coderabbit. |
benjamincanac
changed the title
nested / nestedOptions props and emit hover event
|
It isn't necessary as the docs and changelog are automatic 😊 |
2 participants
❓ Type of change
📚 Description
Nested drag handle options are documented but were not being forwarded in the implementation. This PR updates the editor drag handle to correctly pass through the nested and nestedOptions props exposed by TipTap’s DragHandleProps type.
It also omits these same keys from the button props, where they are not used.
Additionally, this PR introduces a new
@hoverevent, emitted when the currently hovered drag handle node changes, allowing consumers to track the active node without relying on click events.Together, these changes fix and enable proper support for nested drag handles and improve drag handle interaction handling.
📝 Checklist