Incorrect Resize Behavior for Pasted Chips#127
Conversation
ihordubas99
changed the title
judithroth
left a comment
judithroth
left a comment
There was a problem hiding this comment.
Good catch! I saw that bug once but couldn't reproduce it (because I was not aware it affected copied instances).
I haven't looked to closely at your fix now - do you see a chance that we don't need to expose / call as many functions and have the calls in the initialize method? (see my comment).
| pasteDeduplicatePluginKey, | ||
| } from "../plugins/pasteDeduplicatePlugin"; | ||
|
|
||
| export function useDeduplicateInstanceIds( |
There was a problem hiding this comment.
Like mentioned here - do you see a way this and useInlineWpEvents could be called in initializeOpBlockNoteExtensions?
So that someone who wants to use op-blocknote-extensions only needs to care about including initializeOpBlockNoteExtensions?
There was a problem hiding this comment.
Great idea! Unfortunately, since initializeOpBlockNoteExtensions is called at module level outside a React component, we can't place hooks there directly - React would throw an "Invalid hook call" error. Instead, I created a useOpBlockNoteExtensions hook that wraps both useInlineWpEvents and useDeduplicateInstanceIds. Host app now only needs one hook call inside the component alongside initializeOpBlockNoteExtensions outside
There was a problem hiding this comment.
Thanks for trying and for the explanation. It's a pity we can't just have it all in one though 🥲
ihordubas99
force-pushed
the
bug/74190-incorrect-resize-behavior-for-pasted-chips
branch
from
3f13037 to
d1615b4
Compare
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
ihordubas99
force-pushed
the
bug/74190-incorrect-resize-behavior-for-pasted-chips
branch
from
d1615b4 to
4da8f6b
Compare
judithroth
left a comment
judithroth
left a comment
There was a problem hiding this comment.
Your code is nice and clean 👍 and the tests for copy and pasting make me very happy 😍
But I have a general question about the approach (see comment down below)
| await userEvent.keyboard('{Control>}v{/Control}'); | ||
|
|
||
| const chips = page.getByText('#123'); | ||
| expect((await chips.all()).length).toBe(2); |
There was a problem hiding this comment.
Oh, wow, you managed to write tests for copy pasting 🤩
| import { useInlineWpEvents } from './useInlineWpEvents'; | ||
| import { useDeduplicateInstanceIds } from './useDeduplicateInstanceIds'; | ||
|
|
||
| export function useOpBlockNoteExtensions( |
| ) { | ||
| nodes.push( | ||
| node.type.create( | ||
| { ...node.attrs, instanceId: makeInstanceId() }, |
There was a problem hiding this comment.
Ok, I wonder: With the other PR you added a separate copy/paste handler. Could this handler also take care of setting a new instanceId?
Then we wouldn't need this callback here at all, would we?
There was a problem hiding this comment.
Even if you succeed and we don't need the useDeduplicateInstanceIds any more, I think we should keep useOpBlockNoteExtensions as an entry point for callbacks. It's the better name which is more extensible in the future and we avoid having to rename this all the time if we switch to using the generic name now 🙂
2 participants
Ticket
https://community.openproject.org/projects/communicator-stream/work_packages/74190
What are you trying to accomplish?
Fix incorrect resize behavior for pasted inline work package chips.When a chip was copied and pasted,
BlockNote duplicated all props verbatim - including instanceId. Since wpBridge routes resize/delete actions by instanceId, two chips with the same ID caused every action to always affect the first chip in the document, not the intended one.
What approach did you choose and why?
Registered a ProseMirror transformPasted plugin (pasteDeduplicatePlugin) that intercepts pasted content before it enters the document and regenerates instanceId for every inline WP chip found in the slice.
Also updated the host application (OpBlockNoteEditor) to call useDeduplicateInstanceIds: opf/openproject#22969
This is the correct hook for this problem - it prevents the duplicate from ever existing rather than cleaning it up after the fact. The plugin is registered via _tiptapEditor.registerPlugin(), following the same pattern already established in BlockWorkPackageComponent for selectionUpdate.
Alternative considered: an onChange listener that scans for duplicate instanceId values and patches them post-insert. Discarded because it runs on every editor change, has a window where the document is in an inconsistent state, and requires a guard to prevent infinite loops from its own updateBlock calls.
Merge checklist