From 319e605a92f468652b4ee20831dfdbea0153783f Mon Sep 17 00:00:00 2001 From: Mateusz Titz Date: Tue, 8 Apr 2025 12:21:55 +0200 Subject: [PATCH 1/9] Fix scrolling to bottom list on MoneyRequestReportView --- .../MoneyRequestReportActionsList.tsx | 181 ++++++++++-------- .../MoneyRequestReportTransactionList.tsx | 1 + .../useReportScrollManager/index.native.ts | 9 + .../Search/SearchMoneyRequestReportPage.tsx | 52 ++--- src/pages/home/report/ReportActionsList.tsx | 75 ++------ .../shouldDisplayNewMarkerOnReportAction.ts | 87 +++++++++ 6 files changed, 248 insertions(+), 157 deletions(-) create mode 100644 src/pages/home/report/shouldDisplayNewMarkerOnReportAction.ts diff --git a/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx b/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx index 7c5825f598d6..92548fa22cf5 100644 --- a/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx +++ b/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx @@ -1,8 +1,8 @@ import type {ListRenderItemInfo} from '@react-native/virtualized-lists/Lists/VirtualizedList'; import isEmpty from 'lodash/isEmpty'; -import React, {useCallback, useEffect, useMemo, useRef, useState} from 'react'; -import {InteractionManager, View} from 'react-native'; +import React, {useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState} from 'react'; import type {NativeScrollEvent, NativeSyntheticEvent} from 'react-native'; +import {DeviceEventEmitter, InteractionManager, View} from 'react-native'; import type {OnyxCollection, OnyxEntry} from 'react-native-onyx'; import {useOnyx} from 'react-native-onyx'; import FlatList from '@components/FlatList'; @@ -13,6 +13,8 @@ import useNetworkWithOfflineStatus from '@hooks/useNetworkWithOfflineStatus'; import usePrevious from '@hooks/usePrevious'; import useReportScrollManager from '@hooks/useReportScrollManager'; import useThemeStyles from '@hooks/useThemeStyles'; +import DateUtils from '@libs/DateUtils'; +import {parseFSAttributes} from '@libs/Fullstory'; import getNonEmptyStringOnyxID from '@libs/getNonEmptyStringOnyxID'; import {isActionVisibleOnMoneyRequestReport} from '@libs/MoneyRequestReportUtils'; import { @@ -24,8 +26,6 @@ import { isConsecutiveChronosAutomaticTimerAction, isDeletedParentAction, isReportActionUnread, - isReportPreviewAction, - shouldHideNewMarker, shouldReportActionBeVisible, wasMessageReceivedWhileOffline, } from '@libs/ReportActionsUtils'; @@ -34,7 +34,8 @@ import isSearchTopmostFullScreenRoute from '@navigation/helpers/isSearchTopmostF import Navigation from '@navigation/Navigation'; import FloatingMessageCounter from '@pages/home/report/FloatingMessageCounter'; import ReportActionsListItemRenderer from '@pages/home/report/ReportActionsListItemRenderer'; -import {openReport, readNewestAction} from '@userActions/Report'; +import shouldDisplayNewMarkerOnReportAction from '@pages/home/report/shouldDisplayNewMarkerOnReportAction'; +import {openReport, readNewestAction, subscribeToNewActionEvent} from '@userActions/Report'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import ROUTES from '@src/ROUTES'; @@ -49,6 +50,8 @@ import SearchMoneyRequestReportEmptyState from './SearchMoneyRequestReportEmptyS const EmptyParentReportActionForTransactionThread = undefined; const INITIAL_NUM_TO_RENDER = 20; +// Amount of time to wait until all list items should be rendered and scrollToEnd will behave well +const DELAY_FOR_SCROLLING_TO_END = 100; type MoneyRequestReportListProps = { /** The report */ @@ -81,15 +84,12 @@ function selectTransactionsForReportID(transactions: OnyxCollection { const filteredActions = reportActions.filter((reportAction) => { @@ -127,6 +125,18 @@ function MoneyRequestReportActionsList({report, reportActions = [], hasNewerActi return filteredActions.toReversed(); }, [reportActions, isOffline, canPerformWriteAction]); + const reportActionSize = useRef(visibleReportActions.length); + const lastAction = visibleReportActions.at(-1); + const lastActionIndex = lastAction?.reportActionID; + const previousLastIndex = useRef(lastActionIndex); + + const scrollingVerticalOffset = useRef(0); + const readActionSkipped = useRef(false); + const lastVisibleActionCreated = getReportLastVisibleActionCreated(report, transactionThreadReport); + const hasNewestReportAction = lastAction?.created === lastVisibleActionCreated; + const hasNewestReportActionRef = useRef(hasNewestReportAction); + const userActiveSince = useRef(DateUtils.getDBTime()); + const reportActionIDs = useMemo(() => { return reportActions?.map((action) => action.reportActionID) ?? []; }, [reportActions]); @@ -142,27 +152,16 @@ function MoneyRequestReportActionsList({report, reportActions = [], hasNewerActi const onStartReached = useCallback(() => { if (!isSearchTopmostFullScreenRoute()) { - loadNewerChats(false); + loadOlderChats(false); return; } - InteractionManager.runAfterInteractions(() => requestAnimationFrame(() => loadNewerChats(false))); - }, [loadNewerChats]); - - const onEndReached = useCallback(() => { - loadOlderChats(false); + InteractionManager.runAfterInteractions(() => requestAnimationFrame(() => loadOlderChats(false))); }, [loadOlderChats]); - const reportActionSize = useRef(visibleReportActions.length); - const lastAction = visibleReportActions.at(-1); - const lastActionIndex = lastAction?.reportActionID; - const previousLastIndex = useRef(lastActionIndex); - - const scrollingVerticalOffset = useRef(0); - const readActionSkipped = useRef(false); - const lastVisibleActionCreated = getReportLastVisibleActionCreated(report, transactionThreadReport); - const hasNewestReportAction = lastAction?.created === lastVisibleActionCreated; - const hasNewestReportActionRef = useRef(hasNewestReportAction); + const onEndReached = useCallback(() => { + loadNewerChats(false); + }, [loadNewerChats]); useEffect(() => { if ( @@ -218,72 +217,95 @@ function MoneyRequestReportActionsList({report, reportActions = [], hasNewerActi }, [isOffline, lastOfflineAt, lastOnlineAt, preferredLocale, reportActions]); /** - * TODO extract as reusable logic from ReportActionsList - https://github.com/Expensify/App/issues/58891 + * The reportActionID the unread marker should display above */ const unreadMarkerReportActionID = useMemo(() => { - const shouldDisplayNewMarker = (message: OnyxTypes.ReportAction, index: number): boolean => { - const nextMessage = visibleReportActions.at(index + 1); - const isNextMessageUnread = !!nextMessage && isReportActionUnread(nextMessage, unreadMarkerTime); + // If there are message that were received while offline, + // we can skip checking all messages later than the earliest received offline message. + const startIndex = visibleReportActions.length - 1; + const endIndex = earliestReceivedOfflineMessageIndex ?? 0; - // If the current message is the earliest message received while offline, we want to display the unread marker above this message. + // Scan through each visible report action until we find the appropriate action to show the unread marker + for (let index = startIndex; index >= endIndex; index--) { + const reportAction = visibleReportActions.at(index); + const nextAction = visibleReportActions.at(index - 1); + const isNextMessageUnread = !!nextAction && isReportActionUnread(nextAction, unreadMarkerTime); const isEarliestReceivedOfflineMessage = index === earliestReceivedOfflineMessageIndex; - if (isEarliestReceivedOfflineMessage && !isNextMessageUnread) { - return true; - } - - // If the unread marker should be hidden or is not within the visible area, don't show the unread marker. - if (shouldHideNewMarker(message)) { - return false; - } - const isCurrentMessageUnread = isReportActionUnread(message, unreadMarkerTime); + const shouldDisplayNewMarker = + reportAction && + shouldDisplayNewMarkerOnReportAction({ + message: reportAction, + isNextMessageUnread, + isEarliestReceivedOfflineMessage, + accountID: currentUserAccountID, + prevSortedVisibleReportActionsObjects: prevVisibleActionsMap, + unreadMarkerTime, + scrollingVerticalOffset: scrollingVerticalOffset.current, + prevUnreadMarkerReportActionID: prevUnreadMarkerReportActionID.current, + }); - // If the current message is read or the next message is unread, don't show the unread marker. - if (!isCurrentMessageUnread || isNextMessageUnread) { - return false; + // eslint-disable-next-line react-compiler/react-compiler + if (shouldDisplayNewMarker) { + return reportAction.reportActionID; } + } - const isPendingAdd = (action: OnyxTypes.ReportAction) => { - return action?.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD; - }; - - // If no unread marker exists, don't set an unread marker for newly added messages from the current user. - const isFromCurrentUser = currentUserAccountID === (isReportPreviewAction(message) ? message.childLastActorAccountID : message.actorAccountID); - const isNewMessage = !prevVisibleActionsMap[message.reportActionID]; - - // The unread marker will show if the action's `created` time is later than `unreadMarkerTime`. - // The `unreadMarkerTime` has already been updated to match the optimistic action created time, - // but once the new action is saved on the backend, the actual created time will be later than the optimistic one. - // Therefore, we also need to prevent the unread marker from appearing for previously optimistic actions. - const isPreviouslyOptimistic = - (isPendingAdd(prevVisibleActionsMap[message.reportActionID]) && !isPendingAdd(message)) || - (!!prevVisibleActionsMap[message.reportActionID]?.isOptimisticAction && !message.isOptimisticAction); - const shouldIgnoreUnreadForCurrentUserMessage = !prevUnreadMarkerReportActionID.current && isFromCurrentUser && (isNewMessage || isPreviouslyOptimistic); - - if (isFromCurrentUser) { - return !shouldIgnoreUnreadForCurrentUserMessage; - } + return null; + }, [currentUserAccountID, earliestReceivedOfflineMessageIndex, prevVisibleActionsMap, visibleReportActions, unreadMarkerTime]); + prevUnreadMarkerReportActionID.current = unreadMarkerReportActionID; + + /** + * Subscribe to read/unread events and update our unreadMarkerTime + */ + useEffect(() => { + const unreadActionSubscription = DeviceEventEmitter.addListener(`unreadAction_${report.reportID}`, (newLastReadTime: string) => { + setUnreadMarkerTime(newLastReadTime); + userActiveSince.current = DateUtils.getDBTime(); + }); + const readNewestActionSubscription = DeviceEventEmitter.addListener(`readNewestAction_${report.reportID}`, (newLastReadTime: string) => { + setUnreadMarkerTime(newLastReadTime); + }); - return !isNewMessage || scrollingVerticalOffset.current >= CONST.REPORT.ACTIONS.ACTION_VISIBLE_THRESHOLD; + return () => { + unreadActionSubscription.remove(); + readNewestActionSubscription.remove(); }; + }, [report.reportID]); - // If there are message that were recevied while offline, - // we can skip checking all messages later than the earliest recevied offline message. - const startIndex = earliestReceivedOfflineMessageIndex ?? 0; + const scrollToBottomForCurrentUserAction = useCallback( + (isFromCurrentUser: boolean) => { + InteractionManager.runAfterInteractions(() => { + setIsFloatingMessageCounterVisible(false); + // If a new comment is added from the current user, scroll to the bottom, otherwise leave the user position unchanged + if (!isFromCurrentUser) { + return; + } + + // We want to scroll to the end of the list where the newest message is + // however scrollToEnd will not work correctly with items of variable sizes without `getItemLayout` - so we need to delay the scroll until every item rendered + setTimeout(() => { + reportScrollManager.scrollToEnd(); + }, DELAY_FOR_SCROLLING_TO_END); + }); + }, + [reportScrollManager], + ); - // Scan through each visible report action until we find the appropriate action to show the unread marker - for (let index = startIndex; index < visibleReportActions.length; index++) { - const reportAction = visibleReportActions.at(index); + useEffect(() => { + // This callback is triggered when a new action arrives via Pusher and the event is emitted from Report.js. This allows us to maintain + // a single source of truth for the "new action" event instead of trying to derive that a new action has appeared from looking at props. + const unsubscribe = subscribeToNewActionEvent(report.reportID, scrollToBottomForCurrentUserAction); - // eslint-disable-next-line react-compiler/react-compiler - if (reportAction && shouldDisplayNewMarker(reportAction, index)) { - return reportAction.reportActionID; + return () => { + if (!unsubscribe) { + return; } - } + unsubscribe(); + }; - return null; - }, [currentUserAccountID, earliestReceivedOfflineMessageIndex, prevVisibleActionsMap, visibleReportActions, unreadMarkerTime]); - prevUnreadMarkerReportActionID.current = unreadMarkerReportActionID; + // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps + }, [report.reportID]); const renderItem = useCallback( ({item: reportAction, index}: ListRenderItemInfo) => { @@ -351,6 +373,9 @@ function MoneyRequestReportActionsList({report, reportActions = [], hasNewerActi handleUnreadFloatingButton(); }; + // Parse Fullstory attributes on initial render + useLayoutEffect(parseFSAttributes, []); + return ( diff --git a/src/components/MoneyRequestReportView/MoneyRequestReportTransactionList.tsx b/src/components/MoneyRequestReportView/MoneyRequestReportTransactionList.tsx index f901c11ea25f..dac76981ea4f 100644 --- a/src/components/MoneyRequestReportView/MoneyRequestReportTransactionList.tsx +++ b/src/components/MoneyRequestReportView/MoneyRequestReportTransactionList.tsx @@ -180,6 +180,7 @@ function MoneyRequestReportTransactionList({report, transactions, reportActions, id={transaction.transactionID} style={[pressableStyle]} onMouseLeave={handleMouseLeave} + key={transaction.transactionID} > - - - - + + + + + + + + + ); } diff --git a/src/pages/home/report/ReportActionsList.tsx b/src/pages/home/report/ReportActionsList.tsx index da9f0dadfeaf..6ebcf4b10078 100644 --- a/src/pages/home/report/ReportActionsList.tsx +++ b/src/pages/home/report/ReportActionsList.tsx @@ -1,7 +1,5 @@ import type {ListRenderItemInfo} from '@react-native/virtualized-lists/Lists/VirtualizedList'; import {useIsFocused, useRoute} from '@react-navigation/native'; -// eslint-disable-next-line lodash/import-scope -import type {DebouncedFunc} from 'lodash'; import React, {memo, useCallback, useContext, useEffect, useLayoutEffect, useMemo, useRef, useState} from 'react'; import type {LayoutChangeEvent, NativeScrollEvent, NativeSyntheticEvent} from 'react-native'; import {DeviceEventEmitter, InteractionManager, View} from 'react-native'; @@ -35,7 +33,6 @@ import { isReportPreviewAction, isReversedTransaction, isTransactionThread, - shouldHideNewMarker, wasMessageReceivedWhileOffline, } from '@libs/ReportActionsUtils'; import { @@ -65,8 +62,7 @@ import FloatingMessageCounter from './FloatingMessageCounter'; import getInitialNumToRender from './getInitialNumReportActionsToRender'; import ListBoundaryLoader from './ListBoundaryLoader'; import ReportActionsListItemRenderer from './ReportActionsListItemRenderer'; - -type LoadNewerChats = DebouncedFunc<(params: {distanceFromStart: number}) => void>; +import shouldDisplayNewMarkerOnReportAction from './shouldDisplayNewMarkerOnReportAction'; type ReportActionsListProps = { /** The report currently being looked at */ @@ -237,62 +233,31 @@ function ReportActionsList({ * The reportActionID the unread marker should display above */ const unreadMarkerReportActionID = useMemo(() => { - const shouldDisplayNewMarker = (message: OnyxTypes.ReportAction, index: number): boolean => { - const nextMessage = sortedVisibleReportActions.at(index + 1); - const isNextMessageUnread = !!nextMessage && isReportActionUnread(nextMessage, unreadMarkerTime); - - // If the current message is the earliest message received while offline, we want to display the unread marker above this message. - const isEarliestReceivedOfflineMessage = index === earliestReceivedOfflineMessageIndex; - if (isEarliestReceivedOfflineMessage && !isNextMessageUnread) { - return true; - } - - // If the unread marker should be hidden or is not within the visible area, don't show the unread marker. - if (shouldHideNewMarker(message)) { - return false; - } - - const isCurrentMessageUnread = isReportActionUnread(message, unreadMarkerTime); - - // If the current message is read or the next message is unread, don't show the unread marker. - if (!isCurrentMessageUnread || isNextMessageUnread) { - return false; - } - - const isPendingAdd = (action: OnyxTypes.ReportAction) => { - return action?.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD; - }; - - // If no unread marker exists, don't set an unread marker for newly added messages from the current user. - const isFromCurrentUser = accountID === (isReportPreviewAction(message) ? message.childLastActorAccountID : message.actorAccountID); - const isNewMessage = !prevSortedVisibleReportActionsObjects[message.reportActionID]; - - // The unread marker will show if the action's `created` time is later than `unreadMarkerTime`. - // The `unreadMarkerTime` has already been updated to match the optimistic action created time, - // but once the new action is saved on the backend, the actual created time will be later than the optimistic one. - // Therefore, we also need to prevent the unread marker from appearing for previously optimistic actions. - const isPreviouslyOptimistic = - (isPendingAdd(prevSortedVisibleReportActionsObjects[message.reportActionID]) && !isPendingAdd(message)) || - (!!prevSortedVisibleReportActionsObjects[message.reportActionID]?.isOptimisticAction && !message.isOptimisticAction); - const shouldIgnoreUnreadForCurrentUserMessage = !prevUnreadMarkerReportActionID.current && isFromCurrentUser && (isNewMessage || isPreviouslyOptimistic); - - if (isFromCurrentUser) { - return !shouldIgnoreUnreadForCurrentUserMessage; - } - - return !isNewMessage || scrollingVerticalOffset.current >= CONST.REPORT.ACTIONS.ACTION_VISIBLE_THRESHOLD; - }; - - // If there are message that were recevied while offline, - // we can skip checking all messages later than the earliest recevied offline message. + // If there are message that were received while offline, + // we can skip checking all messages later than the earliest received offline message. const startIndex = earliestReceivedOfflineMessageIndex ?? 0; // Scan through each visible report action until we find the appropriate action to show the unread marker for (let index = startIndex; index < sortedVisibleReportActions.length; index++) { const reportAction = sortedVisibleReportActions.at(index); + const nextAction = sortedVisibleReportActions.at(index + 1); + const isNextMessageUnread = !!nextAction && isReportActionUnread(nextAction, unreadMarkerTime); + const isEarliestReceivedOfflineMessage = index === earliestReceivedOfflineMessageIndex; // eslint-disable-next-line react-compiler/react-compiler - if (reportAction && shouldDisplayNewMarker(reportAction, index)) { + const shouldDisplayNewMarker = + reportAction && + shouldDisplayNewMarkerOnReportAction({ + message: reportAction, + isNextMessageUnread, + isEarliestReceivedOfflineMessage, + accountID, + prevSortedVisibleReportActionsObjects, + unreadMarkerTime, + scrollingVerticalOffset: scrollingVerticalOffset.current, + prevUnreadMarkerReportActionID: prevUnreadMarkerReportActionID.current, + }); + if (shouldDisplayNewMarker) { return reportAction.reportActionID; } } @@ -774,4 +739,4 @@ ReportActionsList.displayName = 'ReportActionsList'; export default memo(ReportActionsList); -export type {LoadNewerChats, ReportActionsListProps}; +export type {ReportActionsListProps}; diff --git a/src/pages/home/report/shouldDisplayNewMarkerOnReportAction.ts b/src/pages/home/report/shouldDisplayNewMarkerOnReportAction.ts new file mode 100644 index 000000000000..2e75e330f763 --- /dev/null +++ b/src/pages/home/report/shouldDisplayNewMarkerOnReportAction.ts @@ -0,0 +1,87 @@ +import {isReportActionUnread, isReportPreviewAction, shouldHideNewMarker} from '@libs/ReportActionsUtils'; +import CONST from '@src/CONST'; +import type * as OnyxTypes from '@src/types/onyx'; + +type ShouldDisplayNewMarkerOnReportActionParams = { + /** The reportAction for which the check is done */ + message: OnyxTypes.ReportAction; + + /** Whether the next message in a list is unread */ + isNextMessageUnread: boolean; + + /** Is it the earliestReceivedOfflineMessage */ + isEarliestReceivedOfflineMessage: boolean; + + /** Time for unreadMarker */ + unreadMarkerTime: string | undefined; + + /** User accountID */ + accountID: number | undefined; + + /** Map of reportActions saved via usePrev */ + prevSortedVisibleReportActionsObjects: Record; + + /** Current value for vertical offset */ + scrollingVerticalOffset: number; + + /** Current value for vertical offset */ + prevUnreadMarkerReportActionID: string | null; +}; + +/** + * This function decides whether the given report action (message) should have the new marker indicator displayed + * It's used for the standard "chat" Report and for `MoneyRequestReport` actions lists. + */ +const shouldDisplayNewMarkerOnReportAction = ({ + message, + isNextMessageUnread, + isEarliestReceivedOfflineMessage, + unreadMarkerTime, + accountID, + prevSortedVisibleReportActionsObjects, + prevUnreadMarkerReportActionID, + scrollingVerticalOffset, +}: ShouldDisplayNewMarkerOnReportActionParams): boolean => { + // If the current message is the earliest message received while offline, we want to display the unread marker above this message. + // const isEarliestReceivedOfflineMessage = index === earliestReceivedOfflineMessageIndex; + if (isEarliestReceivedOfflineMessage && !isNextMessageUnread) { + return true; + } + + // If the unread marker should be hidden or is not within the visible area, don't show the unread marker. + if (shouldHideNewMarker(message)) { + return false; + } + + const isCurrentMessageUnread = isReportActionUnread(message, unreadMarkerTime); + + // If the current message is read or the next message is unread, don't show the unread marker. + if (!isCurrentMessageUnread || isNextMessageUnread) { + return false; + } + + const isPendingAdd = (action: OnyxTypes.ReportAction) => { + return action?.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD; + }; + + // If no unread marker exists, don't set an unread marker for newly added messages from the current user. + const isFromCurrentUser = accountID === (isReportPreviewAction(message) ? message.childLastActorAccountID : message.actorAccountID); + const isNewMessage = !prevSortedVisibleReportActionsObjects[message.reportActionID]; + + // The unread marker will show if the action's `created` time is later than `unreadMarkerTime`. + // The `unreadMarkerTime` has already been updated to match the optimistic action created time, + // but once the new action is saved on the backend, the actual created time will be later than the optimistic one. + // Therefore, we also need to prevent the unread marker from appearing for previously optimistic actions. + const isPreviouslyOptimistic = + (isPendingAdd(prevSortedVisibleReportActionsObjects[message.reportActionID]) && !isPendingAdd(message)) || + (!!prevSortedVisibleReportActionsObjects[message.reportActionID]?.isOptimisticAction && !message.isOptimisticAction); + const shouldIgnoreUnreadForCurrentUserMessage = !prevUnreadMarkerReportActionID && isFromCurrentUser && (isNewMessage || isPreviouslyOptimistic); + + if (isFromCurrentUser) { + return !shouldIgnoreUnreadForCurrentUserMessage; + } + + return !isNewMessage || scrollingVerticalOffset >= CONST.REPORT.ACTIONS.ACTION_VISIBLE_THRESHOLD; +}; + +export default shouldDisplayNewMarkerOnReportAction; From d2dc97cf5092124eff994535c69fdcd568f17d94 Mon Sep 17 00:00:00 2001 From: Mateusz Titz Date: Thu, 10 Apr 2025 16:20:57 +0200 Subject: [PATCH 2/9] Fix displaying unread marker correctly --- .../MoneyRequestReportActionsList.tsx | 27 +++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx b/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx index 92548fa22cf5..95cf5fabb6f3 100644 --- a/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx +++ b/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx @@ -189,6 +189,8 @@ function MoneyRequestReportActionsList({report, reportActions = [], hasNewerActi }, [visibleReportActions]); const prevVisibleActionsMap = usePrevious(visibleActionsMap); + const reportLastReadTime = report.lastReadTime ?? ''; + /** * The timestamp for the unread marker. * @@ -197,9 +199,9 @@ function MoneyRequestReportActionsList({report, reportActions = [], hasNewerActi * - marks a message as read/unread * - reads a new message as it is received */ - const [unreadMarkerTime, setUnreadMarkerTime] = useState(report.lastReadTime); + const [unreadMarkerTime, setUnreadMarkerTime] = useState(reportLastReadTime); useEffect(() => { - setUnreadMarkerTime(report.lastReadTime); + setUnreadMarkerTime(reportLastReadTime); // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps }, [report.reportID]); @@ -273,6 +275,27 @@ function MoneyRequestReportActionsList({report, reportActions = [], hasNewerActi }; }, [report.reportID]); + /** + * When the user reads a new message as it is received, we'll push the unreadMarkerTime down to the timestamp of + * the latest report action. When new report actions are received and the user is not viewing them (they're above + * the MSG_VISIBLE_THRESHOLD), the unread marker will display over those new messages rather than the initial + * lastReadTime. + */ + useLayoutEffect(() => { + if (unreadMarkerReportActionID) { + return; + } + + const mostRecentReportActionCreated = lastAction?.created ?? ''; + if (mostRecentReportActionCreated <= unreadMarkerTime) { + return; + } + + setUnreadMarkerTime(mostRecentReportActionCreated); + + // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps + }, [lastAction?.created]); + const scrollToBottomForCurrentUserAction = useCallback( (isFromCurrentUser: boolean) => { InteractionManager.runAfterInteractions(() => { From 6607831aab9766f0e2b5f86d3909e76a6689264f Mon Sep 17 00:00:00 2001 From: Mateusz Titz Date: Fri, 11 Apr 2025 08:36:58 +0200 Subject: [PATCH 3/9] Disable LoadingBar on SearchMoneyRequestReportPage Done in order to make the view feel more fluid and less jumpy. --- src/components/Navigation/TopBar.tsx | 5 +++-- src/pages/Search/SearchMoneyRequestReportPage.tsx | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/components/Navigation/TopBar.tsx b/src/components/Navigation/TopBar.tsx index 634d7b1b12f3..d28845f133fa 100644 --- a/src/components/Navigation/TopBar.tsx +++ b/src/components/Navigation/TopBar.tsx @@ -21,10 +21,11 @@ type TopBarProps = { activeWorkspaceID?: string; shouldDisplaySearch?: boolean; shouldDisplayHelpButton?: boolean; + shouldDisplayLoadingBar?: boolean; cancelSearch?: () => void; }; -function TopBar({breadcrumbLabel, activeWorkspaceID, shouldDisplaySearch = true, shouldDisplayHelpButton = true, cancelSearch}: TopBarProps) { +function TopBar({breadcrumbLabel, activeWorkspaceID, shouldDisplaySearch = true, shouldDisplayHelpButton = true, shouldDisplayLoadingBar = true, cancelSearch}: TopBarProps) { const styles = useThemeStyles(); const {translate} = useLocalize(); const policy = usePolicy(activeWorkspaceID); @@ -76,7 +77,7 @@ function TopBar({breadcrumbLabel, activeWorkspaceID, shouldDisplaySearch = true, {shouldDisplayHelpButton && } {displaySearch && } - + {shouldDisplayLoadingBar && } ); } diff --git a/src/pages/Search/SearchMoneyRequestReportPage.tsx b/src/pages/Search/SearchMoneyRequestReportPage.tsx index e9d4b1122690..b2aaa97e3194 100644 --- a/src/pages/Search/SearchMoneyRequestReportPage.tsx +++ b/src/pages/Search/SearchMoneyRequestReportPage.tsx @@ -131,6 +131,7 @@ function SearchMoneyRequestReportPage({route}: SearchMoneyRequestPageProps) { From acf0d2fd5381b0760433acabbdd52add3d7c7959 Mon Sep 17 00:00:00 2001 From: Mateusz Titz Date: Fri, 11 Apr 2025 08:49:11 +0200 Subject: [PATCH 4/9] Fix conflicts after merge --- .../MoneyRequestReportActionsList.tsx | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx b/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx index 01b35e9c209f..8c92b2afb1cc 100644 --- a/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx +++ b/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx @@ -1,7 +1,6 @@ import type {ListRenderItemInfo} from '@react-native/virtualized-lists/Lists/VirtualizedList'; import isEmpty from 'lodash/isEmpty'; -import React, {useCallback, useEffect, useMemo, useRef, useState} from 'react'; -import {InteractionManager, View} from 'react-native'; +import React, {useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState} from 'react'; import type {NativeScrollEvent, NativeSyntheticEvent} from 'react-native'; import {DeviceEventEmitter, InteractionManager, View} from 'react-native'; import type {OnyxEntry} from 'react-native-onyx'; @@ -76,16 +75,6 @@ function getParentReportAction(parentReportActions: OnyxEntry, reportID: string, reportActions: OnyxTypes.ReportAction[]) { - return Object.values(transactions ?? {}).filter((transaction): transaction is Transaction => { - if (!transaction) { - return false; - } - const action = getIOUActionForTransactionID(reportActions, transaction.transactionID); - return transaction.reportID === reportID && !isDeletedParentAction(action); - }); -} - function MoneyRequestReportActionsList({report, reportActions = [], transactions = [], hasNewerActions, hasOlderActions}: MoneyRequestReportListProps) { const styles = useThemeStyles(); const {translate} = useLocalize(); From fd5eaf29e2fa047b9275432b3da8287a05bc0f69 Mon Sep 17 00:00:00 2001 From: Mateusz Titz Date: Mon, 14 Apr 2025 11:32:23 +0200 Subject: [PATCH 5/9] Fix displaying unread floating button on MoneyRequestReportActionsList --- src/CONST.ts | 2 +- .../MoneyRequestReportActionsList.tsx | 22 +++++++++++++------ 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/src/CONST.ts b/src/CONST.ts index 643a7d4dca61..887942fb3c7a 100755 --- a/src/CONST.ts +++ b/src/CONST.ts @@ -1319,7 +1319,7 @@ const CONST = { }, }, THREAD_DISABLED: ['CREATED'], - // Used when displaying reportActions list to handle of unread messages icon/button + // Used when displaying reportActions list to handle unread messages icon/button SCROLL_VERTICAL_OFFSET_THRESHOLD: 200, ACTION_VISIBLE_THRESHOLD: 250, }, diff --git a/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx b/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx index 8c92b2afb1cc..4eba2daadd83 100644 --- a/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx +++ b/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx @@ -118,7 +118,7 @@ function MoneyRequestReportActionsList({report, reportActions = [], transactions const lastActionIndex = lastAction?.reportActionID; const previousLastIndex = useRef(lastActionIndex); - const scrollingVerticalOffset = useRef(0); + const scrollingVerticalBottomOffset = useRef(0); const readActionSkipped = useRef(false); const lastVisibleActionCreated = getReportLastVisibleActionCreated(report, transactionThreadReport); const hasNewestReportAction = lastAction?.created === lastVisibleActionCreated; @@ -153,7 +153,7 @@ function MoneyRequestReportActionsList({report, reportActions = [], transactions useEffect(() => { if ( - scrollingVerticalOffset.current < AUTOSCROLL_TO_TOP_THRESHOLD && + scrollingVerticalBottomOffset.current < AUTOSCROLL_TO_TOP_THRESHOLD && previousLastIndex.current !== lastActionIndex && reportActionSize.current > reportActions.length && hasNewestReportAction @@ -231,7 +231,7 @@ function MoneyRequestReportActionsList({report, reportActions = [], transactions accountID: currentUserAccountID, prevSortedVisibleReportActionsObjects: prevVisibleActionsMap, unreadMarkerTime, - scrollingVerticalOffset: scrollingVerticalOffset.current, + scrollingVerticalOffset: scrollingVerticalBottomOffset.current, prevUnreadMarkerReportActionID: prevUnreadMarkerReportActionID.current, }); @@ -365,11 +365,11 @@ function MoneyRequestReportActionsList({report, reportActions = [], transactions * Show/hide the new floating message counter when user is scrolling back/forth in the history of messages. */ const handleUnreadFloatingButton = () => { - if (scrollingVerticalOffset.current > CONST.REPORT.ACTIONS.SCROLL_VERTICAL_OFFSET_THRESHOLD && !isFloatingMessageCounterVisible && !!unreadMarkerReportActionID) { + if (scrollingVerticalBottomOffset.current > CONST.REPORT.ACTIONS.SCROLL_VERTICAL_OFFSET_THRESHOLD && !isFloatingMessageCounterVisible && !!unreadMarkerReportActionID) { setIsFloatingMessageCounterVisible(true); } - if (scrollingVerticalOffset.current < CONST.REPORT.ACTIONS.SCROLL_VERTICAL_OFFSET_THRESHOLD && isFloatingMessageCounterVisible) { + if (scrollingVerticalBottomOffset.current < CONST.REPORT.ACTIONS.SCROLL_VERTICAL_OFFSET_THRESHOLD && isFloatingMessageCounterVisible) { if (readActionSkipped.current) { readActionSkipped.current = false; readNewestAction(report.reportID); @@ -378,12 +378,20 @@ function MoneyRequestReportActionsList({report, reportActions = [], transactions } }; - const reportHasComments = visibleReportActions.length > 0; const trackVerticalScrolling = (event: NativeSyntheticEvent) => { - scrollingVerticalOffset.current = event.nativeEvent.contentOffset.y; + const {layoutMeasurement, contentSize, contentOffset} = event.nativeEvent; + const fullContentHeight = contentSize.height; + + /** + * Count the diff between current scroll position and the bottom of the list. + * Diff == (height of all items in the list) - (height of the layout with the list) - (how far user scrolled) + */ + scrollingVerticalBottomOffset.current = fullContentHeight - layoutMeasurement.height - contentOffset.y; handleUnreadFloatingButton(); }; + const reportHasComments = visibleReportActions.length > 0; + // Parse Fullstory attributes on initial render useLayoutEffect(parseFSAttributes, []); From be2f6a7688d5293e70458660617cfa6417c6418a Mon Sep 17 00:00:00 2001 From: Mateusz Titz Date: Mon, 14 Apr 2025 17:09:06 +0200 Subject: [PATCH 6/9] Fix TopBar after changes from main --- src/pages/Search/SearchMoneyRequestReportPage.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/Search/SearchMoneyRequestReportPage.tsx b/src/pages/Search/SearchMoneyRequestReportPage.tsx index b2aaa97e3194..e7585e146246 100644 --- a/src/pages/Search/SearchMoneyRequestReportPage.tsx +++ b/src/pages/Search/SearchMoneyRequestReportPage.tsx @@ -131,7 +131,7 @@ function SearchMoneyRequestReportPage({route}: SearchMoneyRequestPageProps) { From bc7345e8c1f102b505fb5aee15db8630af01cb5b Mon Sep 17 00:00:00 2001 From: Mateusz Titz Date: Tue, 15 Apr 2025 11:35:10 +0200 Subject: [PATCH 7/9] Improve shouldDisplayNewMarkerOnReportAction a bit --- .../MoneyRequestReportActionsList.tsx | 4 +--- src/pages/home/report/ReportActionsList.tsx | 4 +--- .../home/report/shouldDisplayNewMarkerOnReportAction.ts | 9 +++++---- 3 files changed, 7 insertions(+), 10 deletions(-) diff --git a/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx b/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx index 4eba2daadd83..75e4a1544a63 100644 --- a/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx +++ b/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx @@ -24,7 +24,6 @@ import { hasNextActionMadeBySameActor, isConsecutiveChronosAutomaticTimerAction, isDeletedParentAction, - isReportActionUnread, shouldReportActionBeVisible, wasMessageReceivedWhileOffline, } from '@libs/ReportActionsUtils'; @@ -219,14 +218,13 @@ function MoneyRequestReportActionsList({report, reportActions = [], transactions for (let index = startIndex; index >= endIndex; index--) { const reportAction = visibleReportActions.at(index); const nextAction = visibleReportActions.at(index - 1); - const isNextMessageUnread = !!nextAction && isReportActionUnread(nextAction, unreadMarkerTime); const isEarliestReceivedOfflineMessage = index === earliestReceivedOfflineMessageIndex; const shouldDisplayNewMarker = reportAction && shouldDisplayNewMarkerOnReportAction({ message: reportAction, - isNextMessageUnread, + nextMessage: nextAction, isEarliestReceivedOfflineMessage, accountID: currentUserAccountID, prevSortedVisibleReportActionsObjects: prevVisibleActionsMap, diff --git a/src/pages/home/report/ReportActionsList.tsx b/src/pages/home/report/ReportActionsList.tsx index 6ebcf4b10078..1ad42f612ab0 100644 --- a/src/pages/home/report/ReportActionsList.tsx +++ b/src/pages/home/report/ReportActionsList.tsx @@ -29,7 +29,6 @@ import { isConsecutiveActionMadeByPreviousActor, isConsecutiveChronosAutomaticTimerAction, isDeletedParentAction, - isReportActionUnread, isReportPreviewAction, isReversedTransaction, isTransactionThread, @@ -241,7 +240,6 @@ function ReportActionsList({ for (let index = startIndex; index < sortedVisibleReportActions.length; index++) { const reportAction = sortedVisibleReportActions.at(index); const nextAction = sortedVisibleReportActions.at(index + 1); - const isNextMessageUnread = !!nextAction && isReportActionUnread(nextAction, unreadMarkerTime); const isEarliestReceivedOfflineMessage = index === earliestReceivedOfflineMessageIndex; // eslint-disable-next-line react-compiler/react-compiler @@ -249,7 +247,7 @@ function ReportActionsList({ reportAction && shouldDisplayNewMarkerOnReportAction({ message: reportAction, - isNextMessageUnread, + nextMessage: nextAction, isEarliestReceivedOfflineMessage, accountID, prevSortedVisibleReportActionsObjects, diff --git a/src/pages/home/report/shouldDisplayNewMarkerOnReportAction.ts b/src/pages/home/report/shouldDisplayNewMarkerOnReportAction.ts index 2e75e330f763..b850183c9bbc 100644 --- a/src/pages/home/report/shouldDisplayNewMarkerOnReportAction.ts +++ b/src/pages/home/report/shouldDisplayNewMarkerOnReportAction.ts @@ -6,8 +6,8 @@ type ShouldDisplayNewMarkerOnReportActionParams = { /** The reportAction for which the check is done */ message: OnyxTypes.ReportAction; - /** Whether the next message in a list is unread */ - isNextMessageUnread: boolean; + /** The reportAction adjacent to `message` (either previous or next one) */ + nextMessage: OnyxTypes.ReportAction | undefined; /** Is it the earliestReceivedOfflineMessage */ isEarliestReceivedOfflineMessage: boolean; @@ -34,7 +34,7 @@ type ShouldDisplayNewMarkerOnReportActionParams = { */ const shouldDisplayNewMarkerOnReportAction = ({ message, - isNextMessageUnread, + nextMessage, isEarliestReceivedOfflineMessage, unreadMarkerTime, accountID, @@ -42,8 +42,9 @@ const shouldDisplayNewMarkerOnReportAction = ({ prevUnreadMarkerReportActionID, scrollingVerticalOffset, }: ShouldDisplayNewMarkerOnReportActionParams): boolean => { + const isNextMessageUnread = !!nextMessage && isReportActionUnread(nextMessage, unreadMarkerTime); + // If the current message is the earliest message received while offline, we want to display the unread marker above this message. - // const isEarliestReceivedOfflineMessage = index === earliestReceivedOfflineMessageIndex; if (isEarliestReceivedOfflineMessage && !isNextMessageUnread) { return true; } From 89df2e9f40608f9e7967b51d2507e1024d222a4c Mon Sep 17 00:00:00 2001 From: Mateusz Titz Date: Wed, 16 Apr 2025 16:09:32 +0200 Subject: [PATCH 8/9] Improve code comments around MoneyRequestReportActionsList --- .../MoneyRequestReportActionsList.tsx | 7 +++---- .../home/report/shouldDisplayNewMarkerOnReportAction.ts | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx b/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx index 75e4a1544a63..a194e9e4cbdd 100644 --- a/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx +++ b/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx @@ -278,9 +278,7 @@ function MoneyRequestReportActionsList({report, reportActions = [], transactions } setUnreadMarkerTime(mostRecentReportActionCreated); - - // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps - }, [lastAction?.created]); + }, [lastAction?.created, unreadMarkerReportActionID, unreadMarkerTime]); const scrollToBottomForCurrentUserAction = useCallback( (isFromCurrentUser: boolean) => { @@ -302,7 +300,7 @@ function MoneyRequestReportActionsList({report, reportActions = [], transactions ); useEffect(() => { - // This callback is triggered when a new action arrives via Pusher and the event is emitted from Report.js. This allows us to maintain + // This callback is triggered when a new action arrives via Pusher and the event is emitted from Report.ts. This allows us to maintain // a single source of truth for the "new action" event instead of trying to derive that a new action has appeared from looking at props. const unsubscribe = subscribeToNewActionEvent(report.reportID, scrollToBottomForCurrentUserAction); @@ -313,6 +311,7 @@ function MoneyRequestReportActionsList({report, reportActions = [], transactions unsubscribe(); }; + // This effect handles subscribing to events, so we only want to run it to run on mount, and in case reportID changes // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps }, [report.reportID]); diff --git a/src/pages/home/report/shouldDisplayNewMarkerOnReportAction.ts b/src/pages/home/report/shouldDisplayNewMarkerOnReportAction.ts index b850183c9bbc..ae272c1bca42 100644 --- a/src/pages/home/report/shouldDisplayNewMarkerOnReportAction.ts +++ b/src/pages/home/report/shouldDisplayNewMarkerOnReportAction.ts @@ -24,7 +24,7 @@ type ShouldDisplayNewMarkerOnReportActionParams = { /** Current value for vertical offset */ scrollingVerticalOffset: number; - /** Current value for vertical offset */ + /** The id of reportAction that was last marked as read */ prevUnreadMarkerReportActionID: string | null; }; From 088f51694f8c6fdff604268522ac7cdb403643af Mon Sep 17 00:00:00 2001 From: Mateusz Titz Date: Wed, 16 Apr 2025 17:01:31 +0200 Subject: [PATCH 9/9] Fix comment typo --- .../MoneyRequestReportView/MoneyRequestReportActionsList.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx b/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx index a194e9e4cbdd..c902cecb7545 100644 --- a/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx +++ b/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx @@ -311,7 +311,7 @@ function MoneyRequestReportActionsList({report, reportActions = [], transactions unsubscribe(); }; - // This effect handles subscribing to events, so we only want to run it to run on mount, and in case reportID changes + // This effect handles subscribing to events, so we only want to run it on mount, and in case reportID changes // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps }, [report.reportID]);