Skip to content

fix: unify scroll-into-view on scrollIntoViewWithin (CHAT-48) - #99

Merged
rpvilo merged 2 commits into
feat/thread-scroll-timingfrom
feature/chat-48
Oct 23, 2026
Merged

rpvilo merged 2 commits into
feat/thread-scroll-timingfrom
feature/chat-48

Conversation

@rpvilo

@rpvilo rpvilo commented Oct 15, 2026

Copy link
Copy Markdown
Collaborator

Resolves CHAT-48.

Stacked on #90, which adds the alignment math this moves into the shared helper. This PR targets #90's branch so the diff shows only this change; once #90 merges, GitHub retargets it to main, and the branch then needs a rebase onto main.

The package scrolled things into view three ways. The command list used scrollIntoView({ container: "nearest" }), whose container option is Chromium-only: Safari and Firefox ignore it and scroll every ancestor, so arrowing through the list could scroll the page. That was accepted as a known bug on 2026-07-10.

Changes

  • One helper: internal/scroll-into-view-within.ts now holds both pieces.
    • alignedScrollTop (moved from thread/scroll-animation.ts) returns the destination instead of scrolling, for the thread's timed scroll. It follows scrollIntoView's block (start / center / end / nearest) and honors scroll-margin and scroll-padding.
    • scrollIntoViewWithin(target, boundary, { block }) walks the scrollable ancestors up to boundary, aligning in each one with that same math. target is an element, or { rect, from } for the caret, which has no element.
  • Command list: uses the helper. The scroll stops at the popover, or at the composer root when the list is in an in-flow panel, so the page never moves. The Chromium-only container option and the KNOWN BUG comment are removed.
  • Caret, ask-user, thread: use the new signature. Ask-user options now honor their scroll-margin and the list's scroll-padding.
  • No scrollIntoView calls remain in packages/chat/src.

Behavior changes

  • Clamping: the walk keeps every scroll inside the valid range. An old test expected a scroll position of −30, which only its fake layout allowed; it now uses a realistic case.
  • Tall targets: with nearest, a target taller than the view now brings its top in, as browsers do, instead of its bottom. This only affects an unusually tall ask-user option or command row.

Tests

  • alignedScrollTop tests moved to tests/scroll-into-view-within.test.ts, plus a case for a bare rect.
  • New: the command list inside a scrolling page scrolls only the list and honors its scroll-padding.
  • New: an element target is aligned by block, with its scroll-margin included.
  • 584 package tests pass; typecheck and lint are clean.

Not verified

  • Not checked in a real Safari. The fix holds by construction (nothing calls scrollIntoView), and the containment test covers the walk.

@vercel

vercel Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

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

Project Deployment Actions Updated
intentface-chat Ready Ready Preview Oct 23, 2026 6:00pm UTC
intentface-ui Ready Ready Preview Oct 23, 2026 6:00pm UTC

Request Review

@coderabbitai

coderabbitai Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Essentials
  • Run ID: 6c6da834-169f-4664-8266-2ebadcb39712

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

React Doctor found no new issues. 🎉

Reviewed by React Doctor for commit 92380e9.

@rpvilo
rpvilo force-pushed the feat/thread-scroll-timing branch from 3322eef to 2c7a6ca Compare October 23, 2026 17:59
@rpvilo
rpvilo merged commit cc9add6 into feat/thread-scroll-timing Oct 23, 2026
6 of 8 checks passed
@rpvilo
rpvilo deleted the feature/chat-48 branch October 23, 2026 18:02

This branch was successfully deployed

1 active deployment
Preview — 92380e90 Deployed Oct 23, 2026 by vercel[bot]
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.

1 participant