diff --git a/src/components/MoneyRequestReportView/MoneyRequestReportView.tsx b/src/components/MoneyRequestReportView/MoneyRequestReportView.tsx index 4cf5482fce99..ebef8dc41714 100644 --- a/src/components/MoneyRequestReportView/MoneyRequestReportView.tsx +++ b/src/components/MoneyRequestReportView/MoneyRequestReportView.tsx @@ -3,7 +3,7 @@ import MoneyReportHeader from '@components/MoneyReportHeader'; import MoneyRequestHeader from '@components/MoneyRequestHeader'; import OfflineWithFeedback from '@components/OfflineWithFeedback'; import MoneyRequestReceiptView from '@components/ReportActionItem/MoneyRequestReceiptView'; -import ReportActionsSkeletonView from '@components/ReportActionsSkeletonView'; +import ReportActionsSkeletonCover, {ReportActionsAnimatedSkeletonCover} from '@components/ReportActionsSkeletonCover'; import ReportHeaderSkeletonView from '@components/ReportHeaderSkeletonView'; import useContentHeaderHeight from '@hooks/useContentHeaderHeight'; @@ -45,7 +45,7 @@ import type {LayoutChangeEvent} from 'react-native'; import type {OnyxEntry} from 'react-native-onyx'; import {PortalHost} from '@gorhom/portal'; -import React, {useCallback, useEffect, useMemo} from 'react'; +import {useEffect} from 'react'; // We use Animated for all functionality related to wide RHP to make it easier // to interact with react-navigation components (e.g., CardContainer, interpolator), which also use Animated. // eslint-disable-next-line no-restricted-imports @@ -108,7 +108,7 @@ function InitialLoadingSkeleton({styles, onLayout}: {styles: ThemeStyles; onLayo {}} /> - + ); } @@ -129,34 +129,23 @@ function MoneyRequestReportView({report, reportIDFromRoute, reportLoadingState, const {reportActions: unfilteredReportActions} = usePaginatedReportActions(reportID); - const reportActions = useMemo(() => { - return getFilteredReportActionsForReportView(unfilteredReportActions); - }, [unfilteredReportActions]); + const reportActions = getFilteredReportActionsForReportView(unfilteredReportActions); const reportTransactions = useReportTransactionsCollection(reportID); - const transactions = useMemo(() => getAllNonDeletedTransactions(reportTransactions, reportActions, isOffline, true), [reportTransactions, reportActions, isOffline]); + const transactions = getAllNonDeletedTransactions(reportTransactions, reportActions, isOffline, true); - const visibleTransactions = useMemo(() => { - if (isOffline) { - return transactions; - } - - // When there are no pending delete transactions, which is most of the time, we can return the same transactions keeping the same reference avoiding extra work - const hasPendingDelete = transactions.some((transaction) => transaction.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE); - if (!hasPendingDelete) { - return transactions; - } - - return transactions.filter((transaction) => transaction.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE); - }, [transactions, isOffline]); + // When there are no pending delete transactions, which is most of the time, return the same transactions to keep the same reference and avoid extra work. + const hasPendingDelete = transactions.some((transaction) => transaction.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE); + const visibleTransactions = + isOffline || !hasPendingDelete ? transactions : transactions.filter((transaction) => transaction.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE); const reportErrors = visibleTransactions.length === 1 && visibleTransactions.at(0)?.errors ? undefined : allReportErrors; const reportTransactionIDs = visibleTransactions.map((transaction) => transaction.transactionID); const transactionThreadReportID = getOneTransactionThreadReportID(report, chatReport, reportActions ?? [], isOffline, reportTransactionIDs); const isReportLoadPending = useIsReportLoadPending(reportID); - const dismissReportCreationError = useCallback(() => { + const dismissReportCreationError = () => { goBackFromSearchMoneyRequest({afterTransition: () => removeFailedReport(reportID)}); - }, [reportID]); + }; // Special case handling a report that is a transaction thread // If true we will use the standard `ReportActionsList` to display report data and a special header, anything else is handled via `MoneyRequestReportActionsList` @@ -174,33 +163,29 @@ function MoneyRequestReportView({report, reportIDFromRoute, reportLoadingState, const [transactionThreadReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${transactionThreadReportID}`); const shouldShowWideRHPReceipt = visibleTransactions.length === 1 && !isSmallScreenWidth && !!transactionThreadReport; - const reportHeaderView = useMemo( - () => - isTransactionThreadView ? ( - { - if (!backToRoute) { - goBackFromSearchMoneyRequest(); - return; - } - Navigation.goBack(backToRoute); - }} - /> - ) : ( - { - if (!backToRoute) { - goBackFromSearchMoneyRequest(); - return; - } - Navigation.goBack(backToRoute); - }} - /> - ), - [backToRoute, isTransactionThreadView, report?.reportID], + const reportHeaderView = isTransactionThreadView ? ( + { + if (!backToRoute) { + goBackFromSearchMoneyRequest(); + return; + } + Navigation.goBack(backToRoute); + }} + /> + ) : ( + { + if (!backToRoute) { + goBackFromSearchMoneyRequest(); + return; + } + Navigation.goBack(backToRoute); + }} + /> ); // We need to cancel telemetry span when user leaves the screen before full report data is loaded @@ -220,7 +205,7 @@ function MoneyRequestReportView({report, reportIDFromRoute, reportLoadingState, } if (shouldShowEmptyActionsSkeleton) { - return ; + return ; } if (!report) { @@ -230,8 +215,7 @@ function MoneyRequestReportView({report, reportIDFromRoute, reportLoadingState, if (shouldShowAppLoadSkeleton) { return ( - - + {shouldDisplayReportFooter ? : null} ); diff --git a/src/components/ReportActionsSkeletonCover.tsx b/src/components/ReportActionsSkeletonCover.tsx new file mode 100644 index 000000000000..c20632a781ef --- /dev/null +++ b/src/components/ReportActionsSkeletonCover.tsx @@ -0,0 +1,48 @@ +import useThemeStyles from '@hooks/useThemeStyles'; + +import type {ReactNode} from 'react'; + +import React from 'react'; +import {View} from 'react-native'; + +import ReportActionsSkeletonView from './ReportActionsSkeletonView'; + +type ReportActionsSkeletonContainerProps = { + /** The skeleton content to place at the bottom of the report viewport */ + children: ReactNode; +}; + +/** Fills the report-actions viewport with a consistently positioned static loading skeleton. */ +function ReportActionsSkeletonCover() { + return ( + + + + ); +} + +/** Fills the report-actions viewport with a consistently positioned animated loading skeleton. */ +function ReportActionsAnimatedSkeletonCover() { + return ( + + + + ); +} + +function ReportActionsSkeletonContainer({children}: ReportActionsSkeletonContainerProps) { + const styles = useThemeStyles(); + + return ( + + {children} + + ); +} + +export {ReportActionsAnimatedSkeletonCover}; +export default ReportActionsSkeletonCover; diff --git a/src/pages/inbox/report/ReportActionsList.tsx b/src/pages/inbox/report/ReportActionsList.tsx index b247b9c00e0b..94ab925a4b47 100644 --- a/src/pages/inbox/report/ReportActionsList.tsx +++ b/src/pages/inbox/report/ReportActionsList.tsx @@ -1,6 +1,7 @@ import {renderScrollComponent as renderActionSheetAwareScrollView} from '@components/ActionSheetAwareScrollView'; import InvertedFlashList from '@components/FlashList/InvertedFlashList'; import MerchantRuleSuggestionBanner from '@components/MerchantRuleSuggestionBanner'; +import {ReportActionsAnimatedSkeletonCover} from '@components/ReportActionsSkeletonCover'; import ReportActionsSkeletonView from '@components/ReportActionsSkeletonView'; import useConciergeSessionStartTime from '@hooks/useConciergeSessionStartTime'; @@ -449,7 +450,7 @@ function ReportActionsListContent({reportID, conciergeChat, onLayout}: ReportAct // It narrows `report` to non-undefined for the render below and stays a safe fallback if the report // is cleared mid-session while the latch keeps the content mounted. if (!report) { - return ; + return ; } return ( diff --git a/src/pages/inbox/report/ReportActionsLoadingSkeleton.tsx b/src/pages/inbox/report/ReportActionsLoadingSkeleton.tsx index 2fc926b894d0..cda1d6afc91b 100644 --- a/src/pages/inbox/report/ReportActionsLoadingSkeleton.tsx +++ b/src/pages/inbox/report/ReportActionsLoadingSkeleton.tsx @@ -1,4 +1,4 @@ -import ReportActionsSkeletonView from '@components/ReportActionsSkeletonView'; +import ReportActionsSkeletonCover, {ReportActionsAnimatedSkeletonCover} from '@components/ReportActionsSkeletonCover'; import useCancelSendMessageSpanOnSkeleton from '@hooks/useCancelSendMessageSpanOnSkeleton'; import type {SkeletonName} from '@hooks/useCancelSendMessageSpanOnSkeleton'; @@ -28,7 +28,9 @@ type ReportActionsLoadingSkeletonProps = { function ReportActionsLoadingSkeleton({reportID, skeletonName, shouldAnimate = true, shouldMarkOpenReportEnd = true}: ReportActionsLoadingSkeletonProps) { useCancelSendMessageSpanOnSkeleton(reportID, skeletonName); useMarkOpenReportEndOnSkeleton(reportID, shouldMarkOpenReportEnd); - return ; + const SkeletonCover = shouldAnimate ? ReportActionsAnimatedSkeletonCover : ReportActionsSkeletonCover; + + return ; } ReportActionsLoadingSkeleton.displayName = 'ReportActionsLoadingSkeleton'; diff --git a/tests/ui/MoneyRequestReportViewTest.tsx b/tests/ui/MoneyRequestReportViewTest.tsx index f0dc787734b5..4b8a39aa05f4 100644 --- a/tests/ui/MoneyRequestReportViewTest.tsx +++ b/tests/ui/MoneyRequestReportViewTest.tsx @@ -1,9 +1,12 @@ /* eslint-disable @typescript-eslint/no-unsafe-type-assertion */ -import {render} from '@testing-library/react-native'; +import {render, screen} from '@testing-library/react-native'; import MoneyRequestReportActionsList from '@components/MoneyRequestReportView/MoneyRequestReportActionsList'; import MoneyRequestReportView from '@components/MoneyRequestReportView/MoneyRequestReportView'; +import ReportActionsSkeletonView from '@components/ReportActionsSkeletonView'; +import ReportHeaderSkeletonView from '@components/ReportHeaderSkeletonView'; +import {useIsAppLoadPending, useIsReportLoadPending} from '@hooks/useInFlightRequests'; import useNetwork from '@hooks/useNetwork'; import useOnyx from '@hooks/useOnyx'; import usePaginatedReportActions from '@hooks/usePaginatedReportActions'; @@ -34,11 +37,15 @@ jest.mock('@hooks/useOnyx', () => jest.fn()); jest.mock('@hooks/useResponsiveLayout', () => jest.fn()); jest.mock('@hooks/usePaginatedReportActions', () => jest.fn()); jest.mock('@hooks/useReportTransactionsCollection', () => jest.fn()); +jest.mock('@hooks/useInFlightRequests', () => ({ + useIsAppLoadPending: jest.fn(), + useIsReportLoadPending: jest.fn(), +})); -// useThemeStyles throws without a ; return a proxy that yields an empty style object -// for any key so the (mostly-mocked) tree renders without wiring up the full provider stack. +// useThemeStyles throws without a ; return empty styles for most keys and +// a fixed header height for the app-loading layout assertion. jest.mock('@hooks/useThemeStyles', () => { - const styleProxy = new Proxy({}, {get: () => ({})}); + const styleProxy = new Proxy({}, {get: (_target, key) => (key === 'headerBarHeight' ? {height: 80} : {})}); return jest.fn(() => styleProxy); }); @@ -54,6 +61,8 @@ jest.mock('@components/MoneyReportHeader', () => jest.fn(() => null)); jest.mock('@components/MoneyRequestHeader', () => jest.fn(() => null)); jest.mock('@components/CollapsibleHeaderOnKeyboard', () => jest.fn(() => null)); jest.mock('@components/ReportActionItem/MoneyRequestReceiptView', () => jest.fn(() => null)); +jest.mock('@components/ReportActionsSkeletonView', () => jest.fn(() => null)); +jest.mock('@components/ReportHeaderSkeletonView', () => jest.fn(() => null)); jest.mock('@pages/inbox/report/ReportFooter', () => jest.fn(() => null)); jest.mock('@components/OfflineWithFeedback', () => { const reactModule = jest.requireActual('react'); @@ -61,12 +70,15 @@ jest.mock('@components/OfflineWithFeedback', () => { }); const mockUseNetwork = useNetwork as jest.MockedFunction; +const mockUseIsAppLoadPending = jest.mocked(useIsAppLoadPending); +const mockUseIsReportLoadPending = jest.mocked(useIsReportLoadPending); const mockUseOnyx = useOnyx as jest.MockedFunction; const mockUseResponsiveLayout = useResponsiveLayout as jest.MockedFunction; const mockUsePaginatedReportActions = usePaginatedReportActions as jest.MockedFunction; const mockUseReportTransactionsCollection = useReportTransactionsCollection as jest.MockedFunction; const mockMoneyRequestReportActionsList = MoneyRequestReportActionsList as jest.MockedFunction; const mockReportActionsListBody = ReportActionsList as jest.MockedFunction; +const mockReportActionsSkeletonView = jest.mocked(ReportActionsSkeletonView); const mockUserTypingEventListener = UserTypingEventListener as jest.MockedFunction; const defaultPaginatedReportActionsResult: ReturnType = { @@ -134,6 +146,8 @@ describe('MoneyRequestReportView', () => { jest.clearAllMocks(); mockUseNetwork.mockReturnValue({isOffline: false}); + mockUseIsAppLoadPending.mockReturnValue(false); + mockUseIsReportLoadPending.mockReturnValue(false); mockUsePaginatedReportActions.mockReturnValue(defaultPaginatedReportActionsResult); mockUseReportTransactionsCollection.mockReturnValue({}); mockUseResponsiveLayout.mockReturnValue({ @@ -176,6 +190,40 @@ describe('MoneyRequestReportView', () => { expect(MoneyRequestReportUtils.shouldWaitForTransactions).toHaveBeenLastCalledWith(mockReport, [], mockReportLoadingState, false, false); }); + it('uses the bottom-padded cover while the report is waiting for transactions', () => { + jest.spyOn(MoneyRequestReportUtils, 'shouldWaitForTransactions').mockReturnValue(true); + + renderMoneyRequestReportView(jest.fn()); + + expect(screen.getByTestId('ReportActionsSkeletonCover')).toBeTruthy(); + expect(mockReportActionsSkeletonView.mock.calls.at(-1)?.at(0)).toEqual(expect.objectContaining({shouldAnimate: true})); + expect(mockReportActionsListBody).not.toHaveBeenCalled(); + expect(mockMoneyRequestReportActionsList).not.toHaveBeenCalled(); + }); + + it('uses a static bottom-padded cover while report actions are empty', () => { + jest.spyOn(ReportActionsUtils, 'getFilteredReportActionsForReportView').mockReturnValue([]); + + renderMoneyRequestReportView(jest.fn()); + + expect(screen.getByTestId('ReportActionsSkeletonCover')).toBeTruthy(); + expect(mockReportActionsSkeletonView.mock.calls.at(-1)?.at(0)).toEqual(expect.objectContaining({shouldAnimate: false})); + expect(mockReportActionsListBody).not.toHaveBeenCalled(); + expect(mockMoneyRequestReportActionsList).not.toHaveBeenCalled(); + }); + + it('uses the bottom-padded cover while the app is loading', () => { + mockUseIsAppLoadPending.mockReturnValue(true); + + renderMoneyRequestReportView(jest.fn()); + + expect(screen.getByTestId('ReportActionsSkeletonCover')).toBeTruthy(); + expect(screen.UNSAFE_getByType(ReportHeaderSkeletonView).parent).toHaveStyle({height: 80}); + expect(mockReportActionsSkeletonView.mock.calls.at(-1)?.at(0)).toEqual(expect.objectContaining({shouldAnimate: true})); + expect(mockReportActionsListBody).not.toHaveBeenCalled(); + expect(mockMoneyRequestReportActionsList).not.toHaveBeenCalled(); + }); + it('mounts the chat list body and the typing listener (not the table view) for a transaction-thread report', () => { const onLayout = jest.fn(); diff --git a/tests/ui/ReportActionsTest.tsx b/tests/ui/ReportActionsTest.tsx index 18f7f63e2496..6099444f6a52 100644 --- a/tests/ui/ReportActionsTest.tsx +++ b/tests/ui/ReportActionsTest.tsx @@ -149,6 +149,7 @@ describe('ReportActions (orchestrator)', () => { render(); + expect(screen.getByTestId('ReportActionsSkeletonCover')).toBeTruthy(); expect(screen.getByTestId('ReportActionsSkeletonView')).toBeTruthy(); expect(mockReportActionsListBody).not.toHaveBeenCalled(); expect(mockMoneyRequestList).not.toHaveBeenCalled(); @@ -159,6 +160,7 @@ describe('ReportActions (orchestrator)', () => { render(); + expect(screen.getByTestId('ReportActionsSkeletonCover')).toBeTruthy(); expect(screen.getByTestId('ReportActionsSkeletonView')).toBeTruthy(); expect(mockReportActionsListBody).not.toHaveBeenCalled(); expect(mockMoneyRequestList).not.toHaveBeenCalled(); diff --git a/tests/unit/ReportActionsSkeletonCoverTest.tsx b/tests/unit/ReportActionsSkeletonCoverTest.tsx new file mode 100644 index 000000000000..0c73c8083ed1 --- /dev/null +++ b/tests/unit/ReportActionsSkeletonCoverTest.tsx @@ -0,0 +1,37 @@ +import {render, screen} from '@testing-library/react-native'; + +import ReportActionsSkeletonCover, {ReportActionsAnimatedSkeletonCover} from '@components/ReportActionsSkeletonCover'; +import ReportActionsSkeletonView from '@components/ReportActionsSkeletonView'; + +import React from 'react'; + +jest.mock('@components/ReportActionsSkeletonView', () => jest.fn(() => null)); + +const mockReportActionsSkeletonView = jest.mocked(ReportActionsSkeletonView); + +describe('ReportActionsSkeletonCover', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('fills, clips, and bottom-aligns the static report-actions skeleton', () => { + render(); + + expect(screen.getByTestId('ReportActionsSkeletonCover')).toHaveStyle({ + flex: 1, + overflow: 'hidden', + justifyContent: 'flex-end', + paddingBottom: 16, + }); + expect(mockReportActionsSkeletonView).toHaveBeenCalledTimes(1); + expect(mockReportActionsSkeletonView.mock.calls.at(-1)?.at(0)).toEqual(expect.objectContaining({shouldAnimate: false})); + }); + + it('fills the report-actions viewport with an animated skeleton', () => { + render(); + + expect(screen.getByTestId('ReportActionsSkeletonCover')).toBeTruthy(); + expect(mockReportActionsSkeletonView).toHaveBeenCalledTimes(1); + expect(mockReportActionsSkeletonView.mock.calls.at(-1)?.at(0)).toEqual(expect.objectContaining({shouldAnimate: true})); + }); +});