Skip to content

Commit 068f736

Browse files
VickyStashVickyStash
committed
Revert "Merge pull request #94470 from Expensify/revert-93403-VickyStash/feature/open-linked-message-centered"
This reverts commit b9c9d62, reversing changes made to abb2cfc.
1 parent 1eee891 commit 068f736

6 files changed

Lines changed: 146 additions & 128 deletions

File tree

‎patches/@shopify/flash-list/@shopify+flash-list+2.3.0+013+improve-scroll-key-handling.patch‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
diff --git a/node_modules/@shopify/flash-list/dist/FlashListProps.d.ts b/node_modules/@shopify/flash-list/dist/FlashListProps.d.ts
2+
index fa786bf..586014c 100644
3+
--- a/node_modules/@shopify/flash-list/dist/FlashListProps.d.ts
4+
+++ b/node_modules/@shopify/flash-list/dist/FlashListProps.d.ts
5+
@@ -127,10 +127,12 @@ export interface FlashListProps<TItem> extends Omit<ScrollViewProps, "maintainVi
6+
/**
7+
* Additional configuration for initialScrollIndex.
8+
* Use viewOffset to apply an offset to the initial scroll position as defined by initialScrollIndex.
9+
+ * Use viewPosition to position the item within the viewport (0 = start, 0.5 = center, 1 = end), mirroring scrollToIndex.
10+
* Ignored if initialScrollIndex is not set.
11+
*/
12+
initialScrollIndexParams?: {
13+
viewOffset?: number;
14+
+ viewPosition?: number;
15+
} | null | undefined;
16+
/**
17+
* Used to extract a unique key for a given item at the specified index.
18+
diff --git a/node_modules/@shopify/flash-list/dist/recyclerview/RecyclerViewManager.js b/node_modules/@shopify/flash-list/dist/recyclerview/RecyclerViewManager.js
19+
index 3b69234..41bfb84 100644
20+
--- a/node_modules/@shopify/flash-list/dist/recyclerview/RecyclerViewManager.js
21+
+++ b/node_modules/@shopify/flash-list/dist/recyclerview/RecyclerViewManager.js
22+
@@ -294,11 +294,26 @@ export class RecyclerViewManager {
23+
// re-estimate unmeasured items with an updated average height, changing
24+
// the target item's position. Reading before recompute would capture a
25+
// stale offset, causing the wrong items to be rendered.
26+
- this.layoutManager.recomputeLayouts(0, initialScrollIndex);
27+
+ this.layoutManager.recomputeLayouts(0, this.getDataLength() - 1);
28+
const initialItemLayout = this.layoutManager.getLayout(initialScrollIndex);
29+
- const initialItemOffset = this.propsRef.horizontal
30+
+ let initialItemOffset = this.propsRef.horizontal
31+
? initialItemLayout.x
32+
: initialItemLayout.y;
33+
+ // Anchor the initial render window according to initialScrollIndexParams.viewPosition so the
34+
+ // first painted frame already shows the target item at the requested position in the viewport.
35+
+ const initialScrollIndexParams = this.propsRef.initialScrollIndexParams;
36+
+ const viewPosition = initialScrollIndexParams === null || initialScrollIndexParams === void 0 ? void 0 : initialScrollIndexParams.viewPosition;
37+
+ if (viewPosition !== undefined) {
38+
+ const windowSize = this.propsRef.horizontal
39+
+ ? this.getWindowSize().width
40+
+ : this.getWindowSize().height;
41+
+ const itemSize = this.propsRef.horizontal
42+
+ ? initialItemLayout.width
43+
+ : initialItemLayout.height;
44+
+ if (windowSize > 0) {
45+
+ initialItemOffset = Math.max(0, initialItemOffset - (windowSize - itemSize) * viewPosition);
46+
+ }
47+
+ }
48+
this.engagedIndicesTracker.scrollOffset = initialItemOffset;
49+
}
50+
else {
51+
diff --git a/node_modules/@shopify/flash-list/dist/recyclerview/hooks/useRecyclerViewController.js b/node_modules/@shopify/flash-list/dist/recyclerview/hooks/useRecyclerViewController.js
52+
index 18e59ce..17c8063 100644
53+
--- a/node_modules/@shopify/flash-list/dist/recyclerview/hooks/useRecyclerViewController.js
54+
+++ b/node_modules/@shopify/flash-list/dist/recyclerview/hooks/useRecyclerViewController.js
55+
@@ -566,10 +566,45 @@ export function useRecyclerViewController(recyclerViewManager, ref, scrollViewRe
56+
}, 500);
57+
pauseOffsetCorrection.current = true;
58+
const additionalOffset = (_c = initialScrollIndexParams === null || initialScrollIndexParams === void 0 ? void 0 : initialScrollIndexParams.viewOffset) !== null && _c !== void 0 ? _c : 0;
59+
- const offset = horizontal
60+
- ? recyclerViewManager.getLayout(initialScrollIndex).x + additionalOffset
61+
- : recyclerViewManager.getLayout(initialScrollIndex).y +
62+
- additionalOffset;
63+
+ const initialItemLayout = recyclerViewManager.getLayout(initialScrollIndex);
64+
+ let offset = (horizontal ? initialItemLayout.x : initialItemLayout.y) +
65+
+ additionalOffset;
66+
+ // Position the target item within the viewport (0 = start, 0.5 = center, 1 = end), mirroring scrollToIndex.
67+
+ const viewPosition = initialScrollIndexParams === null || initialScrollIndexParams === void 0 ? void 0 : initialScrollIndexParams.viewPosition;
68+
+ if (viewPosition !== undefined) {
69+
+ const containerSize = horizontal
70+
+ ? recyclerViewManager.getWindowSize().width
71+
+ : recyclerViewManager.getWindowSize().height;
72+
+ const itemSize = horizontal
73+
+ ? initialItemLayout.width
74+
+ : initialItemLayout.height;
75+
+ if (containerSize > 0) {
76+
+ offset = Math.max(0, offset - (containerSize - itemSize) * viewPosition);
77+
+ }
78+
+ }
79+
+ // Make it clear there are more items to scroll to underneath the bottom edge.
80+
+ // If the bottom item is (essentially) fully visible against the bottom edge AND there
81+
+ // is an item underneath it, nudge the bottom edge up so CROP_OFFSET px of the current
82+
+ // bottom item gets cropped, signalling that more content can be scrolled into view.
83+
+ if (viewPosition !== undefined && !horizontal && recyclerViewManager.props.inverted && offset > 0) {
84+
+ const CROP_OFFSET = 10;
85+
+ let bottomIndex = -1;
86+
+ for (let i = initialScrollIndex; i >= 0; i--) {
87+
+ if (recyclerViewManager.getLayout(i).y <= offset) {
88+
+ bottomIndex = i;
89+
+ break;
90+
+ }
91+
+ }
92+
+ if (bottomIndex > 0) {
93+
+ const bottomItemLayout = recyclerViewManager.getLayout(bottomIndex);
94+
+ const hiddenPortion = offset - bottomItemLayout.y;
95+
+ // 8px is bottom padding of every item
96+
+ if (hiddenPortion <= 8) {
97+
+ // Crop the current bottom item rather than letting it sit flush against the edge.
98+
+ offset = bottomItemLayout.y + CROP_OFFSET;
99+
+ }
100+
+ }
101+
+ }
102+
handlerMethods.scrollToOffset({
103+
offset,
104+
animated: false,

‎patches/@shopify/flash-list/details.md‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,3 +141,15 @@
141141
- Upstream PR/issue: https://github.com/Shopify/flash-list/issues/2334
142142
- E/App issue: https://github.com/Expensify/App/issues/91584, https://github.com/Expensify/App/issues/92263
143143
- PR introducing patch: https://github.com/Expensify/App/pull/92520
144+
145+
### [@shopify+flash-list+2.3.0+013+improve-scroll-key-handling.patch](@shopify+flash-list+2.3.0+013+improve-scroll-key-handling.patch)
146+
147+
- Reason: Adds `viewPosition` support to `initialScrollIndexParams` (0 = start, 0.5 = center, 1 = end — same semantics as `scrollToIndex`'s `viewPosition`). Four changes:
148+
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.
149+
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.
150+
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.
151+
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.
152+
- Files changed: `dist/FlashListProps.d.ts`, `dist/recyclerview/hooks/useRecyclerViewController.js`, `dist/recyclerview/RecyclerViewManager.js`.
153+
- Upstream PR/issue: https://github.com/Shopify/flash-list/pull/2318 (for point 4)
154+
- E/App issue: https://github.com/Expensify/App/issues/92152
155+
- PR introducing patch: https://github.com/Expensify/App/pull/93403

‎src/CONST/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1751,6 +1751,7 @@ const CONST = {
17511751
THREAD_DISABLED: ['CREATED'],
17521752
LATEST_MESSAGES_PILL_SCROLL_OFFSET_THRESHOLD: 2000,
17531753
ACTION_VISIBLE_THRESHOLD: 250,
1754+
LINKED_MESSAGE_OFFSET: 40,
17541755
MAX_GROUPING_TIME: 300000,
17551756
},
17561757
CANCEL_PAYMENT_REASONS: {

‎src/components/FlashList/InvertedFlashList/index.tsx‎

Lines changed: 2 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,10 @@
11
import type {FlashListProps} from '@shopify/flash-list';
22
import React from 'react';
3-
import useFlashListScrollKey from '@components/FlashList/useFlashListScrollKey';
43
import type {FlatListRefType} from '@pages/inbox/ReportScreenContext';
54
import FlashList from '..';
65
import CellRendererComponent from './CellRendererComponent';
76

87
type InvertedFlashListProps<T> = FlashListProps<T> & {
9-
/** Key of the item to initially scroll to when the list first renders. */
10-
initialScrollKey?: string | null;
11-
128
/** The array of items to render in the list. */
139
data: T[];
1410

