From 58fc9c9048f4147e696e6bfefd78c458f294fb3a Mon Sep 17 00:00:00 2001 From: Mykhailo Kravchenko Date: Mon, 20 Jan 2025 18:30:36 +0100 Subject: [PATCH 01/10] Add window dimensions to LHNOptionsList for dynamic list sizing --- src/components/LHNOptionsList/LHNOptionsList.tsx | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/components/LHNOptionsList/LHNOptionsList.tsx b/src/components/LHNOptionsList/LHNOptionsList.tsx index 9594e6ede24a..599708895e1b 100644 --- a/src/components/LHNOptionsList/LHNOptionsList.tsx +++ b/src/components/LHNOptionsList/LHNOptionsList.tsx @@ -16,6 +16,7 @@ import usePrevious from '@hooks/usePrevious'; import useResponsiveLayout from '@hooks/useResponsiveLayout'; import useTheme from '@hooks/useTheme'; import useThemeStyles from '@hooks/useThemeStyles'; +import useWindowDimensions from '@hooks/useWindowDimensions'; import * as DraftCommentUtils from '@libs/DraftCommentUtils'; import * as OptionsListUtils from '@libs/OptionsListUtils'; import * as ReportActionsUtils from '@libs/ReportActionsUtils'; @@ -47,6 +48,8 @@ function LHNOptionsList({style, contentContainerStyles, data, onSelectRow, optio const styles = useThemeStyles(); const {translate, preferredLocale} = useLocalize(); const {shouldUseNarrowLayout} = useResponsiveLayout(); + const {windowHeight, windowWidth} = useWindowDimensions(); + const listHeight = windowHeight - variables.bottomTabHeight; const shouldShowEmptyLHN = shouldUseNarrowLayout && data.length === 0; // When the first item renders we want to call the onFirstItemRendered callback. @@ -282,6 +285,10 @@ function LHNOptionsList({style, contentContainerStyles, data, onSelectRow, optio showsVerticalScrollIndicator={false} onLayout={onLayout} onScroll={onScroll} + estimatedListSize={{ + height: listHeight, + width: windowWidth, + }} /> )} From 4d6dfea05fcd482355781a416b86e1a52ac4ff24 Mon Sep 17 00:00:00 2001 From: Mykhailo Kravchenko Date: Thu, 23 Jan 2025 17:59:10 +0100 Subject: [PATCH 02/10] Use named imports LHNOptionsList --- .../LHNOptionsList/LHNOptionsList.tsx | 25 ++++++++----------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/src/components/LHNOptionsList/LHNOptionsList.tsx b/src/components/LHNOptionsList/LHNOptionsList.tsx index 599708895e1b..65a2f0f8da95 100644 --- a/src/components/LHNOptionsList/LHNOptionsList.tsx +++ b/src/components/LHNOptionsList/LHNOptionsList.tsx @@ -17,10 +17,10 @@ import useResponsiveLayout from '@hooks/useResponsiveLayout'; import useTheme from '@hooks/useTheme'; import useThemeStyles from '@hooks/useThemeStyles'; import useWindowDimensions from '@hooks/useWindowDimensions'; -import * as DraftCommentUtils from '@libs/DraftCommentUtils'; -import * as OptionsListUtils from '@libs/OptionsListUtils'; -import * as ReportActionsUtils from '@libs/ReportActionsUtils'; -import * as ReportUtils from '@libs/ReportUtils'; +import {isValidDraftComment} from '@libs/DraftCommentUtils'; +import {getIOUReportIDOfLastAction, getLastMessageTextForReport} from '@libs/OptionsListUtils'; +import {getOriginalMessage, getSortedReportActionsForDisplay, isMoneyRequestAction} from '@libs/ReportActionsUtils'; +import {canUserPerformWriteAction} from '@libs/ReportUtils'; import variables from '@styles/variables'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; @@ -134,25 +134,22 @@ function LHNOptionsList({style, contentContainerStyles, data, onSelectRow, optio } const itemInvoiceReceiverPolicy = policy?.[`${ONYXKEYS.COLLECTION.POLICY}${invoiceReceiverPolicyID}`]; - const iouReportIDOfLastAction = OptionsListUtils.getIOUReportIDOfLastAction(itemFullReport); + const iouReportIDOfLastAction = getIOUReportIDOfLastAction(itemFullReport); const itemIouReportReportActions = iouReportIDOfLastAction ? reportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${iouReportIDOfLastAction}`] : undefined; const itemPolicy = policy?.[`${ONYXKEYS.COLLECTION.POLICY}${itemFullReport?.policyID}`]; - const transactionID = ReportActionsUtils.isMoneyRequestAction(itemParentReportAction) - ? ReportActionsUtils.getOriginalMessage(itemParentReportAction)?.IOUTransactionID ?? '-1' - : '-1'; + const transactionID = isMoneyRequestAction(itemParentReportAction) ? getOriginalMessage(itemParentReportAction)?.IOUTransactionID ?? '-1' : '-1'; const itemTransaction = transactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`]; - const hasDraftComment = DraftCommentUtils.isValidDraftComment(draftComments?.[`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${reportID}`]); + const hasDraftComment = isValidDraftComment(draftComments?.[`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${reportID}`]); - const canUserPerformWriteAction = ReportUtils.canUserPerformWriteAction(itemFullReport); - const sortedReportActions = ReportActionsUtils.getSortedReportActionsForDisplay(itemReportActions, canUserPerformWriteAction); + const sortedReportActions = getSortedReportActionsForDisplay(itemReportActions, canUserPerformWriteAction(itemFullReport)); const lastReportAction = sortedReportActions.at(0); // Get the transaction for the last report action let lastReportActionTransactionID = ''; - if (ReportActionsUtils.isMoneyRequestAction(lastReportAction)) { - lastReportActionTransactionID = ReportActionsUtils.getOriginalMessage(lastReportAction)?.IOUTransactionID ?? '-1'; + if (isMoneyRequestAction(lastReportAction)) { + lastReportActionTransactionID = getOriginalMessage(lastReportAction)?.IOUTransactionID ?? '-1'; } const lastReportActionTransaction = transactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${lastReportActionTransactionID}`]; @@ -168,7 +165,7 @@ function LHNOptionsList({style, contentContainerStyles, data, onSelectRow, optio } : null; } - const lastMessageTextFromReport = OptionsListUtils.getLastMessageTextForReport(itemFullReport, lastActorDetails, itemPolicy); + const lastMessageTextFromReport = getLastMessageTextForReport(itemFullReport, lastActorDetails, itemPolicy); return ( Date: Thu, 23 Jan 2025 18:03:57 +0100 Subject: [PATCH 03/10] Do not default string IDs LHNOptionsList --- src/components/LHNOptionsList/LHNOptionsList.tsx | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/components/LHNOptionsList/LHNOptionsList.tsx b/src/components/LHNOptionsList/LHNOptionsList.tsx index 65a2f0f8da95..9bda00ebdf54 100644 --- a/src/components/LHNOptionsList/LHNOptionsList.tsx +++ b/src/components/LHNOptionsList/LHNOptionsList.tsx @@ -120,10 +120,10 @@ function LHNOptionsList({style, contentContainerStyles, data, onSelectRow, optio const renderItem = useCallback( ({item: reportID}: RenderItemProps): ReactElement => { const itemFullReport = reports?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`]; - const itemParentReport = reports?.[`${ONYXKEYS.COLLECTION.REPORT}${itemFullReport?.parentReportID ?? '-1'}`]; + const itemParentReport = reports?.[`${ONYXKEYS.COLLECTION.REPORT}${itemFullReport?.parentReportID}`]; const itemReportActions = reportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`]; const itemParentReportActions = reportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${itemFullReport?.parentReportID}`]; - const itemParentReportAction = itemParentReportActions?.[itemFullReport?.parentReportActionID ?? '-1']; + const itemParentReportAction = itemParentReportActions?.[itemFullReport?.parentReportActionID ?? CONST.DEFAULT_NUMBER_ID]; let invoiceReceiverPolicyID = '-1'; if (itemFullReport?.invoiceReceiver && 'policyID' in itemFullReport.invoiceReceiver) { @@ -138,7 +138,9 @@ function LHNOptionsList({style, contentContainerStyles, data, onSelectRow, optio const itemIouReportReportActions = iouReportIDOfLastAction ? reportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${iouReportIDOfLastAction}`] : undefined; const itemPolicy = policy?.[`${ONYXKEYS.COLLECTION.POLICY}${itemFullReport?.policyID}`]; - const transactionID = isMoneyRequestAction(itemParentReportAction) ? getOriginalMessage(itemParentReportAction)?.IOUTransactionID ?? '-1' : '-1'; + const transactionID = isMoneyRequestAction(itemParentReportAction) + ? getOriginalMessage(itemParentReportAction)?.IOUTransactionID ?? CONST.DEFAULT_NUMBER_ID + : CONST.DEFAULT_NUMBER_ID; const itemTransaction = transactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`]; const hasDraftComment = isValidDraftComment(draftComments?.[`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${reportID}`]); @@ -146,10 +148,10 @@ function LHNOptionsList({style, contentContainerStyles, data, onSelectRow, optio const lastReportAction = sortedReportActions.at(0); // Get the transaction for the last report action - let lastReportActionTransactionID = ''; + let lastReportActionTransactionID: string | number = ''; if (isMoneyRequestAction(lastReportAction)) { - lastReportActionTransactionID = getOriginalMessage(lastReportAction)?.IOUTransactionID ?? '-1'; + lastReportActionTransactionID = getOriginalMessage(lastReportAction)?.IOUTransactionID ?? CONST.DEFAULT_NUMBER_ID; } const lastReportActionTransaction = transactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${lastReportActionTransactionID}`]; From 838ee697ae74d2e23dec99f47c04e617d4ebb6ac Mon Sep 17 00:00:00 2001 From: Mykhailo Kravchenko Date: Thu, 23 Jan 2025 18:55:59 +0100 Subject: [PATCH 04/10] rerun tests From fd4ea52cd4f1a6016d1b2382ac002ab5b29df4b1 Mon Sep 17 00:00:00 2001 From: Mykhailo Kravchenko Date: Mon, 27 Jan 2025 17:49:13 +0100 Subject: [PATCH 05/10] create useEstimatedListSize --- src/hooks/useEstimatedListSize/index.native.ts | 15 +++++++++++++++ src/hooks/useEstimatedListSize/index.ts | 5 +++++ src/hooks/useEstimatedListSize/types.ts | 8 ++++++++ 3 files changed, 28 insertions(+) create mode 100644 src/hooks/useEstimatedListSize/index.native.ts create mode 100644 src/hooks/useEstimatedListSize/index.ts create mode 100644 src/hooks/useEstimatedListSize/types.ts diff --git a/src/hooks/useEstimatedListSize/index.native.ts b/src/hooks/useEstimatedListSize/index.native.ts new file mode 100644 index 000000000000..1c5f985bdcd7 --- /dev/null +++ b/src/hooks/useEstimatedListSize/index.native.ts @@ -0,0 +1,15 @@ +import useWindowDimensions from '@hooks/useWindowDimensions'; +import variables from '@styles/variables'; +import type UseEstimatedListSize from './types'; + +const useEstimatedListSize: UseEstimatedListSize = () => { + const {windowHeight, windowWidth} = useWindowDimensions(); + const listHeight = windowHeight - variables.bottomTabHeight; + + return { + height: listHeight, + width: windowWidth, + }; +}; + +export default useEstimatedListSize; diff --git a/src/hooks/useEstimatedListSize/index.ts b/src/hooks/useEstimatedListSize/index.ts new file mode 100644 index 000000000000..814d7593102f --- /dev/null +++ b/src/hooks/useEstimatedListSize/index.ts @@ -0,0 +1,5 @@ +import type UseEstimatedListSize from './types'; + +const useEstimatedListSize: UseEstimatedListSize = () => undefined; + +export default useEstimatedListSize; diff --git a/src/hooks/useEstimatedListSize/types.ts b/src/hooks/useEstimatedListSize/types.ts new file mode 100644 index 000000000000..9abc97eb01f8 --- /dev/null +++ b/src/hooks/useEstimatedListSize/types.ts @@ -0,0 +1,8 @@ +type UseEstimatedListSize = () => + | { + height: number; + width: number; + } + | undefined; + +export default UseEstimatedListSize; From 45220f2f17be9b2bf6d2bde763220385d20f7f75 Mon Sep 17 00:00:00 2001 From: Mykhailo Kravchenko Date: Mon, 27 Jan 2025 17:49:25 +0100 Subject: [PATCH 06/10] use useEstimatedListSize --- src/components/LHNOptionsList/LHNOptionsList.tsx | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/components/LHNOptionsList/LHNOptionsList.tsx b/src/components/LHNOptionsList/LHNOptionsList.tsx index 81709b38629d..3a788c738ddc 100644 --- a/src/components/LHNOptionsList/LHNOptionsList.tsx +++ b/src/components/LHNOptionsList/LHNOptionsList.tsx @@ -11,12 +11,12 @@ import * as Expensicons from '@components/Icon/Expensicons'; import LottieAnimations from '@components/LottieAnimations'; import {ScrollOffsetContext} from '@components/ScrollOffsetContextProvider'; import TextBlock from '@components/TextBlock'; +import useEstimatedListSize from '@hooks/useEstimatedListSize'; import useLocalize from '@hooks/useLocalize'; import usePrevious from '@hooks/usePrevious'; import useResponsiveLayout from '@hooks/useResponsiveLayout'; import useTheme from '@hooks/useTheme'; import useThemeStyles from '@hooks/useThemeStyles'; -import useWindowDimensions from '@hooks/useWindowDimensions'; import {isValidDraftComment} from '@libs/DraftCommentUtils'; import {getIOUReportIDOfLastAction, getLastMessageTextForReport} from '@libs/OptionsListUtils'; import {getOriginalMessage, getSortedReportActionsForDisplay, isMoneyRequestAction} from '@libs/ReportActionsUtils'; @@ -49,8 +49,7 @@ function LHNOptionsList({style, contentContainerStyles, data, onSelectRow, optio const styles = useThemeStyles(); const {translate, preferredLocale} = useLocalize(); const {shouldUseNarrowLayout} = useResponsiveLayout(); - const {windowHeight, windowWidth} = useWindowDimensions(); - const listHeight = windowHeight - variables.bottomTabHeight; + const estimatedListSize = useEstimatedListSize(); const shouldShowEmptyLHN = shouldUseNarrowLayout && data.length === 0; // When the first item renders we want to call the onFirstItemRendered callback. @@ -287,10 +286,7 @@ function LHNOptionsList({style, contentContainerStyles, data, onSelectRow, optio showsVerticalScrollIndicator={false} onLayout={onLayout} onScroll={onScroll} - estimatedListSize={{ - height: listHeight, - width: windowWidth, - }} + estimatedListSize={estimatedListSize} /> )} From 717c1846271f95e372f5e920b2d66e45bb51468a Mon Sep 17 00:00:00 2001 From: Mykhailo Kravchenko Date: Wed, 29 Jan 2025 10:47:07 +0100 Subject: [PATCH 07/10] Rename and refactor estimated list size hook for LHN usage --- src/components/LHNOptionsList/LHNOptionsList.tsx | 4 ++-- src/hooks/useEstimatedListSize/index.ts | 5 ----- .../index.native.ts | 6 +++--- src/hooks/useLHNEstimatedListSize/index.ts | 5 +++++ .../types.ts | 4 ++-- 5 files changed, 12 insertions(+), 12 deletions(-) delete mode 100644 src/hooks/useEstimatedListSize/index.ts rename src/hooks/{useEstimatedListSize => useLHNEstimatedListSize}/index.native.ts (66%) create mode 100644 src/hooks/useLHNEstimatedListSize/index.ts rename src/hooks/{useEstimatedListSize => useLHNEstimatedListSize}/types.ts (52%) diff --git a/src/components/LHNOptionsList/LHNOptionsList.tsx b/src/components/LHNOptionsList/LHNOptionsList.tsx index 3a788c738ddc..e48646204f34 100644 --- a/src/components/LHNOptionsList/LHNOptionsList.tsx +++ b/src/components/LHNOptionsList/LHNOptionsList.tsx @@ -11,7 +11,7 @@ import * as Expensicons from '@components/Icon/Expensicons'; import LottieAnimations from '@components/LottieAnimations'; import {ScrollOffsetContext} from '@components/ScrollOffsetContextProvider'; import TextBlock from '@components/TextBlock'; -import useEstimatedListSize from '@hooks/useEstimatedListSize'; +import useLHNEstimatedListSize from '@hooks/useLHNEstimatedListSize'; import useLocalize from '@hooks/useLocalize'; import usePrevious from '@hooks/usePrevious'; import useResponsiveLayout from '@hooks/useResponsiveLayout'; @@ -49,7 +49,7 @@ function LHNOptionsList({style, contentContainerStyles, data, onSelectRow, optio const styles = useThemeStyles(); const {translate, preferredLocale} = useLocalize(); const {shouldUseNarrowLayout} = useResponsiveLayout(); - const estimatedListSize = useEstimatedListSize(); + const estimatedListSize = useLHNEstimatedListSize(); const shouldShowEmptyLHN = shouldUseNarrowLayout && data.length === 0; // When the first item renders we want to call the onFirstItemRendered callback. diff --git a/src/hooks/useEstimatedListSize/index.ts b/src/hooks/useEstimatedListSize/index.ts deleted file mode 100644 index 814d7593102f..000000000000 --- a/src/hooks/useEstimatedListSize/index.ts +++ /dev/null @@ -1,5 +0,0 @@ -import type UseEstimatedListSize from './types'; - -const useEstimatedListSize: UseEstimatedListSize = () => undefined; - -export default useEstimatedListSize; diff --git a/src/hooks/useEstimatedListSize/index.native.ts b/src/hooks/useLHNEstimatedListSize/index.native.ts similarity index 66% rename from src/hooks/useEstimatedListSize/index.native.ts rename to src/hooks/useLHNEstimatedListSize/index.native.ts index 1c5f985bdcd7..6355ca46d6bb 100644 --- a/src/hooks/useEstimatedListSize/index.native.ts +++ b/src/hooks/useLHNEstimatedListSize/index.native.ts @@ -1,8 +1,8 @@ import useWindowDimensions from '@hooks/useWindowDimensions'; import variables from '@styles/variables'; -import type UseEstimatedListSize from './types'; +import type UseLHNEstimatedListSize from './types'; -const useEstimatedListSize: UseEstimatedListSize = () => { +const useLHNEstimatedListSize: UseLHNEstimatedListSize = () => { const {windowHeight, windowWidth} = useWindowDimensions(); const listHeight = windowHeight - variables.bottomTabHeight; @@ -12,4 +12,4 @@ const useEstimatedListSize: UseEstimatedListSize = () => { }; }; -export default useEstimatedListSize; +export default useLHNEstimatedListSize; diff --git a/src/hooks/useLHNEstimatedListSize/index.ts b/src/hooks/useLHNEstimatedListSize/index.ts new file mode 100644 index 000000000000..6fd9bcfe13b0 --- /dev/null +++ b/src/hooks/useLHNEstimatedListSize/index.ts @@ -0,0 +1,5 @@ +import type UseLHNEstimatedListSize from './types'; + +const useLHNEstimatedListSize: UseLHNEstimatedListSize = () => undefined; + +export default useLHNEstimatedListSize; diff --git a/src/hooks/useEstimatedListSize/types.ts b/src/hooks/useLHNEstimatedListSize/types.ts similarity index 52% rename from src/hooks/useEstimatedListSize/types.ts rename to src/hooks/useLHNEstimatedListSize/types.ts index 9abc97eb01f8..dbc3e3b7226a 100644 --- a/src/hooks/useEstimatedListSize/types.ts +++ b/src/hooks/useLHNEstimatedListSize/types.ts @@ -1,8 +1,8 @@ -type UseEstimatedListSize = () => +type UseLHNEstimatedListSize = () => | { height: number; width: number; } | undefined; -export default UseEstimatedListSize; +export default UseLHNEstimatedListSize; From 88f6bdabe1d39c180977c2b7787cfb11d654ef4f Mon Sep 17 00:00:00 2001 From: Mykhailo Kravchenko Date: Wed, 29 Jan 2025 10:52:30 +0100 Subject: [PATCH 08/10] Add documentation for native and web implementations of useLHNEstimatedListSize hook --- src/hooks/useLHNEstimatedListSize/index.native.ts | 3 +++ src/hooks/useLHNEstimatedListSize/index.ts | 3 +++ 2 files changed, 6 insertions(+) diff --git a/src/hooks/useLHNEstimatedListSize/index.native.ts b/src/hooks/useLHNEstimatedListSize/index.native.ts index 6355ca46d6bb..a26e7a8f65eb 100644 --- a/src/hooks/useLHNEstimatedListSize/index.native.ts +++ b/src/hooks/useLHNEstimatedListSize/index.native.ts @@ -2,6 +2,9 @@ import useWindowDimensions from '@hooks/useWindowDimensions'; import variables from '@styles/variables'; import type UseLHNEstimatedListSize from './types'; +/** + * This a native specific implementation for FlashList of LHNOptionsList. It calculates estimated visible height and width of the list. It is not the scroll content size. Defining this prop will enable the list to be rendered immediately. Without it, the list first needs to measure its size, leading to a small delay during the first render. + */ const useLHNEstimatedListSize: UseLHNEstimatedListSize = () => { const {windowHeight, windowWidth} = useWindowDimensions(); const listHeight = windowHeight - variables.bottomTabHeight; diff --git a/src/hooks/useLHNEstimatedListSize/index.ts b/src/hooks/useLHNEstimatedListSize/index.ts index 6fd9bcfe13b0..d9b5cb9d3ec0 100644 --- a/src/hooks/useLHNEstimatedListSize/index.ts +++ b/src/hooks/useLHNEstimatedListSize/index.ts @@ -1,5 +1,8 @@ import type UseLHNEstimatedListSize from './types'; +/** + * This is the web implementation. It is intentionally left unimplemented because it does not function correctly on the web. + */ const useLHNEstimatedListSize: UseLHNEstimatedListSize = () => undefined; export default useLHNEstimatedListSize; From 6f2c91ee3afb1026b9053404c97b6072fcce9944 Mon Sep 17 00:00:00 2001 From: Mykhailo Kravchenko Date: Fri, 31 Jan 2025 12:54:57 +0100 Subject: [PATCH 09/10] retest From 8576b7e1f272c88efb94a6f97bf943a3278d09c8 Mon Sep 17 00:00:00 2001 From: Mykhailo Kravchenko Date: Fri, 31 Jan 2025 13:19:07 +0100 Subject: [PATCH 10/10] Add mock for useLHNEstimatedListSize hook in SidebarLinks performance tests --- tests/perf-test/SidebarLinks.perf-test.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/perf-test/SidebarLinks.perf-test.tsx b/tests/perf-test/SidebarLinks.perf-test.tsx index 7d1e4687eb21..5b24bc4c8bd1 100644 --- a/tests/perf-test/SidebarLinks.perf-test.tsx +++ b/tests/perf-test/SidebarLinks.perf-test.tsx @@ -28,6 +28,7 @@ jest.mock('../../src/libs/Navigation/navigationRef', () => ({ jest.mock('@components/Icon/Expensicons'); jest.mock('@react-navigation/native'); +jest.mock('@src/hooks/useLHNEstimatedListSize/index.native.ts'); const getMockedReportsMap = (length = 100) => { const mockReports = Object.fromEntries(