Skip to content

Conversation

@nperez0111
Copy link
Contributor

@nperez0111 nperez0111 commented Nov 10, 2025

Summary

This changes the logic for selecting a mark to try to surface the mark which is actually closest to the clicked position. Otherwise, it will give preference to marks which span shorter lengths (which are more likely to be hidden when overlapping with other marks).

Rationale

What is happening here is that a text range can contain several marks overlapping on it (pointing to different thread ids). This attempts to surface the "correct" comment by picking which ever is actually closest to the selected position in absolute distance.

Changes

Sorts the list of marks by distance, and chooses the closest mark to the current position to select the thread of

Impact

Testing

Manual testing was done for this. I'm not sure how we'd test this event handler right now, and we want to get this in quickly

Screenshots/Video

Checklist

  • Code follows the project's coding standards.
  • Unit tests covering the new feature have been added.
  • All existing tests pass.
  • The documentation has been updated to reflect the new feature

Additional Notes

Fixes #2073

@vercel
Copy link

vercel bot commented Nov 10, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Updated (UTC)
blocknote Ready Ready Preview Nov 10, 2025 3:11pm
blocknote-website Ready Ready Preview Nov 10, 2025 3:11pm

@nperez0111 nperez0111 changed the title fix(comments): always surface the closest mark to the current position fix(comments): always surface the closest mark to the current position #2073 Nov 10, 2025
@pkg-pr-new
Copy link

pkg-pr-new bot commented Nov 10, 2025

Open in StackBlitz

@blocknote/ariakit

npm i https://pkg.pr.new/TypeCellOS/BlockNote/@blocknote/ariakit@2164

@blocknote/code-block

npm i https://pkg.pr.new/TypeCellOS/BlockNote/@blocknote/code-block@2164

@blocknote/core

npm i https://pkg.pr.new/TypeCellOS/BlockNote/@blocknote/core@2164

@blocknote/mantine

npm i https://pkg.pr.new/TypeCellOS/BlockNote/@blocknote/mantine@2164

@blocknote/react

npm i https://pkg.pr.new/TypeCellOS/BlockNote/@blocknote/react@2164

@blocknote/server-util

npm i https://pkg.pr.new/TypeCellOS/BlockNote/@blocknote/server-util@2164

@blocknote/shadcn

npm i https://pkg.pr.new/TypeCellOS/BlockNote/@blocknote/shadcn@2164

@blocknote/xl-ai

npm i https://pkg.pr.new/TypeCellOS/BlockNote/@blocknote/xl-ai@2164

@blocknote/xl-docx-exporter

npm i https://pkg.pr.new/TypeCellOS/BlockNote/@blocknote/xl-docx-exporter@2164

@blocknote/xl-email-exporter

npm i https://pkg.pr.new/TypeCellOS/BlockNote/@blocknote/xl-email-exporter@2164

@blocknote/xl-multi-column

npm i https://pkg.pr.new/TypeCellOS/BlockNote/@blocknote/xl-multi-column@2164

@blocknote/xl-odt-exporter

npm i https://pkg.pr.new/TypeCellOS/BlockNote/@blocknote/xl-odt-exporter@2164

@blocknote/xl-pdf-exporter

npm i https://pkg.pr.new/TypeCellOS/BlockNote/@blocknote/xl-pdf-exporter@2164

commit: 5ecf388

Copy link
Collaborator

@matthewlipski matthewlipski left a comment

Choose a reason for hiding this comment

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

Looks good, though am I right in thinking that there's still an issue if 2 comments span the exact same range?

@nperez0111
Copy link
Contributor Author

Looks good, though am I right in thinking that there's still an issue if 2 comments span the exact same range?

Yep, theoretically, we could cycle between the marks at a range, (i.e. you have to click again to see the "next" mark), but honestly that is an edge case of an edge case that isn't worth it right now

@matthewlipski
Copy link
Collaborator

Yeah makes sense, probably more sensible to come up with a UI solution to see multiple threads at once at that point anyway (which ofc is way out of scope), since cycling through threads on click would probably be confusing to users.

@nperez0111 nperez0111 merged commit 2d0afdd into main Nov 11, 2025
20 checks passed
@nperez0111 nperez0111 deleted the fix/2073 branch November 11, 2025 09:12
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.

Overlapping comment

3 participants