Skip to content

Commit

Permalink
Merge branch 'feat/#Expensify#23220-bidirectional-pagination' of http…
Browse files Browse the repository at this point in the history
  • Loading branch information
perunt committed Oct 10, 2023
2 parents 8ce9ca0 + bf9c309 commit ab6350b
Show file tree
Hide file tree
Showing 12 changed files with 95 additions and 149 deletions.
2 changes: 1 addition & 1 deletion src/ONYXKEYS.ts
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ const ONYXKEYS = {
POLICY_RECENTLY_USED_TAGS: 'policyRecentlyUsedTags_',
WORKSPACE_INVITE_MEMBERS_DRAFT: 'workspaceInviteMembersDraft_',
REPORT: 'report_',
// REPORT_METADATA is a perf optimization used to hold loading states (isLoadingReportActions, isLoadingMoreReportActions).
// REPORT_METADATA is a perf optimization used to hold loading states (isLoadingInitialReportActions, isLoadingOlderReportActions, isLoadingNewerReportActions).
// A lot of components are connected to the Report entity and do not care about the actions. Setting the loading state
// directly on the report caused a lot of unnecessary re-renders
REPORT_METADATA: 'reportMetadata_',
Expand Down
5 changes: 2 additions & 3 deletions src/components/ReportActionsSkeletonView/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,13 @@ const propTypes = {

const defaultProps = {
shouldAnimate: true,
possibleVisibleContentItems: 0,
possibleVisibleContentItems: Math.ceil(Dimensions.get('window').height / CONST.CHAT_SKELETON_VIEW.AVERAGE_ROW_HEIGHT),
};

function ReportActionsSkeletonView({shouldAnimate, possibleVisibleContentItems}) {
// Determines the number of content items based on container height
const visibleContentItems = possibleVisibleContentItems || Math.ceil(Dimensions.get('window').height / CONST.CHAT_SKELETON_VIEW.AVERAGE_ROW_HEIGHT);
const skeletonViewLines = [];
for (let index = 0; index < visibleContentItems; index++) {
for (let index = 0; index < possibleVisibleContentItems; index++) {
const iconIndex = (index + 1) % 4;
switch (iconIndex) {
case 2:
Expand Down
68 changes: 0 additions & 68 deletions src/libs/actions/Report.js
Original file line number Diff line number Diff line change
Expand Up @@ -755,52 +755,6 @@ function reconnect(reportID) {
);
}

/**
* Gets the older actions that have not been read yet.
* Normally happens when you scroll up on a chat, and the actions have not been read yet.
*
* @param {String} reportID
* @param {String} reportActionID
*/
function readOldestAction(reportID, reportActionID) {
API.read(
'ReadOldestAction',
{
reportID,
reportActionID,
},
{
optimisticData: [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportID}`,
value: {
isLoadingOlderReportActions: true,
},
},
],
successData: [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportID}`,
value: {
isLoadingOlderReportActions: false,
},
},
],
failureData: [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportID}`,
value: {
isLoadingOlderReportActions: true,
},
},
],
},
);
}

/**
* Gets the older actions that have not been read yet.
* Normally happens when you scroll up on a chat, and the actions have not been read yet.
Expand Down Expand Up @@ -863,13 +817,6 @@ function getNewerActions(reportID, reportActionID) {
},
{
optimisticData: [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`,
value: {
isLoadingNewerReportActions: true,
},
},
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportID}`,
Expand All @@ -886,13 +833,6 @@ function getNewerActions(reportID, reportActionID) {
isLoadingNewerReportActions: false,
},
},
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportID}`,
value: {
isLoadingNewerReportActions: false,
},
},
],
failureData: [
{
Expand All @@ -902,13 +842,6 @@ function getNewerActions(reportID, reportActionID) {
isLoadingNewerReportActions: false,
},
},
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportID}`,
value: {
isLoadingNewerReportActions: false,
},
},
],
},
);
Expand Down Expand Up @@ -2330,7 +2263,6 @@ export {
expandURLPreview,
markCommentAsUnread,
readNewestAction,
readOldestAction,
openReport,
openReportFromDeepLink,
navigateToAndOpenReport,
Expand Down
14 changes: 12 additions & 2 deletions src/pages/home/report/ListBoundaryLoader/ListBoundaryLoader.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
import React from 'react';
import {ActivityIndicator, View} from 'react-native';
import PropTypes from 'prop-types';
import ReportActionsSkeletonView from '../../../../components/ReportActionsSkeletonView';
import CONST from '../../../../CONST';
import useNetwork from '../../../../hooks/useNetwork';
import ListHeaderComponentLoader from './ListHeaderComponentLoader/ListHeaderComponentLoader';
import styles, {stylesGenerator} from '../../../../styles/styles';
import themeColors from '../../../../styles/themes/default';
import boundaryLoaderStyles from '../../../../styles/boundaryLoaderStyle/index';

const propTypes = {
/** type of rendered loader. Can be 'header' or 'footer' */
Expand Down Expand Up @@ -55,7 +58,14 @@ function ListBoundaryLoader({type, isLoadingOlderReportActions, isLoadingInitial
if (type === CONST.LIST_COMPONENTS.HEADER && isLoadingNewerReportActions) {
// applied for a header of the list, i.e. when you scroll to the bottom of the list
// the styles for android and the rest components are different that's why we use two different components
return <ListHeaderComponentLoader />;
return (
<View style={[stylesGenerator.alignItemsCenter, styles.justifyContentCenter, styles.chatBottomLoader, boundaryLoaderStyles]}>
<ActivityIndicator
color={themeColors.spinner}
size="small"
/>
</View>
);
}
return null;
}
Expand Down

This file was deleted.

This file was deleted.

18 changes: 13 additions & 5 deletions src/pages/home/report/ReportActionsList.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ const propTypes = {
/** Are we loading more report actions? */
isLoadingOlderReportActions: PropTypes.bool,

/** Are we loading newer report actions? */
isLoadingNewerReportActions: PropTypes.bool,

/** Callback executed on list layout */
onLayout: PropTypes.func.isRequired,

Expand All @@ -48,6 +51,9 @@ const propTypes = {
/** Function to load more chats */
loadOlderChats: PropTypes.func.isRequired,

/** Function to load newer chats */
loadNewerChats: PropTypes.func.isRequired,

/** The policy object for the current route */
policy: PropTypes.shape({
/** The name of the policy */
Expand Down Expand Up @@ -128,7 +134,7 @@ function ReportActionsList({
const [currentUnreadMarker, setCurrentUnreadMarker] = useState(null);
const scrollingVerticalOffset = useRef(0);
const readActionSkipped = useRef(false);
const firstRenderRef = useRef(true);
const firstComponentsRenderRef = useRef({header: true, footer: true});
const reportActionSize = useRef(sortedReportActions.length);
const linkedReportActionID = lodashGet(route, 'params.reportActionID', '');

Expand Down Expand Up @@ -346,8 +352,10 @@ function ReportActionsList({
);

const listFooterComponent = useCallback(() => {
if (firstRenderRef.current) {
firstRenderRef.current = false;
// Skip this hook on the first render, as we are not sure if more actions are going to be loaded
// Therefore showing the skeleton on footer might be misleading
if (firstComponentsRenderRef.current.footer) {
firstComponentsRenderRef.current.footer = false;
return null;
}

Expand All @@ -369,8 +377,8 @@ function ReportActionsList({
);

const listHeaderComponent = useCallback(() => {
if (firstRenderRef.current) {
firstRenderRef.current = false;
if (firstComponentsRenderRef.current.header) {
firstComponentsRenderRef.current.header = false;
return null;
}
return (
Expand Down
64 changes: 36 additions & 28 deletions src/pages/home/report/ReportActionsView.js
Original file line number Diff line number Diff line change
Expand Up @@ -222,34 +222,42 @@ function ReportActionsView({reportActions: allReportActions, ...props}) {
* Retrieves the next set of report actions for the chat once we are nearing the end of what we are currently
* displaying.
*/
const loadNewerChats = _.throttle(({distanceFromStart}) => {
// Only fetch more if we are not already fetching so that we don't initiate duplicate requests.
if (props.isLoadingNewerReportActions || props.isLoadingInitialReportActions) {
return;
}

// Ideally, we wouldn't need to use the 'distanceFromStart' variable. However, due to the low value set for 'maxToRenderPerBatch',
// the component undergoes frequent re-renders. This frequent re-rendering triggers the 'onStartReached' callback multiple times.
//
// To mitigate this issue, we use 'CONST.CHAT_HEADER_LOADER_HEIGHT' as a threshold. This ensures that 'onStartReached' is not
// triggered unnecessarily when the chat is initially opened or when the user has reached the end of the list but hasn't scrolled further.
//
// Additionally, we use throttling on the 'onStartReached' callback to further reduce the frequency of its invocation.
// This should be removed once the issue of frequent re-renders is resolved.
if (!isFetchNewerWasCalled) {
setFetchNewerWasCalled(true);
return;
}
if (!isFetchNewerWasCalled || distanceFromStart <= CONST.CHAT_HEADER_LOADER_HEIGHT) {
return;
}

const newestReportAction = reportActions[0];
Report.getNewerActions(reportID, newestReportAction.reportActionID);
// Report.getNewerActions(reportID, '2420805078232802130');
// Report.getNewerActions(reportID, '569204055949619736');
// Report.getNewerActions(reportID, '1134531619271003224');
}, 500);
const loadNewerChats = useMemo(
() =>
_.throttle(({distanceFromStart}) => {
if (props.isLoadingNewerReportActions || props.isLoadingInitialReportActions) {
return;
}

// Ideally, we wouldn't need to use the 'distanceFromStart' variable. However, due to the low value set for 'maxToRenderPerBatch',
// the component undergoes frequent re-renders. This frequent re-rendering triggers the 'onStartReached' callback multiple times.
//
// To mitigate this issue, we use 'CONST.CHAT_HEADER_LOADER_HEIGHT' as a threshold. This ensures that 'onStartReached' is not
// triggered unnecessarily when the chat is initially opened or when the user has reached the end of the list but hasn't scrolled further.
//
// Additionally, we use throttling on the 'onStartReached' callback to further reduce the frequency of its invocation.
// This should be removed once the issue of frequent re-renders is resolved.
if (!isFetchNewerWasCalled) {
setFetchNewerWasCalled(true);
return;
}
if (!isFetchNewerWasCalled || distanceFromStart <= CONST.CHAT_HEADER_LOADER_HEIGHT) {
return;
}

const newestReportAction = reportActions[0];
Report.getNewerActions(reportID, newestReportAction.reportActionID);
// if (!isFetchNewerWasCalled.current || distanceFromStart <= CONST.CHAT_HEADER_LOADER_HEIGHT) {
// isFetchNewerWasCalled.current = true;
// return;
// }

// const newestReportAction = _.first(props.reportActions);
// Report.getNewerActions(reportID, newestReportAction.reportActionID);
// Report.getNewerActions(reportID, '1134531619271003224');
}, 500),
[props.isLoadingNewerReportActions, props.isLoadingInitialReportActions, props.reportActions, reportID],
);

/**
* Runs when the FlatList finishes laying out
Expand Down
13 changes: 13 additions & 0 deletions src/styles/boundaryLoaderStyle/index.android.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import CONST from '../../CONST';
import BoundaryLoaderStyles from './types';

const boundaryLoaderStyles: BoundaryLoaderStyles = {
position: 'absolute',
bottom: 0,
left: 0,
right: 0,
height: CONST.CHAT_HEADER_LOADER_HEIGHT,
top: -CONST.CHAT_HEADER_LOADER_HEIGHT,
};

export default boundaryLoaderStyles;
13 changes: 13 additions & 0 deletions src/styles/boundaryLoaderStyle/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import CONST from '../../CONST';
import BoundaryLoaderStyles from './types';

const boundaryLoaderStyles: BoundaryLoaderStyles = {
position: 'absolute',
top: 0,
bottom: 0,
left: 0,
right: 0,
height: CONST.CHAT_HEADER_LOADER_HEIGHT,
};

export default boundaryLoaderStyles;
5 changes: 5 additions & 0 deletions src/styles/boundaryLoaderStyle/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import {ViewStyle} from 'react-native';

type BoundaryLoaderStyles = Partial<Pick<ViewStyle, 'position' | 'top' | 'bottom' | 'left' | 'right' | 'height'>>;

export default BoundaryLoaderStyles;
9 changes: 0 additions & 9 deletions src/types/onyx/Report.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,6 @@ type Report = {
/** List of icons for report participants */
icons?: OnyxCommon.Icon[];

/** Are we loading more report actions? */
isLoadingOlderReportActions?: boolean;

/** Are we loading newer report actions? */
isLoadingNewerReportActions?: boolean;

/** Flag to check if the report actions data are loading */
isLoadingInitialReportActions?: boolean;

/** Whether the user is not an admin of policyExpenseChat chat */
isOwnPolicyExpenseChat?: boolean;

Expand Down

0 comments on commit ab6350b

Please sign in to comment.