Merge pull request #94470 from Expensify/revert-93403-VickyStash/feature/open-linked-message-centered

mountiny · web-flow · commit b9c9d623c892 · 2026-06-24T16:43:14.000+02:00
[CP Staging] Revert "Open linked message at top"
diff --git a/patches/@shopify/flash-list/@shopify+flash-list+2.3.0+013+improve-scroll-key-handling.patch b/patches/@shopify/flash-list/@shopify+flash-list+2.3.0+013+improve-scroll-key-handling.patch
diff --git a/patches/@shopify/flash-list/details.md b/patches/@shopify/flash-list/details.md
@@ -141,15 +141,3 @@
 - Upstream PR/issue: https://github.com/Shopify/flash-list/issues/2334
 - E/App issue: https://github.com/Expensify/App/issues/91584, https://github.com/Expensify/App/issues/92263
 - PR introducing patch: https://github.com/Expensify/App/pull/92520
-
-### [@shopify+flash-list+2.3.0+013+improve-scroll-key-handling.patch](@shopify+flash-list+2.3.0+013+improve-scroll-key-handling.patch)
-
-- Reason: Adds `viewPosition` support to `initialScrollIndexParams` (0 = start, 0.5 = center, 1 = end — same semantics as `scrollToIndex`'s `viewPosition`). Four changes:
-  1. **`applyInitialScrollIndex`** in `useRecyclerViewController.js`: the corrective scroll for `initialScrollIndex` now shifts the target offset by `(containerSize - itemSize) * viewPosition` (clamped to ≥ 0, and skipped while the container is unmeasured), mirroring `scrollToIndex`'s math.
-  2. **`applyInitialScrollAdjustment`** in `RecyclerViewManager.js`: the initial render window is anchored with the same `viewPosition` adjustment, so the very first painted frame already renders the items around the centered position — without this, the first frame renders items from the target's raw offset (target at the viewport edge) and visibly jumps once the first corrective scroll lands.
-  3. **Bottom crop** in `applyInitialScrollIndex` (`useRecyclerViewController.js`): for inverted vertical lists positioned via `viewPosition`, when the bottom-most visible item is flush against the bottom edge and another item exists underneath it, the offset is nudged up so the current bottom item is cropped by a few pixels — signaling there is more content below.
-  4. **`recomputeLayouts` range** in `applyInitialScrollAdjustment` (`RecyclerViewManager.js`): the recompute that precedes reading the target offset is widened from `recomputeLayouts(0, initialScrollIndex)` to `recomputeLayouts(0, this.getDataLength() - 1)`, so every item gets a measured/re-estimated layout before the positioning.
-- Files changed: `dist/FlashListProps.d.ts`, `dist/recyclerview/hooks/useRecyclerViewController.js`, `dist/recyclerview/RecyclerViewManager.js`.
-- Upstream PR/issue: https://github.com/Shopify/flash-list/pull/2318 (for point 4)
-- E/App issue: https://github.com/Expensify/App/issues/92152
-- PR introducing patch: https://github.com/Expensify/App/pull/93403
diff --git a/src/CONST/index.ts b/src/CONST/index.ts
@@ -1839,7 +1839,6 @@ const CONST = {
             THREAD_DISABLED: ['CREATED'],
             LATEST_MESSAGES_PILL_SCROLL_OFFSET_THRESHOLD: 2000,
             ACTION_VISIBLE_THRESHOLD: 250,
-            LINKED_MESSAGE_OFFSET: 40,
             MAX_GROUPING_TIME: 300000,
         },
         CANCEL_PAYMENT_REASONS: {
diff --git a/src/components/FlashList/InvertedFlashList/index.tsx b/src/components/FlashList/InvertedFlashList/index.tsx
@@ -1,10 +1,14 @@
 import type {FlashListProps} from '@shopify/flash-list';
 import React from 'react';
+import useFlashListScrollKey from '@components/FlashList/useFlashListScrollKey';
 import type {FlatListRefType} from '@pages/inbox/ReportScreenContext';
 import FlashList from '..';
 import CellRendererComponent from './CellRendererComponent';
 
 type InvertedFlashListProps<T> = FlashListProps<T> & {
+    /** Key of the item to initially scroll to when the list first renders. */
+    initialScrollKey?: string | null;
+
     /** The array of items to render in the list. */
     data: T[];
 
@@ -13,14 +17,48 @@ type InvertedFlashListProps<T> = FlashListProps<T> & {
 
     /** Ref to the underlying list instance. */
     ref: FlatListRefType;
+
+    /** Whether the list should handle `maintainVisibleContentPosition` */
+    shouldMaintainVisibleContentPosition?: boolean;
 };
 
-function InvertedFlashList<T>(props: InvertedFlashListProps<T>) {
+function InvertedFlashList<T>({
+    data,
+    keyExtractor,
+    initialScrollKey,
+    onStartReached: onStartReachedProp,
+    maintainVisibleContentPosition: maintainVisibleContentPositionProp,
+    shouldMaintainVisibleContentPosition,
+    ...restProps
+}: InvertedFlashListProps<T>) {
+    const {
+        displayedData,
+        onStartReached,
+        maintainVisibleContentPosition: maintainVisibleContentPositionForScrollKey,
+    } = useFlashListScrollKey<T>({
+        data,
+        keyExtractor,
+        initialScrollKey,
+        onStartReached: onStartReachedProp,
+        shouldMaintainVisibleContentPosition,
+    });
+
+    const maintainVisibleContentPosition = maintainVisibleContentPositionProp
+        ? {
+              ...maintainVisibleContentPositionForScrollKey,
+              ...maintainVisibleContentPositionProp,
+          }
+        : maintainVisibleContentPositionForScrollKey;
+
     return (
         <FlashList<T>
-            {...props}
+            {...restProps}
             inverted
+            onStartReached={onStartReached}
+            data={displayedData}
+            keyExtractor={keyExtractor}
             CellRendererComponent={CellRendererComponent}
+            maintainVisibleContentPosition={maintainVisibleContentPosition}
         />
     );
 }
diff --git a/src/components/FlashList/useFlashListScrollKey.ts b/src/components/FlashList/useFlashListScrollKey.ts
@@ -0,0 +1,64 @@
+import type {FlashListProps} from '@shopify/flash-list';
+import {useEffect, useState} from 'react';
+
+type FlashListScrollKeyProps<T> = {
+    /** The array of items to render in the list. */
+    data: T[];
+
+    /** Function that extracts a unique key for each item in the list. */
+    keyExtractor: (item: T, index: number) => string;
+
+    /** Key of the item to initially scroll to when the list first renders. */
+    initialScrollKey: string | null | undefined;
+
+    /** Callback invoked when the user scrolls close to the start of the list. */
+    onStartReached: FlashListProps<T>['onStartReached'];
+
+    /** Whether the list should handle `maintainVisibleContentPosition` */
+    shouldMaintainVisibleContentPosition?: boolean;
+};
+
+export default function useFlashListScrollKey<T>({data, keyExtractor, initialScrollKey, onStartReached, shouldMaintainVisibleContentPosition}: FlashListScrollKeyProps<T>) {
+    const [isInitialRender, setIsInitialRender] = useState(!!initialScrollKey);
+    const [hasLinkingSettled, setHasLinkingSettled] = useState(!initialScrollKey);
+
+    // Two-frame handoff for deep-link:
+    // RAF 1: switch from sliced data to the full array — FlashList's default MVCP pins the
+    //        linked item through the data swap.
+    // RAF 2: pinning has happened, disable MVCP so it doesn't cause later jumps.
+    useEffect(() => {
+        if (!isInitialRender) {
+            return;
+        }
+
+        // Without an anchor on this frame, we are not doing the deep-link slice handoff; clear the flag so a key that
+        // appears later (e.g. marking a message unread) cannot reuse the "first paint" slice path.
+        if (!initialScrollKey) {
+            // If the initial scroll key gets unset, we need to disable the initial render flag,
+            // otherwise the list will not render..
+            // eslint-disable-next-line react-hooks/set-state-in-effect
+            setIsInitialRender(false);
+            return;
+        }
+
+        requestAnimationFrame(() => {
+            setIsInitialRender(false);
+            requestAnimationFrame(() => setHasLinkingSettled(true));
+        });
+    }, [isInitialRender, initialScrollKey]);
+
+    const maintainVisibleContentPosition: FlashListProps<T>['maintainVisibleContentPosition'] = {disabled: !shouldMaintainVisibleContentPosition && hasLinkingSettled};
+
+    if (!isInitialRender || !initialScrollKey) {
+        return {displayedData: data, onStartReached, maintainVisibleContentPosition};
+    }
+
+    const targetIndex = data.findIndex((item, index) => keyExtractor(item, index) === initialScrollKey);
+    if (targetIndex <= 0) {
+        return {displayedData: data, onStartReached, maintainVisibleContentPosition};
+    }
+
+    // On the first render, slice from the target onward so the target item
+    // appears at the visual bottom of the inverted list — no scrolling needed.
+    return {displayedData: data.slice(targetIndex), onStartReached: () => {}, maintainVisibleContentPosition};
+}
diff --git a/src/pages/inbox/report/ReportActionsList.tsx b/src/pages/inbox/report/ReportActionsList.tsx
@@ -359,8 +359,22 @@ function ReportActionsList({
         return isExpenseReport(report) || isIOUReport(report) || isInvoiceReport(report);
     }, [parentReportAction, renderedVisibleReportActions, report]);
 
+    // Precompute a reportActionID -> index map so renderItem can resolve the real index in O(1)
+    // instead of scanning renderedVisibleReportActions with indexOf on every render.
+    const actionIndexMap = useMemo(() => {
+        const map = new Map<string, number>();
+        for (const [i, action] of renderedVisibleReportActions.entries()) {
+            map.set(action.reportActionID, i);
+        }
+        return map;
+    }, [renderedVisibleReportActions]);
+
     const renderItem = useCallback(
         ({item: reportAction, index}: ListRenderItemInfo<OnyxTypes.ReportAction>) => {
+            // Use the action's actual index in sortedVisibleReportActions rather than the FlashList-provided index,
+            // because useFlashListScrollKey may slice the data for deep-link scroll positioning, making the
+            // FlashList index offset from the full array and causing wrong displayAsGroup computation.
+            const safeIndex = actionIndexMap.get(reportAction.reportActionID) ?? index;
             const shouldDisableContextMenuForConciergeDraft = draftReportActionID === reportAction.reportActionID;
 
             return (
@@ -374,8 +388,8 @@ function ReportActionsList({
                         chatReport={chatReportStable}
                         linkedReportActionID={linkedReportActionID}
                         displayAsGroup={
-                            !isConsecutiveChronosAutomaticTimerAction(renderedVisibleReportActions, index, chatIncludesChronosWithID(reportAction?.reportID), isOffline) &&
-                            isConsecutiveActionMadeByPreviousActor(renderedVisibleReportActions, index, isOffline)
+                            !isConsecutiveChronosAutomaticTimerAction(renderedVisibleReportActions, safeIndex, chatIncludesChronosWithID(reportAction?.reportID), isOffline) &&
+                            isConsecutiveActionMadeByPreviousActor(renderedVisibleReportActions, safeIndex, isOffline)
                         }
                         shouldHideThreadDividerLine={shouldHideThreadDividerLine}
                         shouldDisplayNewMarker={reportAction.reportActionID === unreadMarkerReportActionID}
@@ -398,6 +412,7 @@ function ReportActionsList({
             );
         },
         [
+            actionIndexMap,
             draftReportActionID,
             firstVisibleReportActionID,
             hasPreviousMessages,
@@ -461,28 +476,6 @@ function ReportActionsList({
         isTrackIntentUser,
     });
 
-    const targetIndex = initialScrollKey ? renderedVisibleReportActions.findIndex((item) => keyExtractor(item) === initialScrollKey) : -1;
-    let initialScrollIndex: number | undefined;
-    let initialScrollIndexParams: {viewPosition?: number; viewOffset?: number} | undefined;
-    if (targetIndex > 0) {
-        initialScrollIndex = targetIndex;
-        initialScrollIndexParams = {viewPosition: 1, viewOffset: CONST.REPORT.ACTIONS.LINKED_MESSAGE_OFFSET};
-    } else if (shouldFocusToTopOnMount) {
-        initialScrollIndex = renderedVisibleReportActions.length - 1;
-        initialScrollIndexParams = {viewOffset: windowHeight};
-    }
-
-    const maintainVisibleContentPosition = {
-        disabled: !shouldMaintainVisibleContentPosition,
-        ...(shouldAutoscrollToBottom ? {autoscrollToBottomThreshold: CONST.REPORT.ACTIONS.ACTION_VISIBLE_THRESHOLD, animateAutoScrollToBottom: false} : {}),
-    };
-
-    // When opening a linked message, wait for the first load before rendering the list: the batch of actions that
-    // arrives right after the initial load shifts the list and breaks the anchor to the linked action.
-    if (initialScrollKey && !isOffline && !reportLoadingState?.hasOnceLoadedReportActions && reportLoadingState?.isLoadingInitialReportActions) {
-        return <ReportActionsSkeletonView />;
-    }
-
     return (
         <>
             <FloatingMessageCounter
@@ -530,10 +523,14 @@ function ReportActionsList({
                         contentOffset: shouldFocusToTopOnMount ? {x: 0, y: windowHeight} : undefined,
                     }}
                     getItemType={(item) => item.actionName}
-                    initialScrollIndex={initialScrollIndex}
-                    initialScrollIndexParams={initialScrollIndexParams}
-                    maintainVisibleContentPosition={maintainVisibleContentPosition}
+                    shouldMaintainVisibleContentPosition={shouldMaintainVisibleContentPosition}
+                    initialScrollIndex={shouldFocusToTopOnMount ? renderedVisibleReportActions.length - 1 : undefined}
+                    initialScrollIndexParams={shouldFocusToTopOnMount ? {viewOffset: windowHeight} : undefined}
+                    maintainVisibleContentPosition={
+                        shouldAutoscrollToBottom ? {autoscrollToBottomThreshold: CONST.REPORT.ACTIONS.ACTION_VISIBLE_THRESHOLD, animateAutoScrollToBottom: false} : undefined
+                    }
                     onLoad={onLoad}
+                    initialScrollKey={initialScrollKey}
                     onContentSizeChange={() => {
                         trackVerticalScrolling(undefined);
                     }}
