fix(desktop): keep agent activity feed pinned to bottom across smooth-scroll#1142
Open
tlongwell-block wants to merge 1 commit into
Open
fix(desktop): keep agent activity feed pinned to bottom across smooth-scroll#1142tlongwell-block wants to merge 1 commit into
tlongwell-block wants to merge 1 commit into
Conversation
…-scroll `useStickToBottom` recomputed `isNearBottomRef` from raw `scrollTop` on every scroll event. When the hook scheduled a `behavior: "smooth"` scroll to follow new content, the browser emitted intermediate scroll events whose `scrollTop` was mid-animation — `distance` > 100 even though the viewport was animating *toward* the bottom. The buggy handler flipped `isNearBottom` to false during those events, and the next content append refused to follow. User-visible symptom: the agent observer / activity feed scrolls down on the first message, then "jumps back a couple lines" as more tokens arrive. Make `onScroll` direction-aware via `lastScrollTopRef`: - `distance < 100` → always sticky (handles initial state and resticks once an animation settles). - `scrollTop < lastScrollTop` → user pulled the viewport upward. Detach. - Otherwise → leave the sticky bit alone (smooth-scroll in flight toward the bottom). Seed `lastScrollTopRef` in the initial-scroll effect so the first real scroll event has a meaningful prior value. Regression test: a Playwright spec drives a fixture that uses the real hook through a deterministic mid-animation race — grow the scroll range with a content burst, then synthetically dispatch an intermediate `scroll` event whose `scrollTop` is climbing toward the new bottom but `distance > 100`. Pre-fix that assertion fails on `isNearBottom`; post-fix it passes. A second spec guards the direction-aware unstick path so the fix doesn't over-correct. Co-authored-by: Tyler Longwell <tlongwell@squareup.com> Signed-off-by: Tyler Longwell <tlongwell@squareup.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Fix the activity / observer feed "jump back a couple lines" bug reported by Will.
useStickToBottom(used byAgentSessionThreadPanel) recomputedisNearBottomReffrom rawscrollTopon every scroll event. When the hook scheduled abehavior: "smooth"scroll-to-bottom to follow streaming agent content, the browser emitted intermediate scroll events whosescrollTopwas mid-animation —distance > 100even though the viewport was animating toward the bottom. The buggy handler flippedisNearBottomto false during those events, and the next content append refused to follow.User-visible symptom: scroll all the way down while an agent is working, wait a few seconds, and the view jumps back a couple lines.
Fix
Make
onScrolldirection-aware vialastScrollTopRef:distance < 100→ always sticky (handles initial state and re-sticks once an animation settles)scrollTop < lastScrollTop→ user pulled the viewport upward, detachThe initial-scroll effect seeds
lastScrollTopRefso the first real scroll event has a meaningful prior value.~10 net lines in
useStickToBottom.ts. No behavior change for the user-pulled-up path.Tests
Two Playwright specs in
tests/e2e/stick-to-bottom-scroll-jump.spec.tsdrive a tiny React fixture (src/testing/stickToBottomFixture.tsx) that uses the real hook in a scroll container, plus a smallsimulateScroll(top)method to deterministically inject the intermediatescrollevent that production sees mid-animation. The fixture mounts via the existing e2e bridge with a lazy-installed install hook so the production bundle isn't bloated.intermediate smooth-scroll events do not unstick the activity feed— pins the regression. Grows the scroll range with a content burst, then dispatches ascrollevent whosescrollTopis climbing toward the new bottom butdistance > 100. Pre-fix this assertion fails onisNearBottom; post-fix it passes. Verified by reverting the hook locally — that single spec fails on exactlyexpect(midAnimation.isNearBottom).toBe(true).user scrolling up still unsticks the activity feed— guards against over-correction. Passes both with and without the fix; ensures the direction-aware logic still releases when the user pulls the view up.Registered in the
smokeproject.Verification
pnpm typecheck✓pnpm check✓pnpm build✓pnpm exec playwright test stick-to-bottom-scroll-jump.spec.ts --project=smoke→ 2 passedReported by @willpfleger.