-
-
Notifications
You must be signed in to change notification settings - Fork 632
fix: Scrolling bug when clicking comment marks #2165
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
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
@blocknote/ariakit
@blocknote/code-block
@blocknote/core
@blocknote/mantine
@blocknote/react
@blocknote/server-util
@blocknote/shadcn
@blocknote/xl-ai
@blocknote/xl-docx-exporter
@blocknote/xl-email-exporter
@blocknote/xl-multi-column
@blocknote/xl-odt-exporter
@blocknote/xl-pdf-exporter
commit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Always happy to remove an effect!
|
|
||
| const threadId = commentMark?.attrs.threadId as string | undefined; | ||
| self.selectThread(threadId, false); | ||
| self.selectThread(threadId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the second param? Why is it removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could set it to false to prevent scrolling to the mark (scrollToThread). We did this before and handled the scrolling in the Card component instead. Turns out. this was the only place it was set so I removed it outright.
|
Unsure if this was introduced @matthewlipski but when I was testing this I ran into this Screen.Recording.2025-11-10.at.18.16.26.movIs this what you meant with your comment? Could we do something quick about this? I think it might just be a race condition of the scroll happening after the floating happens (or vice versa?) |
Summary
This PR fixes a bug which could cause the page to be scrolled to the top when clicking a comment mark. This was because we would attempt to scroll to the floating thread view that pops up after the comment is selected. This would happen before FloatingUI had a chance to position the thread view, so the default position of {0, 0} was used and scrolled to.
This PR removes scrolling to the floating thread view when selecting a comment mark, and instead just scrolls to the mark itself. This makes it reusable for all UI libraries and means we don't have to deal with any FloatingUI weirdness.
Closes #2158
Rationale
The scrolling to the top of the page is pretty annoying when trying to view comments.
Changes
See above.
Impact
There is a slight issue now in that the floating thread view doesn't scroll with the page when a comment is selected and automatically scrolled to. It lacks a scroll listener, which is also an issue for regular user scrolling.
While adding one would be an easy fix, #2143 will negate the need for adding scroll listeners to UI elements in general, so I think it doesn't make sense to add something we will remove in a few days.
Testing
N/A
Screenshots/Video
N/A
Checklist
Additional Notes