Fix mobile legend anchor under automatic iOS insets#3684
Fix mobile legend anchor under automatic iOS insets#3684juliusmarminge wants to merge 7 commits into
Conversation
|
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 Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
vouch:trusted
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
ApprovabilityVerdict: Needs human review 7 blocking correctness issues found. This PR modifies mobile scroll positioning and content inset handling through extensive third-party library patches and new animation logic. Multiple unresolved review comments identify potential issues, including a high-severity concern about Android production releases being misdirected to internal testing. You can customize Macroscope's approvability policy. Learn more. |
github-actions
Bot
added
size:XL
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Header blur uses wrong inset
- handleScroll now measures the scroll threshold with anchorTopInset instead of the zeroed topContentInset, so header material visibility toggles at the true rest position under iOS automatic content insets while non-automatic behavior is unchanged.
Or push these changes by commenting:
@cursor push 18fb3f23d9
Preview (18fb3f23d9)
diff --git a/apps/mobile/src/features/threads/ThreadFeed.tsx b/apps/mobile/src/features/threads/ThreadFeed.tsx
--- a/apps/mobile/src/features/threads/ThreadFeed.tsx
+++ b/apps/mobile/src/features/threads/ThreadFeed.tsx
@@ -1282,11 +1282,15 @@
},
[props.onHeaderMaterialVisibilityChange],
);
+ // anchorTopInset, not topContentInset: with automatic insets the list rests
+ // at contentOffset.y = -headerHeight (the inset only exists in UIKit's
+ // adjustedContentInset), so the header inset must be added back for the
+ // material to toggle relative to the true rest position.
const handleScroll = useCallback(
(event: NativeSyntheticEvent<NativeScrollEvent>) => {
- reportHeaderMaterialVisibility(event.nativeEvent.contentOffset.y + topContentInset > 6);
+ reportHeaderMaterialVisibility(event.nativeEvent.contentOffset.y + anchorTopInset > 6);
},
- [reportHeaderMaterialVisibility, topContentInset],
+ [reportHeaderMaterialVisibility, anchorTopInset],
);
const handleViewportLayout = useCallback((event: LayoutChangeEvent) => {
const nextWidth = Math.round(event.nativeEvent.layout.width);You can send follow-ups to the cloud agent here.
github-actions
Bot
added
size:XXL
- Fold header inset into the anchor offset when iOS automatic content insets are enabled - Prevent new messages from anchoring under the header and snapping past earlier messages
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Work pill height estimate stale
- Increased WORKING_INDICATOR_HEIGHT from 44 to 52 so the overlay height estimate includes the pill wrapper's new pt-2 (8px) padding, keeping the pre-measurement inset math consistent with the measured layout.
Or push these changes by commenting:
@cursor push 8b598399e0
Preview (8b598399e0)
diff --git a/apps/mobile/src/features/threads/ThreadDetailScreen.tsx b/apps/mobile/src/features/threads/ThreadDetailScreen.tsx
--- a/apps/mobile/src/features/threads/ThreadDetailScreen.tsx
+++ b/apps/mobile/src/features/threads/ThreadDetailScreen.tsx
@@ -174,7 +174,10 @@
}, [threadId, feed]);
}
-const WORKING_INDICATOR_HEIGHT = 44;
+// Estimated height of WorkingDurationPill including its pt-2/pb-2 wrapper
+// padding; must track the pill layout so the pre-measurement overlay height
+// estimate matches what onComposerLayout later reports.
+const WORKING_INDICATOR_HEIGHT = 52;
const WorkingDurationPill = memo(function WorkingDurationPill(props: {
readonly startedAt: string;You can send follow-ups to the cloud agent here.
- Patch LegendList and keyboard inset handling for transparent headers - Animate composer, feed, and work-log transitions more smoothly - Add tests and dependency patches for the mobile scroll fixes
| // composer-height short of the end. Layout effect: it must land before the | ||
| // list's first positioning tick or the one-shot initial scroll misses it. | ||
| const listMountKey = `${props.threadId}:${props.feed.length === 0 ? "empty" : "filled"}`; | ||
| useLayoutEffect(() => { |
There was a problem hiding this comment.
🟡 Medium threads/ThreadFeed.tsx:1330
The new useLayoutEffect keyed on listMountKey reads props.contentInsetEndAdjustment.value and reports it to the remounted list, but that shared value still holds the previous thread's composer-overlay height when the thread changes (because useKeyboardChatComposerInset deduplicates by height and hasn't re-measured the fresh overlay yet). initialScrollAtEnd is one-shot during attach, so the first scroll-to-end on a thread switch computes with a stale bottom inset and rests too high or too low by the overlay-height delta. Consider resetting contentInsetEndAdjustment.value to 0 on thread change before the list remounts, or guarding the effect so it only reports the inset once the composer overlay has re-measured for the current thread.
Also found in 1 other location(s)
apps/mobile/src/features/threads/ThreadDetailScreen.tsx:195
Adding
pt-2toWorkingDurationPillmakes the overlay 8 px taller, but the parent still budgets a fixedWORKING_INDICATOR_HEIGHT = 44whenactiveWorkStartedAtis set. That leaves the thread feed's estimated bottom inset/anchor space too small whenever this pill is shown, so the last message or empty-state content can end up partially underneath the floating overlay until later layout correction.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/features/threads/ThreadFeed.tsx around line 1330:
The new `useLayoutEffect` keyed on `listMountKey` reads `props.contentInsetEndAdjustment.value` and reports it to the remounted list, but that shared value still holds the previous thread's composer-overlay height when the thread changes (because `useKeyboardChatComposerInset` deduplicates by height and hasn't re-measured the fresh overlay yet). `initialScrollAtEnd` is one-shot during attach, so the first scroll-to-end on a thread switch computes with a stale bottom inset and rests too high or too low by the overlay-height delta. Consider resetting `contentInsetEndAdjustment.value` to 0 on thread change before the list remounts, or guarding the effect so it only reports the inset once the composer overlay has re-measured for the current thread.
Also found in 1 other location(s):
- apps/mobile/src/features/threads/ThreadDetailScreen.tsx:195 -- Adding `pt-2` to `WorkingDurationPill` makes the overlay 8 px taller, but the parent still budgets a fixed `WORKING_INDICATOR_HEIGHT = 44` when `activeWorkStartedAt` is set. That leaves the thread feed's estimated bottom inset/anchor space too small whenever this pill is shown, so the last message or empty-state content can end up partially underneath the floating overlay until later layout correction.
- Pass safe-area inset compensation through the mobile thread list - Adjust keyboard controller and legend list math so end anchors stay aligned - Fix overlay spacing and drag handling to prevent end-position drift
juliusmarminge
force-pushed
the
t3code/fix-mobile-legend-anchor
branch
from
8bcfdef to
60867bd
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Bugbot Autofix is ON. A cloud agent has been kicked off to fix the reported issue. You can view the agent here.
Reviewed by Cursor Bugbot for commit 60867bd. Configure here.
There was a problem hiding this comment.
🟡 Medium
WorkingDurationPill now adds pt-2 (8px top padding), but WORKING_INDICATOR_HEIGHT is still 44. ThreadDetailScreen uses this constant for activeWorkIndicatorHeight, which feeds into estimatedOverlayHeight, contentInsetEndAdjustment, and the feed's contentBottomInset. The reserved overlay height no longer matches the pill's actual rendered height, so the feed's bottom inset is ~8px too short and the last content can slide under the floating pill. Update WORKING_INDICATOR_HEIGHT to reflect the added top padding, or derive the reserved height from the pill's measured layout instead of a hardcoded constant.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/features/threads/ThreadDetailScreen.tsx around line 174:
`WorkingDurationPill` now adds `pt-2` (8px top padding), but `WORKING_INDICATOR_HEIGHT` is still `44`. `ThreadDetailScreen` uses this constant for `activeWorkIndicatorHeight`, which feeds into `estimatedOverlayHeight`, `contentInsetEndAdjustment`, and the feed's `contentBottomInset`. The reserved overlay height no longer matches the pill's actual rendered height, so the feed's bottom inset is ~8px too short and the last content can slide under the floating pill. Update `WORKING_INDICATOR_HEIGHT` to reflect the added top padding, or derive the reserved height from the pill's measured layout instead of a hardcoded constant.
| onMomentumScrollEnd(event); | ||
| } | ||
| }, | ||
| + onScrollBeginDrag: (event) => { |
There was a problem hiding this comment.
🟡 Medium patches/@legendapp__list@3.2.0.patch:481
The onScrollBeginDrag handler is forwarded through the fns object, but fns is created with an empty dependency array []. After mount, updates to the onScrollBeginDrag prop are never captured — drags always invoke the callback captured on first render, so any parent state that the drag callback depends on goes stale. Consider adding the prop (or a ref holding it) to the dependency array so the handler stays current.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @patches/@legendapp__list@3.2.0.patch around line 481:
The `onScrollBeginDrag` handler is forwarded through the `fns` object, but `fns` is created with an empty dependency array `[]`. After mount, updates to the `onScrollBeginDrag` prop are never captured — drags always invoke the callback captured on first render, so any parent state that the drag callback depends on goes stale. Consider adding the prop (or a ref holding it) to the dependency array so the handler stays current.
| + blankSpace.value = size > 0 ? Math.max(0, size - (adjustedInsetCompensation || 0)) : 0; | ||
| (_a = anchoredEndSpace.onSizeChanged) == null ? void 0 : _a.call(anchoredEndSpace, size); |
There was a problem hiding this comment.
🟡 Medium patches/@legendapp__list@3.2.0.patch:74
onSizeChanged receives the original unadjusted size, but the spacer actually applied to blankSpace.value is size - adjustedInsetCompensation. Any caller tracking the real anchored spacer via onSizeChanged will be off by the compensation amount, causing overlays or end-of-list calculations to drift by the safe-area inset. Pass the same adjusted value to onSizeChanged that is written to blankSpace.value.
| + blankSpace.value = size > 0 ? Math.max(0, size - (adjustedInsetCompensation || 0)) : 0; | |
| (_a = anchoredEndSpace.onSizeChanged) == null ? void 0 : _a.call(anchoredEndSpace, size); | |
| blankSpace.value = size > 0 ? Math.max(0, size - (adjustedInsetCompensation || 0)) : 0; | |
| const adjustedSize = blankSpace.value; | |
| (_a = anchoredEndSpace.onSizeChanged) == null ? void 0 : _a.call(anchoredEndSpace, adjustedSize); |
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @patches/@legendapp__list@3.2.0.patch around lines 74-75:
`onSizeChanged` receives the original unadjusted `size`, but the spacer actually applied to `blankSpace.value` is `size - adjustedInsetCompensation`. Any caller tracking the real anchored spacer via `onSizeChanged` will be off by the compensation amount, causing overlays or end-of-list calculations to drift by the safe-area inset. Pass the same adjusted value to `onSizeChanged` that is written to `blankSpace.value`.
| <ControlPillMenu | ||
| actions={optionsMenuActions} | ||
| onPressAction={({ nativeEvent }) => handleOptionsMenuAction(nativeEvent.event)} | ||
| <Animated.View entering={FadeIn.duration(160)} exiting={FadeOut.duration(120)}> |
There was a problem hiding this comment.
🟡 Medium threads/ThreadComposer.tsx:843
The exiting={FadeOut} wrappers on the expanded-only sections (toolbar row, queue count, attachment strip) keep those elements mounted and occupying space until the exit animation finishes (~120–180 ms). ThreadDetailScreen measures the overlay height via onComposerLayout and uses it for the feed's bottom inset and anchor-scroll math, so when the composer collapses after send the measured height still reflects the expanded layout. This causes the feed to compute its resting position against the wrong inset and anchor the newly sent message too high relative to the collapsed composer. Consider removing the exiting animations on these sections (or using a layout-only transition) so the height measurement updates immediately when isExpanded flips to false.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/features/threads/ThreadComposer.tsx around line 843:
The `exiting={FadeOut}` wrappers on the expanded-only sections (toolbar row, queue count, attachment strip) keep those elements mounted and occupying space until the exit animation finishes (~120–180 ms). `ThreadDetailScreen` measures the overlay height via `onComposerLayout` and uses it for the feed's bottom inset and anchor-scroll math, so when the composer collapses after send the measured height still reflects the expanded layout. This causes the feed to compute its resting position against the wrong inset and anchor the newly sent message too high relative to the collapsed composer. Consider removing the `exiting` animations on these sections (or using a layout-only transition) so the height measurement updates immediately when `isExpanded` flips to false.
Gate LegendList's readyToRender on a new insetEndRevealHold flag owned by the inset-end settle watchdog: content stays hidden while the initial end-scroll chases late inset/size reports, upgrades adaptiveRender to normal while still hidden (the light->normal re-render wave was causing ~100ms of visible drift), then reveals only after the offset holds stable for 3 consecutive frames (40-frame cap, released early on user drag). First visible frame is the final resting position. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
| + const maintainAnchoredEndSpace = state.props.anchoredEndSpace; | ||
| + const maintainAnchorIndex = maintainAnchoredEndSpace == null ? void 0 : maintainAnchoredEndSpace.anchorIndex; | ||
| + if (maintainAnchorIndex !== void 0 && maintainAnchorIndex >= 0 && maintainAnchorIndex < state.props.data.length && !areKnownOrFixedItemSizesAvailable(ctx, maintainAnchorIndex, state.props.data.length - 1)) { | ||
| + return false; |
There was a problem hiding this comment.
🟡 Medium patches/@legendapp__list@3.2.0.patch:277
When contentSize < state.scrollLength and maintainInsetStartAdjustment > 0, doMaintainScrollAtEnd sets state.scroll = -maintainInsetStartAdjustment and returns true without calling scrollTo on the native scroller. So when a list shrinks from overflowing to underflow while maintainScrollAtEnd is active, the internal scroll state says the feed is at -headerInset but the native scroll view stays at its old offset, leaving the viewport and end-anchor out of sync. Consider dispatching a scrollTo({ animated, y: -maintainInsetStartAdjustment }) (or falling through to the normal scrollTo path) so the native scroller matches state.scroll.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @patches/@legendapp__list@3.2.0.patch around line 277:
When `contentSize < state.scrollLength` and `maintainInsetStartAdjustment > 0`, `doMaintainScrollAtEnd` sets `state.scroll = -maintainInsetStartAdjustment` and returns `true` without calling `scrollTo` on the native scroller. So when a list shrinks from overflowing to underflow while `maintainScrollAtEnd` is active, the internal scroll state says the feed is at `-headerInset` but the native scroll view stays at its old offset, leaving the viewport and end-anchor out of sync. Consider dispatching a `scrollTo({ animated, y: -maintainInsetStartAdjustment })` (or falling through to the normal `scrollTo` path) so the native scroller matches `state.scroll`.
- Point production iOS submit config at the new ASC app ID - Keeps EAS submission aligned with the current app record
- Update `apps/mobile/eas.json` so production Android builds target the internal track - Leave iOS release config unchanged
| "ascAppId": "6787819824" | ||
| }, | ||
| "android": { | ||
| "track": "internal" |
There was a problem hiding this comment.
🟠 High mobile/eas.json:52
The submit.production.android.track is set to "internal", so eas submit --profile production -p android uploads the release to Google Play's internal testing track instead of the production track. Because production is the only Android submit profile, production Android releases never reach end users — they go only to internal testers. If internal testing is intended as a staging step before promotion, consider documenting that workflow; otherwise set track to "production" (or remove it so the default production track is used).
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/eas.json around line 52:
The `submit.production.android.track` is set to `"internal"`, so `eas submit --profile production -p android` uploads the release to Google Play's internal testing track instead of the production track. Because `production` is the only Android submit profile, production Android releases never reach end users — they go only to internal testers. If internal testing is intended as a staging step before promotion, consider documenting that workflow; otherwise set `track` to `"production"` (or remove it so the default production track is used).
Address two PR review findings on the automatic-inset work: - ThreadFeed: the header-material scroll check used topContentInset (0 under automatic insets) instead of anchorTopInset, so the header blur toggled a full header height too late. Use anchorTopInset to match the true rest position. - ThreadDetailScreen: WORKING_INDICATOR_HEIGHT (44) no longer matched the pill's rendered height after the pt-2 padding was added; bump to 52 so the pre-measurement overlay estimate seeds an accurate bottom inset. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 participant
Summary
maintainScrollAtEndfrom snapping to the wrong position when UIKit-managed insets are in play.Testing
apps/mobile/src/features/threads/ThreadFeed.tsxanchor calculations.Note
High Risk
Touches core iOS scroll, inset, and keyboard integration via vendor patches and complex list remount/anchor behavior; regressions could affect every thread send and keyboard interaction, though scope is mobile UI only.
Overview
iOS thread scrolling is reworked so
KeyboardAwareLegendListbehaves correctly with transparent headers andcontentInsetAdjustmentBehavior="automatic": anchor offset now includes navigation header height, bottom inset math compensates for UIKit double-counting the home indicator, and the list gains patched props (contentInsetStartAdjustment,contentInsetEndStaticAdjustment,adjustedInsetCompensation,scrollToOverflowEnabled) plus animatedmaintainScrollAtEndand a layout effect to re-report composer inset after empty→filled remounts.Thread detail / composer under-reports measured overlay height by the safe-area bottom (fourth arg to
useKeyboardChatComposerInset), drops extra overlaypaddingTop, and animates the working pill and pending approval/input chrome.UX polish: Reanimated layout and fade transitions on the composer, feed messages (fresh rows only), and work-log rows.
Dependencies: Large pnpm patches for
@legendapp/list@3.2.0(scroll clamp, initial scroll, anchored end space, reveal hold) andreact-native-keyboard-controller@1.21.13(adjustedInsetCompensation, inset mirroring; scroll indicators follow keyboard only).Release:
eas.jsonproduction submit uses a new iOSascAppIdand sets Androidtracktointernal.Reviewed by Cursor Bugbot for commit 1b94454. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Fix mobile legend anchor position under automatic iOS safe-area insets
adjustedInsetCompensationprop toKeyboardChatScrollView(via a patch toreact-native-keyboard-controller) so keyboard/scroll math accounts for the safe-area bottom inset on iOS when automatic content insets are active.useKeyboardChatComposerInset, preventing the legend anchor from being displaced by the double-counted inset.HeaderHeightContextto computeanchorTopInsetand passescontentInsetStartAdjustment,contentInsetEndStaticAdjustment, andadjustedInsetCompensationtoKeyboardAwareLegendListwhen automatic insets are in use.Macroscope summarized 1b94454.