Uh oh!
There was an error while loading. Please reload this page.
fix(mobile): keep the composer from covering the last message on thread open - #4999
fix(mobile): keep the composer from covering the last message on thread open#4999carTloyal123 wants to merge 1 commit into
Conversation
Uh oh!
There was an error while loading. Please reload this page.
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Uh oh!
There was an error while loading. Please reload this page.
ApprovabilityVerdict: Needs human review This mobile UI fix has two unresolved review comments identifying potential bugs: the scrollToEnd logic may not trigger when the composer is expanded, and it may conflict with anchor-based scrolling after message sends. These substantive issues warrant human review before merging. You can customize Macroscope's approvability policy. Learn more. |
34f0ff2 to
e4f2237CompareI haven't looked at the proposed fix myself, but this is an issue I have also noticed that bugs me. The bottom safe-area seems inconsistent depending on things like keyboard and sheet states. |
Dismissing prior approval to re-evaluate 73772b5
73772b5 to
67c9bb9CompareUh oh!
There was an error while loading. Please reload this page.
67c9bb9 to
a20141cCompareDismissing prior approval to re-evaluate a20141c
Uh oh!
There was an error while loading. Please reload this page.
a20141c to
1522d46CompareUh oh!
There was an error while loading. Please reload this page.
1522d46 to
32f6a09CompareUh oh!
There was an error while loading. Please reload this page.
32f6a09 to
0aa65aaCompare| @@ -218,6 +234,15 @@ export const ThreadDetailScreen = memo(function ThreadDetailScreen(props: Thread | |||
| Math.max(0, estimatedOverlayHeight - nativeInsetOvercount), | |||
| -nativeInsetOvercount, | |||
| ); | |||
| const handleComposerOverlayLayout = useCallback( | |||
There was a problem hiding this comment.
🟠 Highthreads/ThreadDetailScreen.tsx:237
When the composer is focused (expanded) and the feed list remounts — e.g. after a pending-approval or pending-user-input card appears or the empty→filled transition fires — ThreadFeed's corrective scrollToEnd effect never runs. The list stays positioned with a stale bottom inset and rests one composer-height short of the end. handleComposerOverlayLayout skips updating composerMeasuredHeight whenever composerExpandedRef.current is true, so the new overlay measurement (which the pending cards make larger) does not trigger the effect's props.composerMeasuredHeight dependency. Consider updating composerMeasuredHeight on every layout, or using the overlay's measured height from onComposerLayout as the effect dependency instead of the gated value.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/mobile/src/features/threads/ThreadDetailScreen.tsx around line 237:
When the composer is focused (expanded) and the feed list remounts — e.g. after a pending-approval or pending-user-input card appears or the empty→filled transition fires — `ThreadFeed`'s corrective `scrollToEnd` effect never runs. The list stays positioned with a stale bottom inset and rests one composer-height short of the end. `handleComposerOverlayLayout` skips updating `composerMeasuredHeight` whenever `composerExpandedRef.current` is true, so the new overlay measurement (which the pending cards make larger) does not trigger the effect's `props.composerMeasuredHeight` dependency. Consider updating `composerMeasuredHeight` on every layout, or using the overlay's measured height from `onComposerLayout` as the effect dependency instead of the gated value.
There was a problem hiding this comment.
Pushing back on this one — both premises do not hold, and the suggested fix (mirror on every layout) is the exact code that caused a reproduced-on-device bug. (1) The gate only skips the React-state mirror; onComposerLayout still runs on every layout, so contentInsetEndAdjustment is always current and a remounting list primes from the live measurement in the listMountKey layout effect — there is no stale-inset remount. (2) While the composer is expanded the keyboard is up, and the keyboard integration reacts to that same shared value (useExtraContentPadding), inset- and scroll-compensating overlay growth natively. Mirroring during that window made the corrective effect reportContentInset the resting overlay height while the keyboard inset was active, which pinned the feed under the composer on a real device — that regression is why the gate exists. Any residue heals on collapse: the collapse onLayout mirrors the final height and the correction runs with the keyboard closed.
There was a problem hiding this comment.
Sorry, I'm unable to act on this request because you do not have permissions within this repository.
0aa65aa to
3540214ComparecarTloyal123
commented
Jul 31, 2026
Been using this for a few hours and generally is better. There are still some other weird behaviors with threads rendering in the right place with respect to the composer box and keyboard but this is a decent start. |
…ad open Opening a remote thread could leave the last message pinned underneath the floating composer with no way to scroll to it. The feed's bottom inset comes from an async onLayout measurement of the composer overlay, but ThreadFeed remounts its list synchronously when messages arrive and primes it from whatever the shared inset value holds — while the overlay's true height (the pending approval / user-input cards animate in on that same transition) lands a frame or more later, and nothing re-scrolled afterwards. Fix, in two parts: - Correct once the real height is known: mirror the overlay's measured height into plain React state (the reanimated shared value can't trigger effects) and thread it to ThreadFeed, which distinguishes the initial prime for a fresh list from a later correction — re-reporting the inset and re-pinning to the end if the user is still near it. Also tightens the bottom > 0 guard so a zero-inset mount no longer counts as primed. - Over-reserve during the pre-measurement frames: when a pending card is knowable synchronously from props, seed the estimate with its conservative chrome so the window before the first onLayout errs tall rather than short. The cards render in normal flow inside the measured overlay, so the real measurement includes them and always overrides the seed. The "Loading/Syncing messages..." status pill is deliberately NOT part of the inset — in the estimate or the measurement. It stays an absolutely-positioned overlay: sync starts and stops on every fetch, and counting the pill would bounce the feed up and down each time it toggles. It may briefly cover the last message while visible instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
3540214 to
0a62ad9CompareThere was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 0a62ad9. Configure here.
| }, [listMountKey, props.contentInsetEndAdjustment, props.listRef]); | ||
| props.listRef.current?.reportContentInset({ bottom }); | ||
| if (!isInitialMountForKey && nearListEnd.value && !userDraggedSinceMountRef.current) { | ||
| props.listRef.current?.scrollToEnd({ animated: true }); |
There was a problem hiding this comment.
Inset correction ignores send anchor
Medium Severity
After a send, ThreadDetailScreen keeps anchorMessageId set and drives anchoredEndSpace so the list pins the sent message, but the new post-mount inset effect still calls scrollToEnd when composerMeasuredHeight changes. That scroll targets the raw list end, not the anchor, so composer remeasurement (keyboard collapse, pending cards, etc.) can override the anchor scroll while the user is still near the bottom.
Reviewed by Cursor Bugbot for commit 0a62ad9. Configure here.
What changed
Opening a remote thread could leave the last message pinned underneath the chat input bar, with no way to scroll down to it.
Why it happens
The feed's bottom inset is derived from an asynchronous measurement of the floating composer (
onComposerLayout→contentInsetEndAdjustment), not from layout flow — the composer is an absolutely-positioned overlay, so the list has to reserve space for it explicitly.Meanwhile
ThreadFeedremounts its list synchronously the moment messages arrive (theempty→filledlistMountKey) and primes the fresh instance from whatever that shared value currently holds.The composer's real height is data-dependent too. The
"Loading messages..."/"Syncing messages..."pill and the pending approval / user-input cards appear on that same transition, animated over 220ms. So the composer's true height lands a frame or more after the list has already finished its initial end-positioning against a stale, too-small inset — and nothing re-scrolls afterwards.On top of the timing race, the status pill was rendered outside layout flow (
absolute bottom-fullinside the composer), so the overlay'sonLayoutmeasurement never included it at all — the reserved inset was structurally wrong whenever the pill was visible, not just transiently.The fix
Three parts — the measurement, the correction, and the estimate each needed work:
Make the measurement truthful. The status pill now renders in normal layout flow inside the measured composer overlay (next to the pending approval/user-input cards, which already lived there), instead of absolutely positioned inside
ThreadComposer. The overlay'sonLayoutheight therefore genuinely includes everything the feed must reserve space for, and "the real measurement wins" is actually correct. This also collapses the duplicatedcomposerConnectionStatuscomputation (screen-side estimate vs composer-side render) into one call, and drops the composer props that only existed to feed the pill.Correct once the real height is known. The composer's measured height is mirrored into plain React state (the reanimated shared value can't trigger effects) and threaded to
ThreadFeed, which distinguishes the initial prime for a fresh list from a later correction. When the height changes post-mount it re-reports the inset and re-pins to the end if the user is still near it.Over-reserve during the pre-measurement frames. The seed estimate adds conservative chrome for the status pill and pending cards whenever they're knowable synchronously from props, so the brief window before the first
onLayouterrs tall rather than short. These constants are now only first-frame seeds (the standard estimated-size pattern) — correctness no longer depends on them, since the real measurement includes the pill and cards and always overrides.Also tightens the
bottom > 0guard, which previously let a zero-inset mount count as primed and skip the corrective path entirely.Notes
No visual change when things go right — the pill sits a few px higher (it now clears the composer's top padding), otherwise this only affects the failure case. Addresses the high-severity finding from Cursor Bugbot and Macroscope that the absolutely-positioned pill was excluded from the measured height.
Verification
tsc --noEmitcleanvp lintclean on changed filesBranched off current
main; the patch applies cleanly on top of #4882.Related issues
No open issue tracks this exact mobile bug, so nothing is auto-closed. Related but not closed by this PR: #4619 — the web
ChatViewhas the same architecture (composer overlay measurement feeding LegendList'scontentInsetEndAdjustment) and shows the mirror-image transient symptom (over-inset blank space at the end of long threads, self-correcting later). Its "stale composer measurement" investigation lead matches the race fixed here; the web side likely wants the same late-measurement correction.Note
Medium Risk
Touches chat list inset priming and auto scroll-to-end behavior on inset updates; wrong guards could surprise users who scrolled up, but changes are scoped to near-bottom, no-drag cases.
Overview
Fixes mobile thread open where the last message could sit under the floating composer because the feed’s bottom inset was seeded from estimates and async
onLayoutwhileThreadFeedremounts when messages arrive.ThreadDetailScreenaddsCOMPOSER_PENDING_CARD_MIN_CHROMEto the overlap estimate when pending approval or user-input cards are active, tracks collapsed composer overlay height from layout (skipping updates while the composer is keyboard-expanded), and passescomposerMeasuredHeightinto the feed.ThreadFeedreprimes LegendList’s bottom inset on mount and when that measured height changes: initial mount for alistMountKeyonly reports the inset; later updates scroll to end if the user is still near the bottom and hasn’t dragged since mount.onScrollBeginDragsets that drag guard.Reviewed by Cursor Bugbot for commit 0a62ad9. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Fix thread composer covering the last message on thread open
composerExtraChromevalue in ThreadDetailScreen.tsx that addsCOMPOSER_PENDING_CARD_MIN_CHROME(160px) per visible pending card (approval or user input) to the bottom overlap/inset.Macroscope summarized 0a62ad9.