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 new file mode 100644 index 000000000000..521561d5ef2b --- /dev/null +++ b/patches/@shopify/flash-list/@shopify+flash-list+2.3.0+013+improve-scroll-key-handling.patch @@ -0,0 +1,166 @@ +diff --git a/node_modules/@shopify/flash-list/dist/FlashListProps.d.ts b/node_modules/@shopify/flash-list/dist/FlashListProps.d.ts +index fa786bf..586014c 100644 +--- a/node_modules/@shopify/flash-list/dist/FlashListProps.d.ts ++++ b/node_modules/@shopify/flash-list/dist/FlashListProps.d.ts +@@ -127,10 +127,12 @@ export interface FlashListProps extends Omit 0) { ++ initialItemOffset = Math.max(0, initialItemOffset - (windowSize - itemSize) * viewPosition); ++ } ++ } + this.engagedIndicesTracker.scrollOffset = initialItemOffset; + } + else { +@@ -317,8 +332,20 @@ export class RecyclerViewManager { + this.applyInitialScrollAdjustment(); + const visibleIndices = this.computeVisibleIndices(); + // console.log("---------> visibleIndices", visibleIndices); +- this.hasRenderedProgressively = visibleIndices.every((index) => layoutManager.getLayout(index).isHeightMeasured && +- layoutManager.getLayout(index).isWidthMeasured); ++ const isFullyMeasured = (index) => layoutManager.getLayout(index).isHeightMeasured && ++ layoutManager.getLayout(index).isWidthMeasured; ++ // With an explicit initialScrollIndex, also wait for the drawDistance buffer to be measured before ++ // completing the first layout, so estimate-driven layout shifts converge before anything is on screen. ++ let targetIndices = visibleIndices; ++ if (this.propsRef.initialScrollIndex !== undefined && visibleIndices.length > 0 && visibleIndices.every(isFullyMeasured)) { ++ const windowSize = this.propsRef.horizontal ? this.getWindowSize().width : this.getWindowSize().height; ++ const viewportStart = this.engagedIndicesTracker.scrollOffset; ++ // Cover the worst-case one-sided buffer the engaged tracker can mount after ++ // first layout (totalBuffer * largeMultiplier in the scroll direction). ++ const bufferDistance = this.engagedIndicesTracker.drawDistance * 2 * this.engagedIndicesTracker.largeMultiplier; ++ targetIndices = layoutManager.getVisibleLayouts(Math.max(0, viewportStart - bufferDistance), viewportStart + windowSize + bufferDistance); ++ } ++ this.hasRenderedProgressively = targetIndices.every(isFullyMeasured); + if (this.hasRenderedProgressively) { + this.isFirstLayoutComplete = true; + } +@@ -327,9 +354,13 @@ export class RecyclerViewManager { + // If everything is measured then render stack will be in sync. The buffer items will get rendered in the next update + // triggered by the useOnLoad hook. + !this.hasRenderedProgressively && +- this.updateRenderStack( +- // pick first n indices from visible ones based on batch size +- visibleIndices.slice(0, Math.min(visibleIndices.length, this.getRenderStack().size + batchSize))); ++ this.updateRenderStack(targetIndices === visibleIndices ++ ? // pick first n indices from visible ones based on batch size ++ visibleIndices.slice(0, Math.min(visibleIndices.length, this.getRenderStack().size + batchSize)) ++ : // buffer phase: visible items are already measured, mount the whole ++ // buffer window at once. Same single-commit cost the engaged tracker ++ // would pay post-paint, just moved to where nothing is visible yet. ++ targetIndices); + } + } + getItemType(index) { +diff --git a/node_modules/@shopify/flash-list/dist/recyclerview/hooks/useRecyclerViewController.js b/node_modules/@shopify/flash-list/dist/recyclerview/hooks/useRecyclerViewController.js +index 18e59ce..40bdddb 100644 +--- a/node_modules/@shopify/flash-list/dist/recyclerview/hooks/useRecyclerViewController.js ++++ b/node_modules/@shopify/flash-list/dist/recyclerview/hooks/useRecyclerViewController.js +@@ -25,6 +25,10 @@ export function useRecyclerViewController(recyclerViewManager, ref, scrollViewRe + const isUnmounted = useUnmountFlag(); + const [_, setRenderId] = useState(0); + const pauseOffsetCorrection = useRef(false); ++ // Latest offset computed by applyInitialScrollIndex. The deferred (setTimeout) re-scroll reads this at ++ // fire-time instead of the value it closed over, so a stale timeout scheduled by an earlier commit can't ++ // snap back to an outdated offset after a newer commit. ++ const latestInitialScrollOffsetRef = useRef(0); + // True while a `scrollToIndex` / `scrollToOffset` smooth scroll is in + // flight. Cleared exactly once on `isMomentumEnd` via + // `notifyProgrammaticScrollSettled`. +@@ -566,18 +570,55 @@ export function useRecyclerViewController(recyclerViewManager, ref, scrollViewRe + }, 500); + pauseOffsetCorrection.current = true; + const additionalOffset = (_c = initialScrollIndexParams === null || initialScrollIndexParams === void 0 ? void 0 : initialScrollIndexParams.viewOffset) !== null && _c !== void 0 ? _c : 0; +- const offset = horizontal +- ? recyclerViewManager.getLayout(initialScrollIndex).x + additionalOffset +- : recyclerViewManager.getLayout(initialScrollIndex).y + +- additionalOffset; ++ const initialItemLayout = recyclerViewManager.getLayout(initialScrollIndex); ++ let offset = (horizontal ? initialItemLayout.x : initialItemLayout.y) + ++ additionalOffset; ++ // Position the target item within the viewport (0 = start, 0.5 = center, 1 = end), mirroring scrollToIndex. ++ const viewPosition = initialScrollIndexParams === null || initialScrollIndexParams === void 0 ? void 0 : initialScrollIndexParams.viewPosition; ++ if (viewPosition !== undefined) { ++ const containerSize = horizontal ++ ? recyclerViewManager.getWindowSize().width ++ : recyclerViewManager.getWindowSize().height; ++ const itemSize = horizontal ++ ? initialItemLayout.width ++ : initialItemLayout.height; ++ if (containerSize > 0) { ++ offset = Math.max(0, offset - (containerSize - itemSize) * viewPosition); ++ } ++ } ++ // Make it clear there are more items to scroll to underneath the bottom edge. ++ // If the bottom item is (essentially) fully visible against the bottom edge AND there ++ // is an item underneath it, nudge the bottom edge up so CROP_OFFSET px of the current ++ // bottom item gets cropped, signalling that more content can be scrolled into view. ++ if (viewPosition !== undefined && !horizontal && recyclerViewManager.props.inverted && offset > 0) { ++ const CROP_OFFSET = 10; ++ let bottomIndex = -1; ++ for (let i = initialScrollIndex; i >= 0; i--) { ++ if (recyclerViewManager.getLayout(i).y <= offset) { ++ bottomIndex = i; ++ break; ++ } ++ } ++ if (bottomIndex > 0) { ++ const bottomItemLayout = recyclerViewManager.getLayout(bottomIndex); ++ const hiddenPortion = offset - bottomItemLayout.y; ++ // 8px is bottom padding of every item ++ if (hiddenPortion <= 8) { ++ // Crop the current bottom item rather than letting it sit flush against the edge. ++ offset = bottomItemLayout.y + CROP_OFFSET; ++ } ++ } ++ } ++ latestInitialScrollOffsetRef.current = offset; + handlerMethods.scrollToOffset({ + offset, + animated: false, + skipFirstItemOffset: false, + }); ++ + setTimeout(() => { + handlerMethods.scrollToOffset({ +- offset, ++ offset: latestInitialScrollOffsetRef.current, + animated: false, + skipFirstItemOffset: false, + }); diff --git a/patches/@shopify/flash-list/details.md b/patches/@shopify/flash-list/details.md index a826ad3f90df..0ae0ea33142f 100644 --- a/patches/@shopify/flash-list/details.md +++ b/patches/@shopify/flash-list/details.md @@ -141,3 +141,17 @@ - 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`). Six 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. + 5. **Deferred re-scroll reads the latest offset** in `applyInitialScrollIndex` (`useRecyclerViewController.js`): the `setTimeout(0)` re-scroll used to close over the `offset` from its own commit. When a later commit recomputed a newer offset before that timeout fired, the stale timeout snapped the list back to the outdated offset — a visible jump. The offset is now stored in `latestInitialScrollOffsetRef` and read at fire-time, so any pending re-scroll targets the current offset instead of a stale one. + 6. **Progressive render covers the drawDistance buffer** in `renderProgressively` (`RecyclerViewManager.js`): with an explicit `initialScrollIndex`, the drawDistance buffer used to mount right after first paint; its measurements re-estimated every still-unmeasured item before the target, which could collapse the content height below the applied scroll offset and make the native ScrollView clamp. Now the progressive-render phase also waits for the buffer around the viewport to be measured, so the layout converges before anything is painted. Only applies when `initialScrollIndex` is set; other lists keep stock behavior. +- 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 index 887e89a783a6..b0b88f861495 100644 --- a/src/CONST/index.ts +++ b/src/CONST/index.ts @@ -1775,6 +1775,7 @@ 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 index e6c5306309eb..2b1d5b72633f 100644 --- a/src/components/FlashList/InvertedFlashList/index.tsx +++ b/src/components/FlashList/InvertedFlashList/index.tsx @@ -1,5 +1,3 @@ -import useFlashListScrollKey from '@components/FlashList/useFlashListScrollKey'; - import type {FlatListRefType} from '@pages/inbox/ReportScreenContext'; import type {FlashListProps} from '@shopify/flash-list'; @@ -10,9 +8,6 @@ import FlashList from '..'; import CellRendererComponent from './CellRendererComponent'; type InvertedFlashListProps = FlashListProps & { - /** 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[]; @@ -21,48 +16,14 @@ type InvertedFlashListProps = FlashListProps & { /** Ref to the underlying list instance. */ ref: FlatListRefType; - - /** Whether the list should handle `maintainVisibleContentPosition` */ - shouldMaintainVisibleContentPosition?: boolean; }; -function InvertedFlashList({ - data, - keyExtractor, - initialScrollKey, - onStartReached: onStartReachedProp, - maintainVisibleContentPosition: maintainVisibleContentPositionProp, - shouldMaintainVisibleContentPosition, - ...restProps -}: InvertedFlashListProps) { - const { - displayedData, - onStartReached, - maintainVisibleContentPosition: maintainVisibleContentPositionForScrollKey, - } = useFlashListScrollKey({ - data, - keyExtractor, - initialScrollKey, - onStartReached: onStartReachedProp, - shouldMaintainVisibleContentPosition, - }); - - const maintainVisibleContentPosition = maintainVisibleContentPositionProp - ? { - ...maintainVisibleContentPositionForScrollKey, - ...maintainVisibleContentPositionProp, - } - : maintainVisibleContentPositionForScrollKey; - +function InvertedFlashList(props: InvertedFlashListProps) { return ( - {...restProps} + {...props} 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 deleted file mode 100644 index 9c28f79eeb0c..000000000000 --- a/src/components/FlashList/useFlashListScrollKey.ts +++ /dev/null @@ -1,65 +0,0 @@ -import type {FlashListProps} from '@shopify/flash-list'; - -import {useEffect, useState} from 'react'; - -type FlashListScrollKeyProps = { - /** 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['onStartReached']; - - /** Whether the list should handle `maintainVisibleContentPosition` */ - shouldMaintainVisibleContentPosition?: boolean; -}; - -export default function useFlashListScrollKey({data, keyExtractor, initialScrollKey, onStartReached, shouldMaintainVisibleContentPosition}: FlashListScrollKeyProps) { - 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['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/hooks/useReportActionsScroll.ts b/src/hooks/useReportActionsScroll.ts index f277d6ecff7a..a17cfec0e309 100644 --- a/src/hooks/useReportActionsScroll.ts +++ b/src/hooks/useReportActionsScroll.ts @@ -34,6 +34,7 @@ import useOnyx from './useOnyx'; import usePrevious from './usePrevious'; import useReportScrollManager from './useReportScrollManager'; import useScrollToEndOnNewMessageReceived from './useScrollToEndOnNewMessageReceived'; +import useWindowDimensions from './useWindowDimensions'; type UseReportActionsScrollParams = { /** The ID of the report currently being looked at */ @@ -51,6 +52,15 @@ type UseReportActionsScrollParams = { /** Sorted actions that should be visible to the user */ sortedVisibleReportActions: OnyxTypes.ReportAction[]; + /** Actions actually rendered by the list (may include a synthetic draft), used for mount scroll positioning */ + renderedVisibleReportActions: OnyxTypes.ReportAction[]; + + /** Extracts the list key for an action; used to locate the initial scroll target */ + keyExtractor: (item: OnyxTypes.ReportAction) => string; + + /** Whether the user has scrolled past the "visible" threshold */ + hasScrolledOverThreshold: boolean; + /** Marks the newest action as read and clears any pending skipped mark-as-read */ markNewestActionAsRead: () => void; @@ -116,12 +126,13 @@ type UseReportActionsScrollResult = { initialScrollKey: string | undefined; /** maintainVisibleContentPosition config for the inverted list */ - maintainVisibleContentPosition: - | { - autoscrollToBottomThreshold?: number; - animateAutoScrollToBottom?: boolean; - } - | undefined; + maintainVisibleContentPosition: {disabled: boolean; autoscrollToBottomThreshold?: number; animateAutoScrollToBottom?: boolean}; + + /** The index the list should scroll to on mount (undefined to keep default position) */ + initialScrollIndex: number | undefined; + + /** Positioning params (viewPosition/viewOffset) paired with initialScrollIndex */ + initialScrollIndexParams: {viewPosition?: number; viewOffset?: number} | undefined; /** onLoad handler that disables autoscroll-to-top once the initial render settles */ onLoad: () => void; @@ -133,6 +144,9 @@ function useReportActionsScroll({ transactionThreadReport, parentReportAction, sortedVisibleReportActions, + renderedVisibleReportActions, + keyExtractor, + hasScrolledOverThreshold, markNewestActionAsRead, completeSkippedMarkAsRead, unreadMarkerReportActionID, @@ -145,6 +159,7 @@ function useReportActionsScroll({ setTreatAsNoPaginationAnchor, }: UseReportActionsScrollParams): UseReportActionsScrollResult { const reportScrollManager = useReportScrollManager(); + const {windowHeight} = useWindowDimensions(); const {scrollOffsetRef} = useContext(ActionListContext); const route = useRoute>(); const linkedReportActionID = route?.params?.reportActionID; @@ -180,19 +195,19 @@ function useReportActionsScroll({ } const shouldFocusToTopOnMount = shouldBeAlignedToTop && !initialScrollKey; + const shouldMaintainVisibleContentPosition = hasScrolledOverThreshold || shouldFocusToTopOnMount; const [shouldAutoscrollToBottom, setShouldAutoscrollToBottom] = useState(shouldFocusToTopOnMount); - // Once the initial pin to the visual top is released, the threshold must drop to 0 instead of removing the config: - // FlashList only clears its internal pending-autoscroll flag while `autoscrollToBottomThreshold >= 0`. Removing the - // config leaves a flag planted during the pin phase stale forever, and the next content change (e.g. marking a - // message as unread) would scroll the list back to the top. - const maintainVisibleContentPosition = shouldFocusToTopOnMount - ? { - autoscrollToBottomThreshold: shouldAutoscrollToBottom ? CONST.REPORT.ACTIONS.ACTION_VISIBLE_THRESHOLD : 0, - animateAutoScrollToBottom: false, - } - : undefined; - - const {isFloatingMessageCounterVisible, setIsFloatingMessageCounterVisible, isActionBadgeAboveViewport, trackVerticalScrolling, onViewableItemsChanged} = + const [shouldDisablePillTracking, setShouldDisablePillTracking] = useState(!!initialScrollKey); + + const maintainVisibleContentPosition = { + disabled: !shouldMaintainVisibleContentPosition, + // Focus-to-top mode: once autoscroll is released, keep the threshold at 0 rather than + // removing it — FlashList only clears its pending-autoscroll flag while threshold >= 0, + // otherwise the next content change (e.g. mark-as-unread) scrolls back to top. + ...(shouldFocusToTopOnMount ? {autoscrollToBottomThreshold: shouldAutoscrollToBottom ? CONST.REPORT.ACTIONS.ACTION_VISIBLE_THRESHOLD : 0, animateAutoScrollToBottom: false} : {}), + }; + + const {isFloatingMessageCounterVisible, setIsFloatingMessageCounterVisible, isActionBadgeAboveViewport, trackVerticalScrolling, onViewableItemsChanged, updatePillVisibility} = useReportUnreadMessageScrollTracking({ reportID, currentVerticalScrollingOffsetRef: scrollOffsetRef, @@ -200,6 +215,7 @@ function useReportActionsScroll({ hasNewerActions, unreadMarkerReportActionIndex, isInverted: true, + shouldDisablePillTracking, onTrackScrolling: (event: NativeSyntheticEvent) => { scrollOffsetRef.current = event.nativeEvent.contentOffset.y; }, @@ -359,14 +375,21 @@ function useReportActionsScroll({ }; // Data is ready at the moment FlashList finishes its first render. - // Wait one frame so the initial autoscroll-to-top can settle, then disable it. const onLoad = () => { + if (shouldDisablePillTracking) { + // Wait one frame so the initial positioning can settle, then disable it. + requestAnimationFrame(() => { + setShouldDisablePillTracking(false); + updatePillVisibility(); + }); + } if (!shouldFocusToTopOnMount) { return; } if (!reportLoadingState?.hasOnceLoadedReportActions && !isOffline) { return; } + // Wait one frame so the initial autoscroll-to-top can settle, then disable it. requestAnimationFrame(() => setShouldAutoscrollToBottom(false)); }; const prevHasOnceLoadedReportActions = usePrevious(reportLoadingState?.hasOnceLoadedReportActions); @@ -383,6 +406,22 @@ function useReportActionsScroll({ requestAnimationFrame(() => setShouldAutoscrollToBottom(false)); }, [shouldFocusToTopOnMount, shouldAutoscrollToBottom, prevHasOnceLoadedReportActions, reportLoadingState?.hasOnceLoadedReportActions]); + // Decide where the list should be positioned on mount. + // 1. If we're opening a linked message (initialScrollKey), find that action in the list and scroll it to the top + // of the viewport (viewPosition: 1) with a small offset so the message above is partly visible. + // 2. Otherwise, if the report should be opened at top (ex: for transaction threads), scroll to the top message and offset by + // the window height so we land at top of the top message for sure. + 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}; + } + return { listRef: reportScrollManager.ref, trackVerticalScrolling, @@ -396,6 +435,8 @@ function useReportActionsScroll({ shouldFocusToTopOnMount, initialScrollKey, maintainVisibleContentPosition, + initialScrollIndex, + initialScrollIndexParams, onLoad, }; } diff --git a/src/pages/inbox/report/ReportActionsList.tsx b/src/pages/inbox/report/ReportActionsList.tsx index f06662732244..16aa3cf2ec41 100644 --- a/src/pages/inbox/report/ReportActionsList.tsx +++ b/src/pages/inbox/report/ReportActionsList.tsx @@ -280,7 +280,8 @@ function ReportActionsListContent({reportID, onLayout}: ReportActionsListProps) flushPendingScrollToBottom, shouldBeAlignedToTop, shouldFocusToTopOnMount, - initialScrollKey, + initialScrollIndex, + initialScrollIndexParams, maintainVisibleContentPosition, onLoad, } = useReportActionsScroll({ @@ -289,6 +290,9 @@ function ReportActionsListContent({reportID, onLayout}: ReportActionsListProps) transactionThreadReport, parentReportAction, sortedVisibleReportActions, + renderedVisibleReportActions, + keyExtractor, + hasScrolledOverThreshold, markNewestActionAsRead, completeSkippedMarkAsRead, unreadMarkerReportActionID, @@ -326,8 +330,6 @@ function ReportActionsListContent({reportID, onLayout}: ReportActionsListProps) }); }; - const shouldMaintainVisibleContentPosition = hasScrolledOverThreshold || shouldFocusToTopOnMount; - /** * Thread's divider line should hide when the first chat in the thread is marked as unread. * This is so that it will not be conflicting with header's separator line. @@ -357,22 +359,8 @@ function ReportActionsListContent({reportID, onLayout}: ReportActionsListProps) 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(); - for (const [i, action] of renderedVisibleReportActions.entries()) { - map.set(action.reportActionID, i); - } - return map; - }, [renderedVisibleReportActions]); - const renderItem = useCallback( ({item: reportAction, index}: ListRenderItemInfo) => { - // 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 ( @@ -386,8 +374,8 @@ function ReportActionsListContent({reportID, onLayout}: ReportActionsListProps) chatReport={chatReportStable} linkedReportActionID={linkedReportActionID} displayAsGroup={ - !isConsecutiveChronosAutomaticTimerAction(renderedVisibleReportActions, safeIndex, chatIncludesChronosWithID(reportAction?.reportID), isOffline) && - isConsecutiveActionMadeByPreviousActor(renderedVisibleReportActions, safeIndex, isOffline) + !isConsecutiveChronosAutomaticTimerAction(renderedVisibleReportActions, index, chatIncludesChronosWithID(reportAction?.reportID), isOffline) && + isConsecutiveActionMadeByPreviousActor(renderedVisibleReportActions, index, isOffline) } shouldHideThreadDividerLine={shouldHideThreadDividerLine} shouldDisplayNewMarker={reportAction.reportActionID === unreadMarkerReportActionID} @@ -410,7 +398,6 @@ function ReportActionsListContent({reportID, onLayout}: ReportActionsListProps) ); }, [ - actionIndexMap, draftReportActionID, firstVisibleReportActionID, hasPreviousMessages, @@ -534,12 +521,10 @@ function ReportActionsListContent({reportID, onLayout}: ReportActionsListProps) contentOffset: shouldFocusToTopOnMount ? {x: 0, y: windowHeight} : undefined, }} getItemType={(item) => item.actionName} - shouldMaintainVisibleContentPosition={shouldMaintainVisibleContentPosition} - initialScrollIndex={shouldFocusToTopOnMount ? renderedVisibleReportActions.length - 1 : undefined} - initialScrollIndexParams={shouldFocusToTopOnMount ? {viewOffset: windowHeight} : undefined} + initialScrollIndex={initialScrollIndex} + initialScrollIndexParams={initialScrollIndexParams} maintainVisibleContentPosition={maintainVisibleContentPosition} onLoad={onLoad} - initialScrollKey={initialScrollKey} onContentSizeChange={() => { trackVerticalScrolling(undefined); }} diff --git a/src/pages/inbox/report/useReportUnreadMessageScrollTracking.ts b/src/pages/inbox/report/useReportUnreadMessageScrollTracking.ts index d8e5db839c23..6a0385d271dc 100644 --- a/src/pages/inbox/report/useReportUnreadMessageScrollTracking.ts +++ b/src/pages/inbox/report/useReportUnreadMessageScrollTracking.ts @@ -33,6 +33,9 @@ type Args = { /** Whether the report is aligned to the top. When true, the "Latest messages" pill should never be shown. */ shouldBeAlignedToTop?: boolean; + + /** If pill tracking should be disabled, used during initial linked message positioning */ + shouldDisablePillTracking?: boolean; }; export default function useReportUnreadMessageScrollTracking({ @@ -45,6 +48,7 @@ export default function useReportUnreadMessageScrollTracking({ isInverted, actionBadgeTargetIndex = -1, shouldBeAlignedToTop = false, + shouldDisablePillTracking = false, }: Args) { const [isFloatingMessageCounterVisible, setIsFloatingMessageCounterVisible] = useState(false); const [isActionBadgeAboveViewport, setIsActionBadgeAboveViewport] = useState(false); @@ -80,14 +84,9 @@ export default function useReportUnreadMessageScrollTracking({ }, [onUnreadActionVisible]); /** - * On every scroll event we want to: * Show/hide the latest message pill when user is scrolling back/forth in the history of messages. - * Call any other callback that the component might need */ - const trackVerticalScrolling = (event: NativeSyntheticEvent | undefined) => { - if (event) { - onTrackScrolling(event); - } + const updatePillVisibility = () => { const hasUnreadMarkerReportAction = unreadMarkerReportActionIndex !== -1; // display floating button if we're scrolled more than the offset @@ -111,6 +110,23 @@ export default function useReportUnreadMessageScrollTracking({ } }; + /** + * On every scroll event we want to: + * Update the current scroll offset ref + * Show/hide the latest message pill, if it's not disabled + */ + const trackVerticalScrolling = (event: NativeSyntheticEvent | undefined) => { + if (event) { + onTrackScrolling(event); + } + + if (shouldDisablePillTracking) { + return; + } + + updatePillVisibility(); + }; + const onViewableItemsChanged = useCallback(({viewableItems}: {viewableItems: ViewToken[]; changed: ViewToken[]}) => { if (!ref.current.isFocused) { return; @@ -181,5 +197,6 @@ export default function useReportUnreadMessageScrollTracking({ isActionBadgeAboveViewport, trackVerticalScrolling, onViewableItemsChanged, + updatePillVisibility, }; } diff --git a/tests/unit/useReportActionsScrollTest.tsx b/tests/unit/useReportActionsScrollTest.tsx index 1cd09eaeb8a8..b6df11fb88e2 100644 --- a/tests/unit/useReportActionsScrollTest.tsx +++ b/tests/unit/useReportActionsScrollTest.tsx @@ -178,6 +178,9 @@ function buildParams(overrides: Partial = {}): ScrollParams { transactionThreadReport: undefined, parentReportAction: undefined, sortedVisibleReportActions: [makeAction('1')], + renderedVisibleReportActions: [makeAction('1')], + keyExtractor: (item: ReportAction) => item.reportActionID, + hasScrolledOverThreshold: false, markNewestActionAsRead: mockMarkNewestActionAsRead, completeSkippedMarkAsRead: mockCompleteSkippedMarkAsRead, unreadMarkerReportActionID: null, @@ -249,7 +252,8 @@ describe('useReportActionsScroll', () => { expect(result.current.shouldBeAlignedToTop).toBe(false); expect(result.current.shouldFocusToTopOnMount).toBe(false); - expect(result.current.maintainVisibleContentPosition).toBeUndefined(); + expect(result.current.maintainVisibleContentPosition.disabled).toBe(true); + expect(result.current.maintainVisibleContentPosition.autoscrollToBottomThreshold).toBeUndefined(); }); it('is aligned to top and focuses to top on mount for a transaction thread report', async () => { @@ -408,8 +412,9 @@ describe('useReportActionsScroll', () => { result.current.onLoad(); }); - // Stays undefined for a regular chat. - expect(result.current.maintainVisibleContentPosition).toBeUndefined(); + // Stays disabled with no autoscroll threshold for a regular chat. + expect(result.current.maintainVisibleContentPosition.disabled).toBe(true); + expect(result.current.maintainVisibleContentPosition.autoscrollToBottomThreshold).toBeUndefined(); }); it('waits for the report actions to have loaded before disabling autoscroll-to-top', async () => {