Skip to content

fix(web-ui): stop animating programmatic transcript scrolls - #959

Open
time-attack wants to merge 2 commits into
mainfrom
qm-approval-scroll-ux-fix
Open

fix(web-ui): stop animating programmatic transcript scrolls#959
time-attack wants to merge 2 commits into
mainfrom
qm-approval-scroll-ux-fix

Conversation

@time-attack

@time-attack time-attack commented Sep 6, 2026

Copy link
Copy Markdown
Collaborator

Problem

When a run ended with a pending approval, the web-ui transcript visibly scrolled from the top of the conversation all the way to the bottom as an animation.

The transcript scroller (.chat-scroll) had scroll-behavior: smooth. That property only affects programmatic scrolls (assigning scrollTop), never wheel or trackpad input. Every redraw that re-sticks the transcript to the bottom therefore animated, and when the scroller had been rebuilt at the top the animation covered the whole conversation.

Change

  • Remove scroll-behavior: smooth from .chat-scroll.
  • Delete the three call sites that flipped style.scrollBehavior to auto and back to dodge the animation (forced scroll-to-bottom, and both pagination anchor adjustments). They are now plain scrollTop writes.
  • The layout test now asserts no smooth scrolling in the transcript CSS and no scrollBehavior juggling in chat.ts, and still asserts both pagination anchors exist.

Net: 5 insertions, 20 deletions.

Demo

Behavior-only change: the after state renders pixel-identical to before, the difference is that scroll-to-bottom is now instant instead of animated. No screenshot can show it, so none attached. To see it: open a long conversation, trigger a run that ends in an approval, and observe the transcript snaps to the pending approval instead of animating from the top.

Verification

  • plugins/web-ui: layout-thrash, tab-switch-scroll-snap, pane-approval-overflow tests pass.
  • plugins/web-ui typecheck passes; eslint clean on changed files.

Not in this PR

The screenshot that prompted this also asked for an "allow always" option on security-screen quarantine releases. Those are deliberately once-only in the orchestrator (server-side guard plus tests). The existing knob is the security-posture admin resource (dangerous disables content screening). Left unchanged here.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

The transcript scroller had scroll-behavior: smooth, so every
programmatic scroll-to-bottom animated. When a run ended with a pending
approval the transcript redrew and re-stuck to the bottom, and because
the scroller started at the top the browser animated the entire way
down. Smooth scrolling only affects programmatic scrolls, never user
input, so drop it and delete the three sites that toggled the style off
and back on to work around it.
Both earlier-messages loaders captured scrollTop and scrollHeight, rebuilt
the transcript, then re-derived scrollTop from the height delta with the
same inline block. Review flagged the pair as drift-prone once the smooth
scroll workaround was gone. One helper now holds the anchor and returns
the restore step; the layout test asserts both paths go through it
instead of counting copies.
@time-attack

Copy link
Copy Markdown
Collaborator Author

Independent review pass (code-review, medium, eight angles) on the first commit found no correctness issues. Two cleanups it raised are applied in the follow-up commit:

  • The two pagination anchor-restore blocks were verbatim duplicates once the smooth-scroll workaround was gone. They now share one holdScrollAnchor helper.
  • The layout test no longer pins the duplication at exactly two copies. It asserts both loaders route through the helper.

Confirmed root cause from the altitude angle: mountContinuable and mountReadOnly build a fresh host at scrollTop 0 and snap to the bottom a frame later, which scroll-behavior: smooth turned into a full-transcript animation. Removing the CSS rule fixes every current and future programmatic scroll on that element in one place.

Left as pre-existing and out of scope: scrollTranscript has no pending-rAF coalescing guard, and the two mount paths duplicate the build-then-snap shape.

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