From c902c20cc230abf4258781137ffa080f4b3464c6 Mon Sep 17 00:00:00 2001 From: Jules Date: Mon, 24 Jun 2024 12:41:25 +0100 Subject: [PATCH] Revert "fix: re-calc the marker when msgs are deleted" --- src/pages/home/ReportScreen.tsx | 1 - src/pages/home/report/ReportActionsList.tsx | 32 ++++---------- tests/ui/UnreadIndicatorsTest.tsx | 46 --------------------- 3 files changed, 7 insertions(+), 72 deletions(-) diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index 0479c575469a..6f02e4b1c8fd 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -763,7 +763,6 @@ function ReportScreen({ {!shouldShowSkeleton && ( { - // if lastReadTime is null, use the lastReadTimeRef.current - const baseLastReadTime = lastReadTime ?? lastReadTimeRef.current; + (reportAction: OnyxTypes.ReportAction, index: number): boolean => { let shouldDisplay = false; - if (!currentUnreadMarker || !shouldCheckWithCurrentUnreadMarker) { + if (!currentUnreadMarker) { const nextMessage = sortedVisibleReportActions[index + 1]; - const isCurrentMessageUnread = isMessageUnread(reportAction, baseLastReadTime); - shouldDisplay = isCurrentMessageUnread && (!nextMessage || !isMessageUnread(nextMessage, baseLastReadTime)) && !ReportActionsUtils.shouldHideNewMarker(reportAction); + const isCurrentMessageUnread = isMessageUnread(reportAction, lastReadTimeRef.current); + shouldDisplay = isCurrentMessageUnread && (!nextMessage || !isMessageUnread(nextMessage, lastReadTimeRef.current)) && !ReportActionsUtils.shouldHideNewMarker(reportAction); if (shouldDisplay && !messageManuallyMarkedUnread) { const isWithinVisibleThreshold = scrollingVerticalOffset.current < MSG_VISIBLE_THRESHOLD ? reportAction.created < (userActiveSince.current ?? '') : true; // Prevent displaying a new marker line when report action is of type "REPORT_PREVIEW" and last actor is the current user @@ -489,37 +487,21 @@ function ReportActionsList({ return ReportUtils.isExpenseReport(report) || ReportUtils.isIOUReport(report) || ReportUtils.isInvoiceReport(report); }, [parentReportAction, report, sortedVisibleReportActions]); - // storing the last read time used to render the unread marker - const markerLastReadTimeRef = useRef(); - - useEffect(() => { - if (currentUnreadMarker) { - return; - } - markerLastReadTimeRef.current = undefined; - }, [currentUnreadMarker]); - const calculateUnreadMarker = useCallback(() => { // Iterate through the report actions and set appropriate unread marker. // This is to avoid a warning of: // Cannot update a component (ReportActionsList) while rendering a different component (CellRenderer). let markerFound = false; sortedVisibleReportActions.forEach((reportAction, index) => { - if (!shouldDisplayNewMarker(reportAction, index, markerLastReadTimeRef.current, false)) { + if (!shouldDisplayNewMarker(reportAction, index)) { return; } markerFound = true; - if (!currentUnreadMarker || currentUnreadMarker !== reportAction.reportActionID) { + if (!currentUnreadMarker && currentUnreadMarker !== reportAction.reportActionID) { cacheUnreadMarkers.set(report.reportID, reportAction.reportActionID); setCurrentUnreadMarker(reportAction.reportActionID); } }); - - // if marker can be found, set the markerLastReadTimeRef to the last read time if necessary - if (markerFound && !markerLastReadTimeRef.current) { - markerLastReadTimeRef.current = lastReadTimeRef.current; - } - if (!markerFound && !linkedReportActionID) { setCurrentUnreadMarker(null); } @@ -585,7 +567,7 @@ function ReportActionsList({ displayAsGroup={ReportActionsUtils.isConsecutiveActionMadeByPreviousActor(sortedVisibleReportActions, index)} mostRecentIOUReportActionID={mostRecentIOUReportActionID} shouldHideThreadDividerLine={shouldHideThreadDividerLine} - shouldDisplayNewMarker={shouldDisplayNewMarker(reportAction, index, markerLastReadTimeRef.current, true)} + shouldDisplayNewMarker={shouldDisplayNewMarker(reportAction, index)} shouldDisplayReplyDivider={sortedVisibleReportActions.length > 1} isFirstVisibleReportAction={firstVisibleReportActionID === reportAction.reportActionID} shouldUseThreadDividerLine={shouldUseThreadDividerLine} diff --git a/tests/ui/UnreadIndicatorsTest.tsx b/tests/ui/UnreadIndicatorsTest.tsx index 19f552d22a7a..130077d0f017 100644 --- a/tests/ui/UnreadIndicatorsTest.tsx +++ b/tests/ui/UnreadIndicatorsTest.tsx @@ -464,52 +464,6 @@ describe('Unread Indicators', () => { expect(displayNameTexts[1]?.props?.style?.fontWeight).toBe(FontUtils.fontWeight.bold); expect(screen.getByText('B User')).toBeOnTheScreen(); })); - it('Delete a chat message and verify the unread indicator is moved', async () => { - const getUnreadIndicator = () => { - const newMessageLineIndicatorHintText = Localize.translateLocal('accessibilityHints.newMessageLineIndicator'); - return screen.queryAllByLabelText(newMessageLineIndicatorHintText); - }; - - return signInAndGetAppWithUnreadChat() - .then(() => navigateToSidebarOption(0)) - .then(async () => act(() => transitionEndCB?.())) - .then(async () => { - const reportActionsViewWrapper = await screen.findByTestId('report-actions-view-wrapper'); - if (reportActionsViewWrapper) { - fireEvent(reportActionsViewWrapper, 'onLayout', {nativeEvent: {layout: {x: 0, y: 0, width: 100, height: 100}}}); - } - return waitForBatchedUpdates(); - }) - .then(() => { - // Verify the new line indicator is present, and it's before the action with ID 4 - const unreadIndicator = getUnreadIndicator(); - expect(unreadIndicator).toHaveLength(1); - const reportActionID = unreadIndicator[0]?.props?.['data-action-id']; - expect(reportActionID).toBe('4'); - - // simulate delete comment event from Pusher - PusherHelper.emitOnyxUpdate([ - { - onyxMethod: Onyx.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${REPORT_ID}`, - value: { - '4': { - message: [], - }, - }, - }, - ]); - return waitForBatchedUpdates(); - }) - .then(() => - // Verify the new line indicator is now before the action with ID 5 - waitFor(() => { - const unreadIndicator = getUnreadIndicator(); - const reportActionID = unreadIndicator[0]?.props?.['data-action-id']; - expect(reportActionID).toBe('5'); - }), - ); - }); xit('Manually marking a chat message as unread shows the new line indicator and updates the LHN', () => signInAndGetAppWithUnreadChat()