Skip to content

Conversation

@Soxasora
Copy link
Member

@Soxasora Soxasora commented Oct 19, 2025

Description

Fixes #2606
When navigator receives a new comment ref, the deduplication process only checks if the exact ref received is already present in the commentRefs array. When a comment gets collapsed and uncollapsed, its ref dies and the comparison would just fail.

The fix involves making the comment counter aware of stale (disconnected) refs and refreshing it on every action.
This way, the navigator gets rid of stale refs and the counter will always be truthful to what is shown to the user; fixing duplicate counts (collapsing and uncollapsing), and stale counts (collapsing).

Screenshots

tbd

Additional Context

This is another reminder for me to refactor the outlining system, since the outlining effect shouldn't run on every collapse/uncollapse.

Checklist

Are your changes backward compatible? Please answer below:

For example, a change is not backward compatible if you removed a GraphQL field or dropped a database column.
Yes

On a scale of 1-10 how well and how have you QA'd this change and any features it might affect? Please answer below:
7, scrolling to a comment, collapsing and uncollapsing works correctly.

For frontend changes: Tested on mobile, light and dark mode? Please answer below:
n/a

Did you introduce any new environment variables? If so, call them out explicitly here:
n/a

Did you use AI for this? If so, how much did it assist you?
no


Note

Make the comments navigator robust to disconnected refs and add cleanup to untrack comments, ensuring accurate new-comment counts.

  • Comments Navigator (components/use-comments-navigator.js):
    • Filter out disconnected refs before counting in throttleCountUpdate.
    • In untrackNewComment, update count even if the ref is missing; always call throttleCountUpdate after processing.
    • In scrollToComment, handle missing nodes by triggering a count refresh; update dependencies.
  • Comment component (components/comment.js):
    • Add cleanup to useEffect to call navigator.untrackNewComment(ref, { includeDescendants: true }) when effect cleans up, preventing stale tracking.

Written by Cursor Bugbot for commit 1d0c3e5. This will update automatically on new commits. Configure here.

cursor[bot]

This comment was marked as outdated.

@Soxasora Soxasora marked this pull request as draft October 19, 2025 13:19
Comment on lines +191 to +193
return () => {
navigator.untrackNewComment(ref, { includeDescendants: true })
}
Copy link
Member Author

Choose a reason for hiding this comment

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

I thought of a simpler way to fix this: we can untrack the comment on unmount (which collapsing does) and add a check for disconnected refs in the comment counter.

This way, every time there's an invalid ref being passed, or an action, the navigator cleans itself from stale refs.

@Soxasora Soxasora marked this pull request as ready for review October 19, 2025 14:24
@Soxasora Soxasora changed the title Fix duplicate navigator tracking of new comments , Oct 19, 2025
@Soxasora Soxasora changed the title , Fix new comments navigator duplicate counts Oct 19, 2025
@Soxasora Soxasora added the bug label Oct 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Uncollapsing a new comment re-tracks it

1 participant