Skip to content

fix(web): typing a citation comment no longer scrolls the page - #36194

Open
AkiVonAkira wants to merge 2 commits into
pingdotgg:mainfrom
AkiVonAkira:citation-comment-sticky-edge
Open

AkiVonAkira wants to merge 2 commits into
pingdotgg:mainfrom
AkiVonAkira:citation-comment-sticky-edge

Conversation

@AkiVonAkira

Copy link
Copy Markdown
Contributor

Problem

Cite text, scroll the timeline so the quote goes out of view, then type a comment: the whole app jumps. The comment box follows the quote off-screen, so the browser scrolls the page to the caret. Reported in #36193.

Change

The comment box now stays inside the visible part of the timeline, which runs from the timeline's top edge to the top of the composer floating over it.

  • The popup's anchor is clamped to that area, so a quote scrolled out of view pins the box to the edge it left by: just above the composer when you scrolled up, at the top of the timeline when you scrolled down. When the quote comes back into view, the box sits next to it again.
  • The same area is passed to Base UI as the collisionBoundary, so the box flips above the composer instead of covering it. The boundary uses getters because Floating UI reads it on every position update.
  • PopoverPopup passes collisionBoundary through to the positioner. When it's unset, Base UI's default applies, so existing callers don't change.

Typing never scrolls the timeline or the page. This differs from #34787, which scrolls the timeline back to the quote on each keystroke. The user scrolled away for a reason, often to reread something while writing the comment, so this PR keeps their scroll position.

Scope

Fixes #36193. One component, plus one pass-through prop on the shared popover. No contract, server, desktop, or mobile changes. The mobile app doesn't have this popover.

Verification

  • A new browser test in AssistantCitationChip.browser.test.tsx opens the box next to a visible quote, then scrolls the quote under a composer and past the top. On main it fails: the box bottom is at 893, below the composer top at 776. With the fix it passes, and typing leaves page and timeline scroll unchanged.
  • vp run check:changed passes (format, lint, knip, typecheck, tests, React Compiler).
  • Tested in the dev web app with Replay threads, real clicks and typing, at 1280×800 and 390×844. After scrolling the quote 2000px below the window, the box stayed at 551–665 with the composer top at 668. Page scroll stayed at 0 while typing.
  • Not checked: Safari, Firefox, or the packaged desktop app.
Before After
before after, scrolled up

Scrolled down past the quote, and at phone width:

after, scrolled down after, phone

Reviewed with sol-loop: 1 round with GPT-6.1 Sol. Sol verdict: approved.

Created with Claude Opus 5.5 in T3 Code. GPT-6.1 Terra searched for existing issues.

@github-actions github-actions Bot added the vouch:unvouched Author is not yet trusted in the VOUCHED list. label Nov 13, 2026
@github-actions github-actions Bot added the size:M 30-99 changed lines (additions + deletions). label Nov 13, 2026
@macroscopeapp

macroscopeapp Bot commented Nov 13, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at f576073

Macroscope's review found this PR approvable — Citation-comment popovers now stay within the visible timeline when their source quote scrolls offscreen. The change is limited to positioning, with no changes to comment storage or sending behavior.

You can add or adjust custom eligibility rules. Learn more.

@juliusmarminge juliusmarminge added the takeover:in-progress A takeover lead is working on this PR label Nov 13, 2026
@coderabbitai

coderabbitai Bot commented Nov 13, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

The citation comment popup now stays within the visible timeline area above the composer when its source quote scrolls out of view. Browser tests check popup positioning and confirm that typing does not change document or timeline scroll positions.

Changes: Citation popup positioning

Layer / File(s) Summary
Popover collision boundary
apps/web/src/components/ui/popover.tsx
PopoverPopup accepts an optional collision boundary and passes it to the positioner.
Visible timeline positioning and tests
apps/web/src/components/chat/AssistantCitationChip.tsx, apps/web/src/components/chat/AssistantCitationChip.browser.test.tsx
The citation popup clamps its anchor to the visible timeline area and uses that area as its collision boundary. Browser tests check its position as the quote scrolls and verify that typing does not change document or timeline scroll positions.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: juliusmarminge, maria-rcks

Merge Risk: Minimal · up to bcef2

Merge Risk: ⚪ Minimal · up to bcef2

