diff --git a/src/hooks/useReportActionsScroll.ts b/src/hooks/useReportActionsScroll.ts index 81dec82beb3e..d70062c99641 100644 --- a/src/hooks/useReportActionsScroll.ts +++ b/src/hooks/useReportActionsScroll.ts @@ -105,6 +105,9 @@ type UseReportActionsScrollResult = { /** Whether the action badge target is above the viewport */ isActionBadgeAboveViewport: boolean; + /** Whether the action badge target is below the viewport */ + isActionBadgeBelowViewport: boolean; + /** Scrolls to the unread marker when available, otherwise to the newest action, and marks the report as read when appropriate */ scrollToBottomAndMarkReportAsRead: () => void; @@ -216,6 +219,8 @@ function useReportActionsScroll({ setIsFloatingMessageCounterVisible, isActionBadgeAboveViewport, setIsActionBadgeAboveViewport, + isActionBadgeBelowViewport, + setIsActionBadgeBelowViewport, trackVerticalScrolling, onViewableItemsChanged, updatePillVisibility, @@ -417,6 +422,7 @@ function useReportActionsScroll({ // The expanded composer covers the list, so the target never becomes viewable to hide the pill if (isComposerFullSize) { setIsActionBadgeAboveViewport(false); + setIsActionBadgeBelowViewport(false); setIsFloatingMessageCounterVisible(false); } // `animated` is explicit because native defaults to an instant jump, which would teleport the list and lose the user's place. @@ -487,6 +493,7 @@ function useReportActionsScroll({ onViewableItemsChanged, isFloatingMessageCounterVisible, isActionBadgeAboveViewport, + isActionBadgeBelowViewport, scrollToBottomAndMarkReportAsRead, scrollToActionBadgeTarget, flushPendingScrollToBottom, diff --git a/src/pages/inbox/report/FloatingMessageCounter.tsx b/src/pages/inbox/report/FloatingMessageCounter.tsx index a787612977b3..f1c4f94c2e17 100644 --- a/src/pages/inbox/report/FloatingMessageCounter.tsx +++ b/src/pages/inbox/report/FloatingMessageCounter.tsx @@ -85,6 +85,9 @@ type FloatingMessageCounterProps = { /** Callback when the action badge pill is clicked */ onActionBadgePress?: () => void; + /** Whether the action badge target is below the viewport, so the pill points down instead of up */ + isActionBadgeTargetBelow?: boolean; + /** Whether to show "Mark as done" copy instead of "Submit" copy for track-intent users */ shouldShowMarkAsDoneCopy?: boolean; }; @@ -99,6 +102,7 @@ function FloatingMessageCounter({ actionBadge, actionBadgeBrickRoadStatus, onActionBadgePress, + isActionBadgeTargetBelow = false, shouldShowMarkAsDoneCopy, }: FloatingMessageCounterProps) { const icons = useMemoizedLazyExpensifyIcons(['DownArrow', 'UpArrow']); @@ -111,11 +115,13 @@ function FloatingMessageCounter({ const isVisible = isActive || shouldShowActionBadgePill; // Keep the last shown pill while it slides out so hiding the action badge pill does not flash the Latest messages pill - const visibleActionBadge = shouldShowActionBadgePill ? {actionBadge, actionBadgeBrickRoadStatus} : undefined; + const visibleActionBadge = shouldShowActionBadgePill ? {actionBadge, actionBadgeBrickRoadStatus, isActionBadgeTargetBelow} : undefined; const [lastVisibleActionBadge, setLastVisibleActionBadge] = useState(visibleActionBadge); if ( isVisible && - (lastVisibleActionBadge?.actionBadge !== visibleActionBadge?.actionBadge || lastVisibleActionBadge?.actionBadgeBrickRoadStatus !== visibleActionBadge?.actionBadgeBrickRoadStatus) + (lastVisibleActionBadge?.actionBadge !== visibleActionBadge?.actionBadge || + lastVisibleActionBadge?.actionBadgeBrickRoadStatus !== visibleActionBadge?.actionBadgeBrickRoadStatus || + lastVisibleActionBadge?.isActionBadgeTargetBelow !== visibleActionBadge?.isActionBadgeTargetBelow) ) { setLastVisibleActionBadge(visibleActionBadge); } @@ -161,7 +167,7 @@ function FloatingMessageCounter({ diff --git a/src/pages/inbox/report/ReportActionsList/useReportActionsListScroll.ts b/src/pages/inbox/report/ReportActionsList/useReportActionsListScroll.ts index e4de34fece9d..e491847c5dd9 100644 --- a/src/pages/inbox/report/ReportActionsList/useReportActionsListScroll.ts +++ b/src/pages/inbox/report/ReportActionsList/useReportActionsListScroll.ts @@ -112,6 +112,7 @@ function useReportActionsListScroll({ onViewableItemsChanged, isFloatingMessageCounterVisible, isActionBadgeAboveViewport, + isActionBadgeBelowViewport, scrollToBottomAndMarkReportAsRead, scrollToActionBadgeTarget, flushPendingScrollToBottom, @@ -187,6 +188,7 @@ function useReportActionsListScroll({ shouldBeAlignedToTop, isFloatingMessageCounterVisible, isActionBadgeAboveViewport, + isActionBadgeBelowViewport, scrollToBottomAndMarkReportAsRead, scrollToActionBadgeTarget, listProps: { diff --git a/src/pages/inbox/report/useReportUnreadMessageScrollTracking.ts b/src/pages/inbox/report/useReportUnreadMessageScrollTracking.ts index 7c9ed46aabe4..fe03d47df38a 100644 --- a/src/pages/inbox/report/useReportUnreadMessageScrollTracking.ts +++ b/src/pages/inbox/report/useReportUnreadMessageScrollTracking.ts @@ -55,6 +55,7 @@ export default function useReportUnreadMessageScrollTracking({ }: Args) { const [isFloatingMessageCounterVisible, setIsFloatingMessageCounterVisible] = useState(false); const [isActionBadgeAboveViewport, setIsActionBadgeAboveViewport] = useState(false); + const [isActionBadgeBelowViewport, setIsActionBadgeBelowViewport] = useState(false); const isFocused = useIsFocused(); const ref = useRef<{ previousViewableItems: ViewToken[]; @@ -168,17 +169,21 @@ export default function useReportUnreadMessageScrollTracking({ ref.current.onUnreadActionVisible(); } - // Track whether the action badge target is above the viewport (i.e., not visible and at a higher index in the inverted list) + // Track whether the action badge target is out of view above or below the viewport const badgeTargetIndex = ref.current.actionBadgeTargetIndex; if (badgeTargetIndex !== -1) { - // In an inverted list, higher indexes are "above" (older messages). The target is above the viewport - // when its index is greater than the max visible index. + // In an inverted list, higher indexes are "above" (older messages) and lower indexes are "below" (newer messages) // FlashList reports a snapshot taken up to 250ms earlier, which can predate the new row's measured layout, so prefer the live range const visibleRange = ref.current.getVisibleIndexRange?.(); - const isAbove = isInverted ? badgeTargetIndex > (visibleRange?.endIndex ?? maxIndex) : badgeTargetIndex < (visibleRange?.startIndex ?? minIndex); + const startIndex = visibleRange?.startIndex ?? minIndex; + const endIndex = visibleRange?.endIndex ?? maxIndex; + const isAbove = isInverted ? badgeTargetIndex > endIndex : badgeTargetIndex < startIndex; + const isBelow = isInverted ? badgeTargetIndex < startIndex : badgeTargetIndex > endIndex; setIsActionBadgeAboveViewport(isAbove); + setIsActionBadgeBelowViewport(isBelow); } else { setIsActionBadgeAboveViewport(false); + setIsActionBadgeBelowViewport(false); } }; @@ -203,6 +208,8 @@ export default function useReportUnreadMessageScrollTracking({ setIsFloatingMessageCounterVisible, isActionBadgeAboveViewport, setIsActionBadgeAboveViewport, + isActionBadgeBelowViewport, + setIsActionBadgeBelowViewport, trackVerticalScrolling, onViewableItemsChanged, updatePillVisibility, diff --git a/tests/unit/FloatingMessageCounterTest.tsx b/tests/unit/FloatingMessageCounterTest.tsx index 88c185e9e5fa..53e45e1c0045 100644 --- a/tests/unit/FloatingMessageCounterTest.tsx +++ b/tests/unit/FloatingMessageCounterTest.tsx @@ -27,14 +27,26 @@ jest.mock('@hooks/useLocalize', () => })), ); +// The arrow icons render inside an accessibility-hidden wrapper, so record which one rendered to assert the pill direction +const mockRenderedArrows: string[] = []; jest.mock('@hooks/useLazyAsset', () => ({ useMemoizedLazyExpensifyIcons: () => ({ - DownArrow: () => null, - UpArrow: () => null, + DownArrow: () => { + mockRenderedArrows.push('DownArrow'); + return null; + }, + UpArrow: () => { + mockRenderedArrows.push('UpArrow'); + return null; + }, }), })); describe('FloatingMessageCounter', () => { + beforeEach(() => { + mockRenderedArrows.length = 0; + }); + it('renders the new messages pill when hasNewMessages is true and no action badge', () => { render( { expect(screen.getByText('Submit')).toBeTruthy(); }); + it('points the action badge pill up when the target is above the viewport', () => { + // Given an action badge whose target is above the viewport, so the below flag keeps its default + // When the pill renders + render( + , + ); + + // Then the pill points up, because the user has to scroll up to reach the target + expect(screen.getByText('Approve')).toBeTruthy(); + expect(mockRenderedArrows).toContain('UpArrow'); + expect(mockRenderedArrows).not.toContain('DownArrow'); + }); + + it('points the action badge pill down and scrolls to the target when the target is below the viewport', () => { + // Given an action badge whose target is below the viewport while the user is scrolled up past the threshold + const onActionBadgePressMock = jest.fn(); + render( + , + ); + + // When the pill is pressed + fireEvent.press(screen.getByText('Approve')); + + // Then the pill points down toward the target and scrolls to it instead of to the latest messages + expect(mockRenderedArrows).toContain('DownArrow'); + expect(mockRenderedArrows).not.toContain('UpArrow'); + expect(screen.queryByText('Latest messages')).toBeNull(); + expect(onActionBadgePressMock).toHaveBeenCalledTimes(1); + }); + + it('keeps the down arrow while the action badge pill slides out', () => { + // Given the action badge pill points down at a target below the viewport + const {rerender} = render( + , + ); + mockRenderedArrows.length = 0; + + // When the target scrolls into view, so the list stops passing the badge and the below flag clears + rerender( + , + ); + + // Then the exiting pill still points down, because flipping the arrow during the exit animation would flash an up arrow + expect(screen.getByText('Approve')).toBeTruthy(); + expect(mockRenderedArrows).toContain('DownArrow'); + expect(mockRenderedArrows).not.toContain('UpArrow'); + }); + it('does not show action badge pill when only actionBadge is provided without actionBadgeBrickRoadStatus', () => { render( ({ // --- useReportUnreadMessageScrollTracking --- const mockSetIsFloatingMessageCounterVisible = jest.fn(); const mockSetIsActionBadgeAboveViewport = jest.fn(); +const mockSetIsActionBadgeBelowViewport = jest.fn(); const mockTrackVerticalScrolling = jest.fn(); const mockOnViewableItemsChanged = jest.fn(); let mockIsFloatingMessageCounterVisible = false; let mockIsActionBadgeAboveViewport = false; +let mockIsActionBadgeBelowViewport = false; let mockShouldDisablePillTracking: boolean | undefined; jest.mock('@pages/inbox/report/useReportUnreadMessageScrollTracking', () => ({ __esModule: true, @@ -61,6 +63,8 @@ jest.mock('@pages/inbox/report/useReportUnreadMessageScrollTracking', () => ({ setIsFloatingMessageCounterVisible: mockSetIsFloatingMessageCounterVisible, isActionBadgeAboveViewport: mockIsActionBadgeAboveViewport, setIsActionBadgeAboveViewport: mockSetIsActionBadgeAboveViewport, + isActionBadgeBelowViewport: mockIsActionBadgeBelowViewport, + setIsActionBadgeBelowViewport: mockSetIsActionBadgeBelowViewport, trackVerticalScrolling: mockTrackVerticalScrolling, onViewableItemsChanged: mockOnViewableItemsChanged, }; @@ -240,6 +244,7 @@ describe('useReportActionsScroll', () => { mockReportRHPActiveRoute = undefined; mockIsFloatingMessageCounterVisible = false; mockIsActionBadgeAboveViewport = false; + mockIsActionBadgeBelowViewport = false; mockShouldDisablePillTracking = undefined; mockIsScrollToBottomEnabled = false; mockIsTransactionThread = false; @@ -431,6 +436,7 @@ describe('useReportActionsScroll', () => { // Then the pills are not hidden directly because the target scrolls into view and viewability tracking hides the pill expect(mockSetIsActionBadgeAboveViewport).not.toHaveBeenCalled(); + expect(mockSetIsActionBadgeBelowViewport).not.toHaveBeenCalled(); expect(mockSetIsFloatingMessageCounterVisible).not.toHaveBeenCalled(); }); @@ -446,6 +452,7 @@ describe('useReportActionsScroll', () => { // Then both pills are hidden directly because the target can never become viewable to hide them, and the list still scrolls expect(mockSetIsActionBadgeAboveViewport).toHaveBeenCalledWith(false); + expect(mockSetIsActionBadgeBelowViewport).toHaveBeenCalledWith(false); expect(mockSetIsFloatingMessageCounterVisible).toHaveBeenCalledWith(false); expect(mockScrollToIndex).toHaveBeenCalledWith(5, {animated: true, viewPosition: 1, viewOffset: CONST.REPORT.ACTIONS.LINKED_MESSAGE_OFFSET}); }); @@ -730,5 +737,17 @@ describe('useReportActionsScroll', () => { result.current.onViewableItemsChanged({viewableItems: [], changed: []}); expect(mockOnViewableItemsChanged).toHaveBeenCalled(); }); + + it('forwards isActionBadgeBelowViewport from the tracking hook', async () => { + // Given the tracking hook reports the action badge target below the viewport + mockIsActionBadgeBelowViewport = true; + + // When the scroll hook renders + const {result} = await renderScroll(); + + // Then the below flag reaches the list so the pill can point down + expect(result.current.isActionBadgeBelowViewport).toBe(true); + expect(result.current.isActionBadgeAboveViewport).toBe(false); + }); }); }); diff --git a/tests/unit/useReportUnreadMessageScrollTrackingTest.ts b/tests/unit/useReportUnreadMessageScrollTrackingTest.ts index 914a29973d3d..1f13ecc0b2c8 100644 --- a/tests/unit/useReportUnreadMessageScrollTrackingTest.ts +++ b/tests/unit/useReportUnreadMessageScrollTrackingTest.ts @@ -477,7 +477,7 @@ describe('useReportUnreadMessageScrollTracking', () => { }); }); - describe('action badge above viewport tracking', () => { + describe('action badge viewport tracking', () => { const onTrackScrollingMockFn = jest.fn(); it('returns isActionBadgeAboveViewport as false initially', () => { @@ -675,6 +675,172 @@ describe('useReportUnreadMessageScrollTracking', () => { expect(result.current.isActionBadgeAboveViewport).toBe(true); }); + it('returns isActionBadgeBelowViewport as true when action badge target is below the viewport in inverted list', () => { + // Given an inverted list whose action badge target is at index 1 + const offsetRef = {current: 0}; + const {result} = renderHook(() => + useReportUnreadMessageScrollTracking({ + reportID, + currentVerticalScrollingOffsetRef: offsetRef, + onUnreadActionVisible: onUnreadActionVisibleMockFn, + onTrackScrolling: onTrackScrollingMockFn, + hasNewerActions: false, + unreadMarkerReportActionIndex: -1, + isInverted: true, + actionBadgeTargetIndex: 1, + }), + ); + + // When the user has scrolled up so only indexes 3 to 5 are visible + act(() => { + result.current.onViewableItemsChanged({ + viewableItems: [ + {index: 3, key: 'reportActions_3', isViewable: true, item: {}}, + {index: 4, key: 'reportActions_4', isViewable: true, item: {}}, + {index: 5, key: 'reportActions_5', isViewable: true, item: {}}, + ], + changed: [], + }); + }); + + // Then the target is below the viewport, because lower indexes are newer messages at the bottom of an inverted list + expect(result.current.isActionBadgeBelowViewport).toBe(true); + expect(result.current.isActionBadgeAboveViewport).toBe(false); + }); + + it('returns both direction flags as false when action badge target is visible in viewport', () => { + // Given an inverted list whose action badge target is at index 2 + const offsetRef = {current: 0}; + const {result} = renderHook(() => + useReportUnreadMessageScrollTracking({ + reportID, + currentVerticalScrollingOffsetRef: offsetRef, + onUnreadActionVisible: onUnreadActionVisibleMockFn, + onTrackScrolling: onTrackScrollingMockFn, + hasNewerActions: false, + unreadMarkerReportActionIndex: -1, + isInverted: true, + actionBadgeTargetIndex: 2, + }), + ); + + // When the visible indexes include the target + act(() => { + result.current.onViewableItemsChanged({ + viewableItems: [ + {index: 1, key: 'reportActions_1', isViewable: true, item: {}}, + {index: 2, key: 'reportActions_2', isViewable: true, item: {}}, + {index: 3, key: 'reportActions_3', isViewable: true, item: {}}, + ], + changed: [], + }); + }); + + // Then neither flag is set, because the pill only shows while the target is out of view + expect(result.current.isActionBadgeBelowViewport).toBe(false); + expect(result.current.isActionBadgeAboveViewport).toBe(false); + }); + + it('returns isActionBadgeBelowViewport as true for a higher index when the list is not inverted', () => { + // Given a non-inverted list whose action badge target is at index 9 + const offsetRef = {current: 0}; + const {result} = renderHook(() => + useReportUnreadMessageScrollTracking({ + reportID, + currentVerticalScrollingOffsetRef: offsetRef, + onUnreadActionVisible: onUnreadActionVisibleMockFn, + onTrackScrolling: onTrackScrollingMockFn, + hasNewerActions: false, + unreadMarkerReportActionIndex: -1, + isInverted: false, + actionBadgeTargetIndex: 9, + }), + ); + + // When only indexes 3 and 4 are visible + act(() => { + result.current.onViewableItemsChanged({ + viewableItems: [ + {index: 3, key: 'reportActions_3', isViewable: true, item: {}}, + {index: 4, key: 'reportActions_4', isViewable: true, item: {}}, + ], + changed: [], + }); + }); + + // Then the target is below the viewport, because higher indexes are further down in a non-inverted list + expect(result.current.isActionBadgeBelowViewport).toBe(true); + expect(result.current.isActionBadgeAboveViewport).toBe(false); + }); + + it('returns isActionBadgeAboveViewport as true for a lower index when the list is not inverted', () => { + // Given a non-inverted list whose action badge target is at index 1 + const offsetRef = {current: 0}; + const {result} = renderHook(() => + useReportUnreadMessageScrollTracking({ + reportID, + currentVerticalScrollingOffsetRef: offsetRef, + onUnreadActionVisible: onUnreadActionVisibleMockFn, + onTrackScrolling: onTrackScrollingMockFn, + hasNewerActions: false, + unreadMarkerReportActionIndex: -1, + isInverted: false, + actionBadgeTargetIndex: 1, + }), + ); + + // When only indexes 3 and 4 are visible + act(() => { + result.current.onViewableItemsChanged({ + viewableItems: [ + {index: 3, key: 'reportActions_3', isViewable: true, item: {}}, + {index: 4, key: 'reportActions_4', isViewable: true, item: {}}, + ], + changed: [], + }); + }); + + // Then the target is above the viewport, because lower indexes are further up in a non-inverted list + expect(result.current.isActionBadgeBelowViewport).toBe(false); + expect(result.current.isActionBadgeAboveViewport).toBe(true); + }); + + it('switches the badge direction from above to below when the user scrolls past the target', () => { + // Given an inverted list whose action badge target at index 5 is above the viewport + const offsetRef = {current: 0}; + const {result} = renderHook(() => + useReportUnreadMessageScrollTracking({ + reportID, + currentVerticalScrollingOffsetRef: offsetRef, + onUnreadActionVisible: onUnreadActionVisibleMockFn, + onTrackScrolling: onTrackScrollingMockFn, + hasNewerActions: false, + unreadMarkerReportActionIndex: -1, + isInverted: true, + actionBadgeTargetIndex: 5, + }), + ); + act(() => { + result.current.onViewableItemsChanged({ + viewableItems: [{index: 0, key: 'reportActions_0', isViewable: true, item: {}}], + changed: [], + }); + }); + expect(result.current.isActionBadgeAboveViewport).toBe(true); + + // When the user scrolls further up so only older messages are visible + act(() => { + result.current.onViewableItemsChanged({ + viewableItems: [{index: 8, key: 'reportActions_8', isViewable: true, item: {}}], + changed: [], + }); + }); + + // Then the pill flips to point down, because the target is now between the viewport and the newest message + expect(result.current.isActionBadgeBelowViewport).toBe(true); + expect(result.current.isActionBadgeAboveViewport).toBe(false); + }); + it.each([true, false])('keeps the action badge hidden despite stale viewable items when inverted is %s', (isInverted) => { // Given a live range containing the target in either list order. const offsetRef = {current: 0};