diff --git a/src/ONYXKEYS.ts b/src/ONYXKEYS.ts index 0a17d3a1d2f7..3aaaff7beda6 100755 --- a/src/ONYXKEYS.ts +++ b/src/ONYXKEYS.ts @@ -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_', diff --git a/src/components/ReportActionsSkeletonView/index.js b/src/components/ReportActionsSkeletonView/index.js index c98470f32613..83a14ecb99e6 100644 --- a/src/components/ReportActionsSkeletonView/index.js +++ b/src/components/ReportActionsSkeletonView/index.js @@ -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: diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index b40851b3ceaa..36f37ac5db7d 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -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. @@ -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}`, @@ -886,13 +833,6 @@ function getNewerActions(reportID, reportActionID) { isLoadingNewerReportActions: false, }, }, - { - onyxMethod: Onyx.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportID}`, - value: { - isLoadingNewerReportActions: false, - }, - }, ], failureData: [ { @@ -902,13 +842,6 @@ function getNewerActions(reportID, reportActionID) { isLoadingNewerReportActions: false, }, }, - { - onyxMethod: Onyx.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportID}`, - value: { - isLoadingNewerReportActions: false, - }, - }, ], }, ); @@ -2330,7 +2263,6 @@ export { expandURLPreview, markCommentAsUnread, readNewestAction, - readOldestAction, openReport, openReportFromDeepLink, navigateToAndOpenReport, diff --git a/src/pages/home/report/ListBoundaryLoader/ListBoundaryLoader.js b/src/pages/home/report/ListBoundaryLoader/ListBoundaryLoader.js index 44a8584cec6d..6e4f23d3a43b 100644 --- a/src/pages/home/report/ListBoundaryLoader/ListBoundaryLoader.js +++ b/src/pages/home/report/ListBoundaryLoader/ListBoundaryLoader.js @@ -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' */ @@ -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 ; + return ( + + + + ); } return null; } diff --git a/src/pages/home/report/ListBoundaryLoader/ListHeaderComponentLoader/ListHeaderComponentLoader.android.js b/src/pages/home/report/ListBoundaryLoader/ListHeaderComponentLoader/ListHeaderComponentLoader.android.js deleted file mode 100644 index 20abe332692d..000000000000 --- a/src/pages/home/report/ListBoundaryLoader/ListHeaderComponentLoader/ListHeaderComponentLoader.android.js +++ /dev/null @@ -1,16 +0,0 @@ -import {View, ActivityIndicator} from 'react-native'; -import styles, {stylesGenerator} from '../../../../../styles/styles'; -import themeColors from '../../../../../styles/themes/default'; - -function ListHeaderComponentLoader() { - return ( - - - - ); -} - -export default ListHeaderComponentLoader; diff --git a/src/pages/home/report/ListBoundaryLoader/ListHeaderComponentLoader/ListHeaderComponentLoader.js b/src/pages/home/report/ListBoundaryLoader/ListHeaderComponentLoader/ListHeaderComponentLoader.js deleted file mode 100644 index 9d4d85a84e87..000000000000 --- a/src/pages/home/report/ListBoundaryLoader/ListHeaderComponentLoader/ListHeaderComponentLoader.js +++ /dev/null @@ -1,17 +0,0 @@ -import React from 'react'; -import {View, ActivityIndicator} from 'react-native'; -import styles, {stylesGenerator} from '../../../../../styles/styles'; -import themeColors from '../../../../../styles/themes/default'; - -function ListHeaderComponentLoader() { - return ( - - - - ); -} - -export default ListHeaderComponentLoader; diff --git a/src/pages/home/report/ReportActionsList.js b/src/pages/home/report/ReportActionsList.js index e8264d148cf9..1acc2f9ba385 100644 --- a/src/pages/home/report/ReportActionsList.js +++ b/src/pages/home/report/ReportActionsList.js @@ -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, @@ -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 */ @@ -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', ''); @@ -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; } @@ -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 ( diff --git a/src/pages/home/report/ReportActionsView.js b/src/pages/home/report/ReportActionsView.js index 046c062d70be..f8ed94618f70 100755 --- a/src/pages/home/report/ReportActionsView.js +++ b/src/pages/home/report/ReportActionsView.js @@ -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 diff --git a/src/styles/boundaryLoaderStyle/index.android.ts b/src/styles/boundaryLoaderStyle/index.android.ts new file mode 100644 index 000000000000..39390cf5ed10 --- /dev/null +++ b/src/styles/boundaryLoaderStyle/index.android.ts @@ -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; diff --git a/src/styles/boundaryLoaderStyle/index.ts b/src/styles/boundaryLoaderStyle/index.ts new file mode 100644 index 000000000000..a8e0a1bf3358 --- /dev/null +++ b/src/styles/boundaryLoaderStyle/index.ts @@ -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; diff --git a/src/styles/boundaryLoaderStyle/types.ts b/src/styles/boundaryLoaderStyle/types.ts new file mode 100644 index 000000000000..d95efa63929c --- /dev/null +++ b/src/styles/boundaryLoaderStyle/types.ts @@ -0,0 +1,5 @@ +import {ViewStyle} from 'react-native'; + +type BoundaryLoaderStyles = Partial>; + +export default BoundaryLoaderStyles; diff --git a/src/types/onyx/Report.ts b/src/types/onyx/Report.ts index a855a91f4ec1..ca81e2c2946a 100644 --- a/src/types/onyx/Report.ts +++ b/src/types/onyx/Report.ts @@ -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;