Skip to content

fix: Cut/copy handling in non-editable blocks and empty selections #1427

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

Merged
merged 1 commit into from
Feb 12, 2025

Conversation

matthewlipski
Copy link
Collaborator

@matthewlipski matthewlipski commented Feb 11, 2025

This PR does 2 things:

When the selection is empty, we no longer use our own cut/copy handling and instead let the browser handle the event (so nothing happens). Currently an error is thrown as we attempt to create clipboard data from an empty selection.

When the selection is within a descendant of a "contenteditable"="false" element, we no longer use our own cut/copy handling and instead let the browser handle the event. This means the user is selecting some content in a non-editable block. Currently the editor selection is copied instead as this scenario still triggers PM event handlers.

TODO: Unit tests?

Copy link

vercel bot commented Feb 11, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
blocknote ✅ Ready (Inspect) Visit Preview Feb 11, 2025 11:53pm
blocknote-website ✅ Ready (Inspect) Visit Preview Feb 11, 2025 11:53pm

Copy link
Collaborator

@YousefED YousefED left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. What was the reason for working on this? Curious how / where you ran into it!

Code-wise, some questions / feedback:

  • what if you're within a contenteditable="true" nested in a contenteditable="false"?
  • you might be able to use closest() instead of looping upo the tree

Copy link
Contributor

@nperez0111 nperez0111 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@matthewlipski
Copy link
Collaborator Author

Nice. What was the reason for working on this? Curious how / where you ran into it!

Code-wise, some questions / feedback:

  • what if you're within a contenteditable="true" nested in a contenteditable="false"?
  • you might be able to use closest() instead of looping upo the tree

Good point r.e. the contenteditable="true", I think it would make sense to return false if we hit an editable element before we hit a non-editable one, but I'll do some manual testing with that first.

@matthewlipski
Copy link
Collaborator Author

Ok looks like putting your contentDOM element within a "contenteditable"="false" element is not really supported by ProseMirror anyway (it gets very confused regarding positions). Alternatively if you had say, just a text box within a non-editable block, that text is not part of the editor state and so the browser should handle cut/copy events there.

Long story short looks like the current behaviour is fine

@matthewlipski
Copy link
Collaborator Author

Nice. What was the reason for working on this? Curious how / where you ran into it!

Code-wise, some questions / feedback:

  • what if you're within a contenteditable="true" nested in a contenteditable="false"?
  • you might be able to use closest() instead of looping upo the tree

Also r.e. using closest(), here it doesn't make much sense to do since the event target may just be a text Node, and closest() is only available on Elements. So we would have to traverse the parents until an Element is found to call closest(), and at that point we may as well just continue traversing the parents instead.

@YousefED YousefED merged commit 125442b into main Feb 12, 2025
5 checks passed
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.

3 participants