@@ -17,48 +13,14 @@ type InvertedFlashListProps<T> = FlashListProps<T> & {
1713

1814
/** Ref to the underlying list instance. */
1915
ref: FlatListRefType;
20-
21-
/** Whether the list should handle `maintainVisibleContentPosition` */
22-
shouldMaintainVisibleContentPosition?: boolean;
2316
};
2417

25-
function InvertedFlashList<T>({
26-
data,
27-
keyExtractor,
28-
initialScrollKey,
29-
onStartReached: onStartReachedProp,
30-
maintainVisibleContentPosition: maintainVisibleContentPositionProp,
31-
shouldMaintainVisibleContentPosition,
32-
...restProps
33-
}: InvertedFlashListProps<T>) {
34-
const {
35-
displayedData,
36-
onStartReached,
37-
maintainVisibleContentPosition: maintainVisibleContentPositionForScrollKey,
38-
} = useFlashListScrollKey<T>({
39-
data,
40-
keyExtractor,
41-
initialScrollKey,
42-
onStartReached: onStartReachedProp,
43-
shouldMaintainVisibleContentPosition,
44-
});
45-
46-
const maintainVisibleContentPosition = maintainVisibleContentPositionProp
47-
? {
48-
...maintainVisibleContentPositionForScrollKey,
49-
...maintainVisibleContentPositionProp,
50-
}
51-
: maintainVisibleContentPositionForScrollKey;
52-
18+
function InvertedFlashList<T>(props: InvertedFlashListProps<T>) {
5319
return (
5420
<FlashList<T>
55-
{...restProps}
21+
{...props}
5622
inverted
57-
onStartReached={onStartReached}
58-
data={displayedData}
59-
keyExtractor={keyExtractor}
6023
CellRendererComponent={CellRendererComponent}
61-
maintainVisibleContentPosition={maintainVisibleContentPosition}
6224
/>
6325
);
6426
}

‎src/components/FlashList/useFlashListScrollKey.ts‎

Lines changed: 0 additions & 64 deletions
This file was deleted.

‎src/pages/inbox/report/ReportActionsList.tsx‎

Lines changed: 27 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -366,22 +366,8 @@ function ReportActionsList({
366366
return isExpenseReport(report) || isIOUReport(report) || isInvoiceReport(report);
367367
}, [parentReportAction, renderedVisibleReportActions, report]);
368368

369-
// Precompute a reportActionID -> index map so renderItem can resolve the real index in O(1)
370-
// instead of scanning renderedVisibleReportActions with indexOf on every render.
371-
const actionIndexMap = useMemo(() => {
372-
const map = new Map<string, number>();
373-
for (const [i, action] of renderedVisibleReportActions.entries()) {
374-
map.set(action.reportActionID, i);
375-
}
376-
return map;
377-
}, [renderedVisibleReportActions]);
378-
379369
const renderItem = useCallback(
380370
({item: reportAction, index}: ListRenderItemInfo<OnyxTypes.ReportAction>) => {
381-
// Use the action's actual index in sortedVisibleReportActions rather than the FlashList-provided index,
382-
// because useFlashListScrollKey may slice the data for deep-link scroll positioning, making the
383-
// FlashList index offset from the full array and causing wrong displayAsGroup computation.
384-
const safeIndex = actionIndexMap.get(reportAction.reportActionID) ?? index;
385371
const shouldDisableContextMenuForConciergeDraft = draftReportActionID === reportAction.reportActionID;
386372

387373
return (
@@ -395,8 +381,8 @@ function ReportActionsList({
395381
chatReport={chatReportStable}
396382
linkedReportActionID={linkedReportActionID}
397383
displayAsGroup={
398-
!isConsecutiveChronosAutomaticTimerAction(renderedVisibleReportActions, safeIndex, chatIncludesChronosWithID(reportAction?.reportID), isOffline) &&
399-
isConsecutiveActionMadeByPreviousActor(renderedVisibleReportActions, safeIndex, isOffline)
384+
!isConsecutiveChronosAutomaticTimerAction(renderedVisibleReportActions, index, chatIncludesChronosWithID(reportAction?.reportID), isOffline) &&
385+
isConsecutiveActionMadeByPreviousActor(renderedVisibleReportActions, index, isOffline)
400386
}
401387
shouldHideThreadDividerLine={shouldHideThreadDividerLine}
402388
shouldDisplayNewMarker={reportAction.reportActionID === unreadMarkerReportActionID}
@@ -419,7 +405,6 @@ function ReportActionsList({
419405
);
420406
},
421407
[
422-
actionIndexMap,
423408
draftReportActionID,
424409
firstVisibleReportActionID,
425410
hasPreviousMessages,
@@ -483,6 +468,28 @@ function ReportActionsList({
483468
isTrackIntentUser,
484469
});
485470

471+
const targetIndex = initialScrollKey ? renderedVisibleReportActions.findIndex((item) => keyExtractor(item) === initialScrollKey) : -1;
472+
let initialScrollIndex: number | undefined;
473+
let initialScrollIndexParams: {viewPosition?: number; viewOffset?: number} | undefined;
474+
if (targetIndex > 0) {
475+
initialScrollIndex = targetIndex;
476+
initialScrollIndexParams = {viewPosition: 1, viewOffset: CONST.REPORT.ACTIONS.LINKED_MESSAGE_OFFSET};
477+
} else if (shouldFocusToTopOnMount) {
478+
initialScrollIndex = renderedVisibleReportActions.length - 1;
479+
initialScrollIndexParams = {viewOffset: windowHeight};
480+
}
481+
482+
const maintainVisibleContentPosition = {
483+
disabled: !shouldMaintainVisibleContentPosition,
484+
...(shouldAutoscrollToBottom ? {autoscrollToBottomThreshold: CONST.REPORT.ACTIONS.ACTION_VISIBLE_THRESHOLD, animateAutoScrollToBottom: false} : {}),
485+
};
486+
487+
// When opening a linked message, wait for the first load before rendering the list: the batch of actions that
488+
// arrives right after the initial load shifts the list and breaks the anchor to the linked action.
489+
if (initialScrollKey && !isOffline && !reportLoadingState?.hasOnceLoadedReportActions && reportLoadingState?.isLoadingInitialReportActions) {
490+
return <ReportActionsSkeletonView />;
491+
}
492+
486493
return (
487494
<>
488495
<FloatingMessageCounter
@@ -530,14 +537,10 @@ function ReportActionsList({
530537
contentOffset: shouldFocusToTopOnMount ? {x: 0, y: windowHeight} : undefined,
531538
}}
532539
getItemType={(item) => item.actionName}
533-
shouldMaintainVisibleContentPosition={shouldMaintainVisibleContentPosition}
534-
initialScrollIndex={shouldFocusToTopOnMount ? renderedVisibleReportActions.length - 1 : undefined}
535-
initialScrollIndexParams={shouldFocusToTopOnMount ? {viewOffset: windowHeight} : undefined}
536-
maintainVisibleContentPosition={
537-
shouldAutoscrollToBottom ? {autoscrollToBottomThreshold: CONST.REPORT.ACTIONS.ACTION_VISIBLE_THRESHOLD, animateAutoScrollToBottom: false} : undefined
538-
}
540+
initialScrollIndex={initialScrollIndex}
541+
initialScrollIndexParams={initialScrollIndexParams}
542+
maintainVisibleContentPosition={maintainVisibleContentPosition}
539543
onLoad={onLoad}
540-
initialScrollKey={initialScrollKey}
541544
onContentSizeChange={() => {
542545
trackVerticalScrolling(undefined);
543546
}}

0 commit comments

Comments
 (0)