-
Notifications
You must be signed in to change notification settings - Fork 3.5k
fix(mobile): keep the composer from covering the last message on thread open #4999
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,7 +16,12 @@ import type { | |
| } from "@t3tools/contracts"; | ||
| import * as Haptics from "expo-haptics"; | ||
| import { memo, useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from "react"; | ||
| import { Platform, View, type GestureResponderEvent } from "react-native"; | ||
| import { | ||
| Platform, | ||
| View, | ||
| type GestureResponderEvent, | ||
| type LayoutChangeEvent, | ||
| } from "react-native"; | ||
| import { KeyboardController, KeyboardStickyView } from "react-native-keyboard-controller"; | ||
| import Animated, { FadeInDown, FadeOut } from "react-native-reanimated"; | ||
| import { useSafeAreaInsets } from "react-native-safe-area-context"; | ||
|
|
@@ -37,6 +42,7 @@ import { PendingUserInputCard } from "./PendingUserInputCard"; | |
| import { | ||
| COMPOSER_COLLAPSED_CHROME, | ||
| COMPOSER_EXPANDED_CHROME, | ||
| COMPOSER_PENDING_CARD_MIN_CHROME, | ||
| ThreadComposer, | ||
| } from "./ThreadComposer"; | ||
| import { ThreadFeed } from "./ThreadFeed"; | ||
|
|
@@ -180,7 +186,13 @@ export const ThreadDetailScreen = memo(function ThreadDetailScreen(props: Thread | |
| const selectedThreadKeyRef = useRef(selectedThreadKey); | ||
| const lastScrolledAnchorMessageIdRef = useRef<MessageId | null>(null); | ||
| const [composerExpanded, setComposerExpanded] = useState(false); | ||
| const composerExpandedRef = useRef(false); | ||
| const handleComposerExpandedChange = useCallback((expanded: boolean) => { | ||
| composerExpandedRef.current = expanded; | ||
| setComposerExpanded(expanded); | ||
| }, []); | ||
| const [anchorMessageId, setAnchorMessageId] = useState<MessageId | null>(null); | ||
| const [composerMeasuredHeight, setComposerMeasuredHeight] = useState<number | null>(null); | ||
| const composerBottomInset = composerExpanded ? 0 : Math.max(insets.bottom, 12); | ||
| const contentPresentationKind = props.contentPresentation.kind; | ||
| // The raw sync status enters "synchronizing" on every full fetch, cached or | ||
|
|
@@ -201,7 +213,11 @@ export const ThreadDetailScreen = memo(function ThreadDetailScreen(props: Thread | |
| })(); | ||
| const selectedThreadFeed = props.selectedThreadFeed; | ||
| const composerChrome = composerExpanded ? COMPOSER_EXPANDED_CHROME : COMPOSER_COLLAPSED_CHROME; | ||
| const composerOverlapHeight = composerChrome + composerBottomInset; | ||
| const composerExtraChrome = | ||
| ((props.activePendingApproval === null ? 0 : 1) + | ||
| (props.activePendingUserInput === null ? 0 : 1)) * | ||
| COMPOSER_PENDING_CARD_MIN_CHROME; | ||
| const composerOverlapHeight = composerChrome + composerExtraChrome + composerBottomInset; | ||
| const estimatedOverlayHeight = composerOverlapHeight; | ||
| // The overlay's measured height includes the home-indicator inset (the | ||
| // composer pads it), but contentInsetAdjustmentBehavior="automatic" makes | ||
|
|
@@ -218,6 +234,16 @@ export const ThreadDetailScreen = memo(function ThreadDetailScreen(props: Thread | |
| Math.max(0, estimatedOverlayHeight - nativeInsetOvercount), | ||
| -nativeInsetOvercount, | ||
| ); | ||
| const handleComposerOverlayLayout = useCallback( | ||
|
macroscopeapp[bot] marked this conversation as resolved.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 High 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 — 🤖 Copy this AI Prompt to have your agent fix this:
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, I'm unable to act on this request because you do not have permissions within this repository. |
||
| (event: LayoutChangeEvent) => { | ||
| onComposerLayout(event); | ||
| // expanded = keyboard transition, which manages its own inset | ||
| if (!composerExpandedRef.current) { | ||
| setComposerMeasuredHeight(event.nativeEvent.layout.height); | ||
| } | ||
| }, | ||
| [onComposerLayout], | ||
| ); | ||
| const { freeze, scrollMessageToEnd } = useKeyboardScrollToEnd({ listRef }); | ||
| const showContent = props.showContent ?? true; | ||
| const layoutVariant = props.layoutVariant ?? "compact"; | ||
|
|
@@ -240,6 +266,7 @@ export const ThreadDetailScreen = memo(function ThreadDetailScreen(props: Thread | |
| setAnchorMessageId(null); | ||
| lastScrolledAnchorMessageIdRef.current = null; | ||
| freeze.set(false); | ||
| setComposerMeasuredHeight(null); | ||
| }, [freeze, selectedThreadKey]); | ||
|
|
||
| useEffect(() => { | ||
|
|
@@ -364,6 +391,7 @@ export const ThreadDetailScreen = memo(function ThreadDetailScreen(props: Thread | |
| freeze={freeze} | ||
| anchorMessageId={anchorMessageId} | ||
| contentInsetEndAdjustment={contentInsetEndAdjustment} | ||
| composerMeasuredHeight={composerMeasuredHeight} | ||
| contentTopInset={0} | ||
| contentBottomInset={estimatedOverlayHeight} | ||
| contentMaxWidth={contentMaxWidth} | ||
|
|
@@ -386,7 +414,7 @@ export const ThreadDetailScreen = memo(function ThreadDetailScreen(props: Thread | |
| {/* No paddingTop here: the overlay's measured height becomes the | ||
| list's bottom inset, so any padding above the pill/composer | ||
| pushes the resting content floor up by the same amount. */} | ||
| <View ref={composerOverlayRef} onLayout={onComposerLayout} className="w-full"> | ||
| <View ref={composerOverlayRef} onLayout={handleComposerOverlayLayout} className="w-full"> | ||
| <View className="w-full self-center" style={{ maxWidth: contentMaxWidth }}> | ||
| {props.activePendingApproval || props.activePendingUserInput ? ( | ||
| <Animated.View | ||
|
|
@@ -443,7 +471,7 @@ export const ThreadDetailScreen = memo(function ThreadDetailScreen(props: Thread | |
| onUpdateModelSelection={props.onUpdateThreadModelSelection} | ||
| onUpdateRuntimeMode={props.onUpdateThreadRuntimeMode} | ||
| onUpdateInteractionMode={props.onUpdateThreadInteractionMode} | ||
| onExpandedChange={setComposerExpanded} | ||
| onExpandedChange={handleComposerExpandedChange} | ||
| /> | ||
| </View> | ||
| </KeyboardStickyView> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -157,6 +157,7 @@ export interface ThreadFeedProps { | |
| readonly freeze: SharedValue<boolean>; | ||
| readonly anchorMessageId: MessageId | null; | ||
| readonly contentInsetEndAdjustment: SharedValue<number>; | ||
| readonly composerMeasuredHeight?: number | null; | ||
| readonly contentTopInset?: number; | ||
| readonly contentBottomInset?: number; | ||
| readonly contentMaxWidth?: number; | ||
|
|
@@ -1438,6 +1439,10 @@ export const ThreadFeed = memo(function ThreadFeed(props: ThreadFeedProps) { | |
| // only region where layout shifts should animate. Starts true because the | ||
| // list opens pinned to the end. | ||
| const nearListEnd = useSharedValue(true); | ||
| const userDraggedSinceMountRef = useRef(false); | ||
| const handleScrollBeginDrag = useCallback(() => { | ||
| userDraggedSinceMountRef.current = true; | ||
| }, []); | ||
|
|
||
| const handleScroll = useCallback( | ||
| (event: NativeSyntheticEvent<NativeScrollEvent>) => { | ||
|
|
@@ -1526,12 +1531,29 @@ export const ThreadFeed = memo(function ThreadFeed(props: ThreadFeedProps) { | |
| // 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"}`; | ||
| const lastReprimedMountKeyRef = useRef<string | null>(null); | ||
| useLayoutEffect(() => { | ||
|
macroscopeapp[bot] marked this conversation as resolved.
|
||
| const bottom = props.contentInsetEndAdjustment.value; | ||
| if (bottom > 0) { | ||
| props.listRef.current?.reportContentInset({ bottom }); | ||
| const isInitialMountForKey = lastReprimedMountKeyRef.current !== listMountKey; | ||
| lastReprimedMountKeyRef.current = listMountKey; | ||
| if (isInitialMountForKey) { | ||
| nearListEnd.value = true; | ||
| userDraggedSinceMountRef.current = false; | ||
| } | ||
| if (bottom <= 0) { | ||
| return; | ||
| } | ||
| }, [listMountKey, props.contentInsetEndAdjustment, props.listRef]); | ||
| props.listRef.current?.reportContentInset({ bottom }); | ||
| if (!isInitialMountForKey && nearListEnd.value && !userDraggedSinceMountRef.current) { | ||
| props.listRef.current?.scrollToEnd({ animated: true }); | ||
|
cursor[bot] marked this conversation as resolved.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Inset correction ignores send anchorMedium Severity After a send, Reviewed by Cursor Bugbot for commit 0a62ad9. Configure here. |
||
| } | ||
| }, [ | ||
| listMountKey, | ||
| props.composerMeasuredHeight, | ||
| props.contentInsetEndAdjustment, | ||
| props.listRef, | ||
| nearListEnd, | ||
| ]); | ||
|
|
||
| const anchoredEndSpace = useMemo( | ||
| () => | ||
|
|
@@ -1891,6 +1913,7 @@ export const ThreadFeed = memo(function ThreadFeed(props: ThreadFeedProps) { | |
| alignItemsAtEnd | ||
| initialScrollAtEnd | ||
| onScroll={handleScroll} | ||
| onScrollBeginDrag={handleScrollBeginDrag} | ||
| scrollEventThrottle={16} | ||
| ListHeaderComponent={ | ||
| usesNativeAutomaticInsets ? null : <View style={{ height: topContentInset }} /> | ||
|
|
||


Uh oh!
There was an error while loading. Please reload this page.