Skip to content

fix(desktop): keep agent activity feed pinned to bottom across smooth-scroll#1142

Open
tlongwell-block wants to merge 1 commit into
mainfrom
sami/fix-activity-feed-scroll-jump
Open

fix(desktop): keep agent activity feed pinned to bottom across smooth-scroll#1142
tlongwell-block wants to merge 1 commit into
mainfrom
sami/fix-activity-feed-scroll-jump

Conversation

@tlongwell-block

Copy link
Copy Markdown
Collaborator

What

Fix the activity / observer feed "jump back a couple lines" bug reported by Will.

useStickToBottom (used by AgentSessionThreadPanel) recomputed isNearBottomRef from raw scrollTop on every scroll event. When the hook scheduled a behavior: "smooth" scroll-to-bottom to follow streaming agent 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: scroll all the way down while an agent is working, wait a few seconds, and the view jumps back a couple lines.

Fix

Make onScroll direction-aware via lastScrollTopRef:

  • distance < 100 → always sticky (handles initial state and re-sticks 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)

The initial-scroll effect seeds lastScrollTopRef so 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.ts drive a tiny React fixture (src/testing/stickToBottomFixture.tsx) that uses the real hook in a scroll container, plus a small simulateScroll(top) method to deterministically inject the intermediate scroll event 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.

  1. intermediate smooth-scroll events do not unstick the activity feed — pins the regression. Grows the scroll range with a content burst, then dispatches a scroll event whose scrollTop is climbing toward the new bottom but distance > 100. Pre-fix this assertion fails on isNearBottom; post-fix it passes. Verified by reverting the hook locally — that single spec fails on exactly expect(midAnimation.isNearBottom).toBe(true).
  2. 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 smoke project.

Verification

  • pnpm typecheck
  • pnpm check
  • pnpm build
  • pnpm exec playwright test stick-to-bottom-scroll-jump.spec.ts --project=smoke → 2 passed
  • Full smoke suite re-run on the 4 unrelated flakes (custom-emoji, messaging, spoiler, video-attachment) → 46/46 pass in isolation; not caused by this change

Reported by @willpfleger.

…-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>
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