Skip to content

Conversation

@matthewlipski
Copy link
Collaborator

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

  • 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

@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:41pm
blocknote-website Ready Ready Preview Nov 10, 2025 3:41pm

@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@2165

@blocknote/code-block

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

@blocknote/core

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

@blocknote/mantine

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

@blocknote/react

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

@blocknote/server-util

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

@blocknote/shadcn

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

@blocknote/xl-ai

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

@blocknote/xl-docx-exporter

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

@blocknote/xl-email-exporter

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

@blocknote/xl-multi-column

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

@blocknote/xl-odt-exporter

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

@blocknote/xl-pdf-exporter

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

commit: a5ccd28

Copy link
Contributor

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);
Copy link
Contributor

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

Copy link
Collaborator Author

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.

@nperez0111
Copy link
Contributor

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.mov

Is 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?)

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.

Comments: Opening a thread trigger a scroll to the top

3 participants