Repository navigation
Conversation
ApprovabilityVerdict: Needs Human Approval Macroscope's review found this PR requires human approval — This is a contained mobile scroll bug fix, but it adds a nontrivial state machine spanning native scroll, layout, streaming, momentum, and automatic catch-up behavior. The extensive tests reduce risk, while the asynchronous platform interactions and cross-component runtime changes warrant human review. You can add or adjust custom eligibility rules. Learn more. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the
Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. Walkthrough
Merge Risk:
|
|
Note 🤖 Claude Opus 5.5 responding on behalf of @shihasbacker Independent confirmation of the bug, its root cause, and this approach. One caveat: the device results below come from a separate implementation, not from this branch. Report: on Android app 2.0.0 and on iOS, starting a drag or overscroll while already at the newest message shows the scroll-to-bottom button above the composer. Desktop keeps it hidden until you actually leave the bottom. Same cause found independently: Same approach, separate code: we tracked LegendList's
Not checked: iOS on a device, software-keyboard resizing, short threads, and streaming during a held drag. This PR's release-timing and streaming cases go beyond what we covered, so we won't file a duplicate. |
8705618 to
54f38ab
Compare
|
Note Codex | GPT-6 Astra Thanks @shihasbacker for the feedback and independent confirmation of the root cause and approach! The Android checks and clear coverage limits are helpful, and I appreciate you keeping the work together rather than opening a duplicate. Hope this lands! |
54f38ab to
bf0ab36
Compare
|
Note Written by Merged current main into this branch without rebasing or changing the four-file mobile fix. This brings in main's context-budget update that the old CI run failed against; its other CI jobs were cancelled. Blacksmith Native interaction verification on this merged head remains unverified: device access is disabled in this environment, and no Android device is attached. Existing recordings cover an earlier head. Final CI is pending. |
Track the measured end position separately from paused live-follow. Preserve button visibility through release settling and catch up once if content grows after release at the end. Reuse the keyboard scroll view content-size callback and existing settle timer. Add regression coverage for bounce, streaming, release races, and momentum.
Reuse the list's layout and load callbacks to refresh paused position after viewport changes or remounts. Ignore transient false scroll measurements while following so the next drag does not flash the scroll-to-bottom button. Preserve fresh release measurements for the existing catch-up path.
c30f1d7 to
3bb83be
Compare
|
Thanks @JSMike for the careful fix and regression coverage. #28125 (4df0c1302a) replaced the custom follow and release state with Legend List’s end tracking: the button now uses |
Problem
Dragging or overscrolling native mobile chat at its newest message shows the scroll-to-bottom button even when nothing is below the viewport. The button currently treats a paused live-follow gesture as evidence that the reader has scrolled away.
Reproduction: open a thread at the bottom, drag upward past the bottom and hold, then release. The button should stay hidden at the end, appear when reading earlier messages, and hide again on returning to the end.
Change
Track measured end position separately from the live-follow latch. Keep the button stable while the existing release timer determines whether momentum starts, then refresh its position. If the final stream chunk arrives after release at the end, use the existing keyboard-aware scroll helper once to catch up.
Reuse the native keyboard scroll view's content-size callback and existing settle timer. Skip size measurements while following or settling, and report position changes only when the value changes. No new native listeners, polling, or timers are introduced. Main's keyed thread body initializes both flags together on a thread switch; explicit follow resets also resynchronize the reported end position. The existing list layout/load callbacks refresh paused position after viewport changes and list remounts; false scroll measurements while following cannot seed a flash on the next drag.
Scope and approval
Related to the drag-at-bottom half of #12754, also reported in duplicate #13364. A maintainer identified the paused-follow visibility rule in #12754 (comment). This change is limited to that native mobile button behavior and the release/streaming cases needed to keep it accurate. It does not change the threshold for returning to live follow or affect web/desktop.
This is intended as a single, more complete alternative to #24869 for the same underlying problem: the scroll-to-bottom control must accurately represent whether unseen content remains after a bottom gesture. It includes the basic overscroll visibility fix and covers two additional release cases: transient position reports before native momentum starts, and a final streaming chunk arriving after a release at the end. The latter requires an explicit catch-up so resuming follow does not hide an outstanding gap.
Compared with #24869, this implementation reuses the existing scroll handlers, shared native content-size callback, and release timer instead of adding separate LegendList
isAtEndandtotalSizesubscriptions. This avoids those additional subscriptions; no measured runtime performance advantage is claimed. The streaming/release regression tests distinguish this work from a duplicate of the basic visibility patch. No maintainer decision to supersede or close #24869, or separate approval of this implementation, is claimed.Verification
Rebased onto upstream main at
4013183171on November 8. Resolved the feed JSX conflict by preserving main’s workspace mutation provider and current feed props, then retaining the position and catch-up callbacks. Both fix commits remain in the branch; the layout fix applied unchanged.vp test run apps/mobile/src/features/threads/thread-feed-live-follow.test.ts: 43 passed, including bottom bounce, held-finger content growth, guarded streaming updates, no-momentum release growth, momentum handoff, and catch-up exclusions. The release-window regression cases failed before the corresponding fix.vp run --filter @t3tools/mobile typecheck: passed.vp linton the four changed files: no errors; warnings remain in the existing large components.git diff --check: passed.vp run check:changed upstream/mainafter the November 8 rebase: passed formatting, lint, both knip passes, affected-package typechecks (mobile and scripts), all 43 focused tests, and all 8 selected React compiler checks. Full test suites remain with CI.Android verification completed on a Medium Phone API 36.1 emulator (Android 16, 4 GB RAM), using the fingerprint-matched development client and an isolated backend with a synthetic four-message conversation. The before capture uses the four changed files from upstream
87d22b00f; the after capture uses870561842, with the same native binary and fixture.Observed: dragging upward past the bottom shows the arrow before the fix and keeps it hidden afterward. Scrolling back into history still shows the arrow; tapping it returns to the last message and hides it. The screenshots below are frames from the recordings during the bottom gesture.
15-second comparison, before on the left, after on the right. Both use the same 700 px upward drag over 8 seconds; the clips are trimmed for comparison, not a performance benchmark.
t3-overscroll-comparison.mp4
Limitations: the Android captures above are from the pre-rebase commit
870561842; device verification was not repeated after this rebase. No iOS device verification or on-device streaming/release-race injection. Those edge cases have reducer regression coverage and source review; the Android recording demonstrates the original bottom-gesture defect and its fix. No performance benchmark was run.Implementation: GPT-6 Astra through the Codex harness in T3 Code. Peer review: Claude Opus 5.5 and Grok 4.7 through Cursor CLI.