Skip to content
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

Move logic to start report actions list at index to BaseInvertedFlatList #52149

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 48 additions & 6 deletions src/components/InvertedFlatList/BaseInvertedFlatList/index.tsx
Original file line number Diff line number Diff line change
@@ -1,29 +1,66 @@
import type {ForwardedRef} from 'react';
import React, {forwardRef, useMemo} from 'react';
import type {FlatListProps, FlatList as RNFlatList, ScrollViewProps} from 'react-native';
import React, {forwardRef, useCallback, useMemo, useState} from 'react';
import type {FlatListProps, ListRenderItem, ListRenderItemInfo, FlatList as RNFlatList, ScrollViewProps} from 'react-native';
import FlatList from '@components/FlatList';
import usePrevious from '@hooks/usePrevious';
import getInitialPaginationSize from './getInitialPaginationSize';

type BaseInvertedFlatListProps<T> = FlatListProps<T> & {
type BaseInvertedFlatListProps<T> = Omit<FlatListProps<T>, 'data' | 'renderItem'> & {
shouldEnableAutoScrollToTopThreshold?: boolean;
data: T[];
renderItem: ListRenderItem<T>;
};

const AUTOSCROLL_TO_TOP_THRESHOLD = 250;

function BaseInvertedFlatList<T>(props: BaseInvertedFlatListProps<T>, ref: ForwardedRef<RNFlatList>) {
const {shouldEnableAutoScrollToTopThreshold, ...rest} = props;
const {shouldEnableAutoScrollToTopThreshold, initialScrollIndex, data, onStartReached, renderItem, ...rest} = props;

// `initialScrollIndex` doesn't work properly with FlatList, this uses an alternative approach to achieve the same effect.
// What we do is start rendering the list from `initialScrollIndex` and then whenever we reach the start we render more
// previous items, until everything is rendered.
const [currentDataIndex, setCurrentDataIndex] = useState(initialScrollIndex ?? 0);
const displayedData = useMemo(() => {
if (currentDataIndex > 0) {
return data.slice(currentDataIndex);
}
return data;
}, [data, currentDataIndex]);
const isLoadingData = data.length > displayedData.length;
const wasLoadingData = usePrevious(isLoadingData);
const dataIndexDifference = data.length - displayedData.length;

const handleStartReached = useCallback(
(info: {distanceFromStart: number}) => {
if (isLoadingData) {
setCurrentDataIndex((prevIndex) => prevIndex - getInitialPaginationSize);
} else {
onStartReached?.(info);
}
},
[onStartReached, isLoadingData],
);

const handleRenderItem = useCallback(
({item, index, separators}: ListRenderItemInfo<T>) => {
// Adjust the index passed here so it matches the original data.
return renderItem({item, index: index + dataIndexDifference, separators});
},
[renderItem, dataIndexDifference],
);

const maintainVisibleContentPosition = useMemo(() => {
const config: ScrollViewProps['maintainVisibleContentPosition'] = {
// This needs to be 1 to avoid using loading views as anchors.
minIndexForVisible: 1,
};

if (shouldEnableAutoScrollToTopThreshold) {
if (shouldEnableAutoScrollToTopThreshold && !isLoadingData && !wasLoadingData) {
config.autoscrollToTopThreshold = AUTOSCROLL_TO_TOP_THRESHOLD;
}

return config;
}, [shouldEnableAutoScrollToTopThreshold]);
}, [shouldEnableAutoScrollToTopThreshold, isLoadingData, wasLoadingData]);

return (
<FlatList
Expand All @@ -32,6 +69,9 @@ function BaseInvertedFlatList<T>(props: BaseInvertedFlatListProps<T>, ref: Forwa
ref={ref}
maintainVisibleContentPosition={maintainVisibleContentPosition}
inverted
data={displayedData}
onStartReached={handleStartReached}
renderItem={handleRenderItem}
/>
);
}
Expand All @@ -41,3 +81,5 @@ BaseInvertedFlatList.displayName = 'BaseInvertedFlatList';
export default forwardRef(BaseInvertedFlatList);

export {AUTOSCROLL_TO_TOP_THRESHOLD};

export type {BaseInvertedFlatListProps};
5 changes: 3 additions & 2 deletions src/components/InvertedFlatList/index.native.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import type {ForwardedRef} from 'react';
import React, {forwardRef} from 'react';
import type {FlatList, FlatListProps} from 'react-native';
import type {FlatList} from 'react-native';
import BaseInvertedFlatList from './BaseInvertedFlatList';
import type {BaseInvertedFlatListProps} from './BaseInvertedFlatList';
import CellRendererComponent from './CellRendererComponent';

function BaseInvertedFlatListWithRef<T>(props: FlatListProps<T>, ref: ForwardedRef<FlatList>) {
function BaseInvertedFlatListWithRef<T>(props: BaseInvertedFlatListProps<T>, ref: ForwardedRef<FlatList>) {
return (
<BaseInvertedFlatList
// eslint-disable-next-line react/jsx-props-no-spreading
Expand Down
9 changes: 3 additions & 6 deletions src/components/InvertedFlatList/index.tsx
Original file line number Diff line number Diff line change
@@ -1,18 +1,15 @@
import type {ForwardedRef} from 'react';
import React, {forwardRef, useEffect, useRef} from 'react';
import type {FlatList, FlatListProps, NativeScrollEvent, NativeSyntheticEvent} from 'react-native';
import type {FlatList, NativeScrollEvent, NativeSyntheticEvent} from 'react-native';
import {DeviceEventEmitter} from 'react-native';
import CONST from '@src/CONST';
import BaseInvertedFlatList from './BaseInvertedFlatList';
import type {BaseInvertedFlatListProps} from './BaseInvertedFlatList';
import CellRendererComponent from './CellRendererComponent';

type InvertedFlatListProps<T> = FlatListProps<T> & {
shouldEnableAutoScrollToTopThreshold?: boolean;
};

// This is adapted from https://codesandbox.io/s/react-native-dsyse
// It's a HACK alert since FlatList has inverted scrolling on web
function InvertedFlatList<T>({onScroll: onScrollProp = () => {}, ...props}: InvertedFlatListProps<T>, ref: ForwardedRef<FlatList>) {
function InvertedFlatList<T>({onScroll: onScrollProp = () => {}, ...props}: BaseInvertedFlatListProps<T>, ref: ForwardedRef<FlatList>) {
const lastScrollEvent = useRef<number | null>(null);
const scrollEndTimeout = useRef<NodeJS.Timeout | null>(null);
const updateInProgress = useRef<boolean>(false);
Expand Down
16 changes: 10 additions & 6 deletions src/pages/home/report/ReportActionsList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,6 @@ type ReportActionsListProps = {
/** The transaction thread report associated with the current report, if any */
transactionThreadReport: OnyxEntry<OnyxTypes.Report>;

/** Array of report actions for the current report */
reportActions: OnyxTypes.ReportAction[];

/** The report's parentReportAction */
parentReportAction: OnyxEntry<OnyxTypes.ReportAction>;

Expand Down Expand Up @@ -128,7 +125,6 @@ const onScrollToIndexFailed = () => {};
function ReportActionsList({
report,
transactionThreadReport,
reportActions = [],
parentReportAction,
isLoadingInitialReportActions = false,
isLoadingOlderReportActions = false,
Expand Down Expand Up @@ -582,7 +578,7 @@ function ReportActionsList({
({item: reportAction, index}: ListRenderItemInfo<OnyxTypes.ReportAction>) => (
<ReportActionsListItemRenderer
reportAction={reportAction}
reportActions={reportActions}
reportActions={sortedReportActions}
parentReportAction={parentReportAction}
parentReportActionForTransactionThread={parentReportActionForTransactionThread}
index={index}
Expand All @@ -608,7 +604,7 @@ function ReportActionsList({
mostRecentIOUReportActionID,
shouldHideThreadDividerLine,
parentReportAction,
reportActions,
sortedReportActions,
transactionThreadReport,
parentReportActionForTransactionThread,
shouldUseThreadDividerLine,
Expand Down Expand Up @@ -706,6 +702,13 @@ function ReportActionsList({
loadOlderChats(false);
}, [loadOlderChats]);

const indexOfLinkedAction = useMemo(() => {
if (!linkedReportActionID) {
return -1;
}
return sortedVisibleReportActions.findIndex((obj) => String(obj.reportActionID) === linkedReportActionID);
}, [sortedVisibleReportActions, linkedReportActionID]);

// When performing comment linking, initially 25 items are added to the list. Subsequent fetches add 15 items from the cache or 50 items from the server.
// This is to ensure that the user is able to see the 'scroll to newer comments' button when they do comment linking and have not reached the end of the list yet.
const canScrollToNewerComments = !isLoadingInitialReportActions && !hasNewestReportAction && sortedReportActions.length > 25 && !isLastPendingActionIsDelete;
Expand Down Expand Up @@ -740,6 +743,7 @@ function ReportActionsList({
extraData={extraData}
key={listID}
shouldEnableAutoScrollToTopThreshold={shouldEnableAutoScrollToTopThreshold}
initialScrollIndex={indexOfLinkedAction}
/>
</View>
</>
Expand Down
114 changes: 26 additions & 88 deletions src/pages/home/report/ReportActionsView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import ONYXKEYS from '@src/ONYXKEYS';
import type SCREENS from '@src/SCREENS';
import type * as OnyxTypes from '@src/types/onyx';
import {isEmptyObject} from '@src/types/utils/EmptyObject';
import getInitialPaginationSize from './getInitialPaginationSize';
import PopoverReactionList from './ReactionList/PopoverReactionList';
import ReportActionsList from './ReportActionsList';
import UserTypingEventListener from './UserTypingEventListener';
Expand Down Expand Up @@ -101,9 +100,6 @@ function ReportActionsView({
const didLoadNewerChats = useRef(false);
const {isOffline} = useNetwork();

// triggerListID is used when navigating to a chat with messages loaded from LHN. Typically, these include thread actions, task actions, etc. Since these messages aren't the latest,we don't maintain their position and instead trigger a recalculation of their positioning in the list.
// we don't set currentReportActionID on initial render as linkedID as it should trigger visibleReportActions after linked message was positioned
const [currentReportActionID, setCurrentReportActionID] = useState('');
const isFirstLinkedActionRender = useRef(true);

const network = useNetwork();
Expand Down Expand Up @@ -142,8 +138,6 @@ function ReportActionsView({
// eslint-disable-next-line react-compiler/react-compiler
listOldID = newID;

setCurrentReportActionID('');

return newID;
// eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
}, [route, reportActionID]);
Expand Down Expand Up @@ -210,7 +204,7 @@ function ReportActionsView({

// Get a sorted array of reportActions for both the current report and the transaction thread report associated with this report (if there is one)
// so that we display transaction-level and report-level report actions in order in the one-transaction view
const combinedReportActions = useMemo(
const reportActions = useMemo(
() => ReportActionsUtils.getCombinedReportActions(reportActionsToDisplay, transactionThreadReportID ?? null, transactionThreadReportActions ?? []),
[reportActionsToDisplay, transactionThreadReportActions, transactionThreadReportID],
);
Expand All @@ -223,31 +217,6 @@ function ReportActionsView({
[allReportActions, transactionThreadReportActions, transactionThreadReport?.parentReportActionID],
);

const indexOfLinkedAction = useMemo(() => {
if (!reportActionID) {
return -1;
}
return combinedReportActions.findIndex((obj) => String(obj.reportActionID) === String(isFirstLinkedActionRender.current ? reportActionID : currentReportActionID));
}, [combinedReportActions, currentReportActionID, reportActionID]);

const reportActions = useMemo(() => {
if (!reportActionID) {
return combinedReportActions;
}
if (indexOfLinkedAction === -1) {
return [];
}

if (isFirstLinkedActionRender.current) {
return combinedReportActions.slice(indexOfLinkedAction);
}
const paginationSize = getInitialPaginationSize;
return combinedReportActions.slice(Math.max(indexOfLinkedAction - paginationSize, 0));

// currentReportActionID is needed to trigger batching once the report action has been positioned
// eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
}, [reportActionID, combinedReportActions, indexOfLinkedAction, currentReportActionID]);

const reportActionIDMap = useMemo(() => {
const reportActionIDs = allReportActions.map((action) => action.reportActionID);
return reportActions.map((action) => ({
Expand All @@ -256,33 +225,6 @@ function ReportActionsView({
}));
}, [allReportActions, reportID, transactionThreadReport, reportActions]);

/**
* Retrieves the next set of report actions for the chat once we are nearing the end of what we are currently
* displaying.
*/
const fetchNewerAction = useCallback(
(newestReportAction: OnyxTypes.ReportAction) => {
if (!hasNewerActions || isLoadingNewerReportActions || isLoadingInitialReportActions || (reportActionID && isOffline)) {
return;
}

// If this is a one transaction report, ensure we load newer actions for both this report and the report associated with the transaction
if (!isEmptyObject(transactionThreadReport)) {
// Get newer actions based on the newest reportAction for the current report
const newestActionCurrentReport = reportActionIDMap.find((item) => item.reportID === reportID);
Report.getNewerActions(newestActionCurrentReport?.reportID ?? '-1', newestActionCurrentReport?.reportActionID ?? '-1');

// Get newer actions based on the newest reportAction for the transaction thread report
const newestActionTransactionThreadReport = reportActionIDMap.find((item) => item.reportID === transactionThreadReport.reportID);
Report.getNewerActions(newestActionTransactionThreadReport?.reportID ?? '-1', newestActionTransactionThreadReport?.reportActionID ?? '-1');
} else {
Report.getNewerActions(reportID, newestReportAction.reportActionID);
}
},
[isLoadingNewerReportActions, isLoadingInitialReportActions, reportActionID, isOffline, transactionThreadReport, reportActionIDMap, reportID, hasNewerActions],
);

const hasMoreCached = reportActions.length < combinedReportActions.length;
const newestReportAction = useMemo(() => reportActions?.at(0), [reportActions]);
const mostRecentIOUReportActionID = useMemo(() => ReportActionsUtils.getMostRecentIOURequestActionID(reportActions), [reportActions]);
const hasCachedActionOnFirstRender = useInitialValue(() => reportActions.length > 0);
Expand Down Expand Up @@ -315,23 +257,6 @@ function ReportActionsView({
contentListHeight.current = h;
}, []);

const handleReportActionPagination = useCallback(
({firstReportActionID}: {firstReportActionID: string}) => {
// This function is a placeholder as the actual pagination is handled by visibleReportActions
if (!hasMoreCached && !hasNewestReportAction) {
isFirstLinkedActionRender.current = false;
if (newestReportAction) {
fetchNewerAction(newestReportAction);
}
}
if (isFirstLinkedActionRender.current) {
isFirstLinkedActionRender.current = false;
}
setCurrentReportActionID(firstReportActionID);
},
[fetchNewerAction, hasMoreCached, newestReportAction, hasNewestReportAction],
);

/**
* Retrieves the next set of report actions for the chat once we are nearing the end of what we are currently
* displaying.
Expand Down Expand Up @@ -389,32 +314,46 @@ function ReportActionsView({
!force &&
(!reportActionID ||
!isFocused ||
(isLoadingInitialReportActions && !hasMoreCached) ||
!newestReportAction ||
isLoadingInitialReportActions ||
isLoadingNewerReportActions ||
!hasNewerActions ||
isOffline ||
// If there was an error only try again once on initial mount. We should also still load
// more in case we have cached messages.
(!hasMoreCached && didLoadNewerChats.current && hasLoadingNewerReportActionsError) ||
newestReportAction?.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE)
(didLoadNewerChats.current && hasLoadingNewerReportActionsError) ||
newestReportAction.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE)
) {
return;
}

didLoadNewerChats.current = true;

if ((reportActionID && indexOfLinkedAction > -1) || !reportActionID) {
handleReportActionPagination({firstReportActionID: newestReportAction?.reportActionID ?? '-1'});
// If this is a one transaction report, ensure we load newer actions for both this report and the report associated with the transaction
if (!isEmptyObject(transactionThreadReport)) {
// Get newer actions based on the newest reportAction for the current report
const newestActionCurrentReport = reportActionIDMap.find((item) => item.reportID === reportID);
Report.getNewerActions(newestActionCurrentReport?.reportID ?? '-1', newestActionCurrentReport?.reportActionID ?? '-1');

// Get newer actions based on the newest reportAction for the transaction thread report
const newestActionTransactionThreadReport = reportActionIDMap.find((item) => item.reportID === transactionThreadReport.reportID);
Report.getNewerActions(newestActionTransactionThreadReport?.reportID ?? '-1', newestActionTransactionThreadReport?.reportActionID ?? '-1');
} else if (newestReportAction) {
Report.getNewerActions(reportID, newestReportAction.reportActionID);
}
},
[
isLoadingInitialReportActions,
isLoadingNewerReportActions,
reportActionID,
indexOfLinkedAction,
handleReportActionPagination,
newestReportAction,
isFocused,
newestReportAction,
isLoadingInitialReportActions,
isLoadingNewerReportActions,
hasNewerActions,
isOffline,
hasLoadingNewerReportActionsError,
hasMoreCached,
transactionThreadReport,
reportActionIDMap,
reportID,
],
);

Expand Down Expand Up @@ -477,7 +416,6 @@ function ReportActionsView({
<ReportActionsList
report={report}
transactionThreadReport={transactionThreadReport}
reportActions={reportActions}
parentReportAction={parentReportAction}
parentReportActionForTransactionThread={parentReportActionForTransactionThread}
onLayout={recordTimeToMeasureItemLayout}
Expand Down
1 change: 0 additions & 1 deletion tests/perf-test/ReportActionsList.perf-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,6 @@ function ReportActionsListWrapper() {
loadOlderChats={mockLoadChats}
loadNewerChats={mockLoadChats}
transactionThreadReport={LHNTestUtilsModule.getFakeReport()}
reportActions={ReportTestUtils.getMockedSortedReportActions(500)}
/>
</ActionListContext.Provider>
</ReactionListContext.Provider>
Expand Down
Loading