The citation comment popup now stays within the visible timeline above the composer, and typing does not move the scroll position. No concrete merge-blocking defect was identified. A narrow edge case may remain when the composer resizes while the popup is pinned above it; it self-corrects on the next position update.

Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title "fix(web): typing a citation comment no longer scrolls the page" is a conventional commit title. It names the main change: the citation comment box no longer scrolls the page while typing. T…
Description check Passed The description follows the template. It has Problem, Change, Scope, and Verification sections. The Problem section explains the bug and links #36193. The Change section explains the clamping and coll…
Linked Issues check Passed The only directly linked issue is #36193. Its coding requirements are: (1) typing after the quote leaves view must not scroll the page or timeline; (2) the comment box stays in the visible timeline, p…
Out of Scope Changes check Passed All changes map to #36193. AssistantCitationChip.tsx adds the visible-timeline rect helper, the live boundary helper, the clamped anchor, the comment trigger state used to find the composer, and the c…
Full details: Title check

Explanation

The title "fix(web): typing a citation comment no longer scrolls the page" is a conventional commit title. It names the main change: the citation comment box no longer scrolls the page while typing. The changeset does clamp the popup to the visible timeline, pass collisionBoundary through PopoverPopup, and add a browser test for the scroll behavior, so the title matches the primary fix.

Full details: Description check

Explanation

The description follows the template. It has Problem, Change, Scope, and Verification sections. The Problem section explains the bug and links #36193. The Change section explains the clamping and collisionBoundary changes. The Scope section links the issue and states that the change touches one component and one popover prop. The Verification section lists a browser test that fails on main and passes with the fix, a passing check:changed run, manual tests at two viewport sizes, and what was not checked (Safari, Firefox, packaged desktop app). Before/after evidence is embedded. The template asks for a model and harness note, and the description includes one. It does not follow the template's advice to link a maintainer request, but the author states that none was made, so the check still passes.

Full details: Linked Issues check

Explanation

The only directly linked issue is #36193. Its coding requirements are: (1) typing after the quote leaves view must not scroll the page or timeline; (2) the comment box stays in the visible timeline, pinned above the composer when the quote is below and at the timeline top when the quote is above; (3) the box returns beside the quote when the quote is visible again. The diff of AssistantCitationChip.tsx addresses (2) and (3). visibleTimelineRect() computes the timeline area from the viewport top to the composer top (found via closest('[data-chat-composer-stack="true"]')). The popup anchor getter clamps the quote rect's top and bottom into that area. When the quote is visible, the clamp leaves the rect unchanged, so the box sits beside the quote. The collisionBoundary uses liveBoundary() getters, so Base UI sees current rects on each update. Requirement (1) is covered by the browser test described in the PR and the raw summary, which asserts that typing leaves page and timeline scroll unchanged. I did not run that test. The non-coding manual checks (Safari, Firefox, desktop) are outside the requirements and do not apply.

Full details: Out of Scope Changes check

Explanation

All changes map to #36193. AssistantCitationChip.tsx adds the visible-timeline rect helper, the live boundary helper, the clamped anchor, the comment trigger state used to find the composer, and the collisionBoundary prop on PopoverPopup. popover.tsx adds an optional collisionBoundary prop forwarded to the positioner; when unset, the Base UI default applies, so other callers are unaffected. The browser test in AssistantCitationChip.browser.test.tsx covers the new behavior. The PR reports no contract, server, desktop, or mobile changes, and the raw summary shows none. No unrelated change was found in the reviewed diff. The whole-PR diff was checked only for AssistantCitationChip.tsx; the popover.tsx and test file changes were assessed from the summary.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between 9a35ab0 and f576073.

Files selected for processing: 3

  • apps/web/src/components/chat/AssistantCitationChip.browser.test.tsx
  • apps/web/src/components/chat/AssistantCitationChip.tsx
  • apps/web/src/components/ui/popover.tsx

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

@juliusmarminge juliusmarminge added takeover:needs-julius Takeover done; route gate says Julius decides and removed takeover:in-progress A takeover lead is working on this PR labels Nov 13, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). takeover:needs-julius Takeover done; route gate says Julius decides vouch:unvouched Author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Typing a citation comment after scrolling the quote away scrolls the whole page

2 participants