From 3a696218591cf191e50484993fee8f04b6469fe4 Mon Sep 17 00:00:00 2001 From: Tomasz Misiukiewicz Date: Thu, 16 Nov 2023 16:20:47 +0100 Subject: [PATCH 01/10] migrate LHNOptionsList to FlashList --- .../LHNOptionsList/LHNOptionsList.js | 37 +++---------------- src/pages/home/sidebar/SidebarLinks.js | 4 +- src/styles/styles.ts | 1 - 3 files changed, 7 insertions(+), 35 deletions(-) diff --git a/src/components/LHNOptionsList/LHNOptionsList.js b/src/components/LHNOptionsList/LHNOptionsList.js index 5e77947187e9..5bc08a91fc82 100644 --- a/src/components/LHNOptionsList/LHNOptionsList.js +++ b/src/components/LHNOptionsList/LHNOptionsList.js @@ -1,8 +1,9 @@ import lodashGet from 'lodash/get'; import PropTypes from 'prop-types'; import React, {useCallback} from 'react'; -import {FlatList, View} from 'react-native'; +import {View} from 'react-native'; import {withOnyx} from 'react-native-onyx'; +import {FlashList} from '@shopify/flash-list'; import _ from 'underscore'; import participantPropTypes from '@components/participantPropTypes'; import transactionPropTypes from '@components/transactionPropTypes'; @@ -23,8 +24,7 @@ const propTypes = { style: PropTypes.arrayOf(PropTypes.object), /** Extra styles for the section list container */ - // eslint-disable-next-line react/forbid-prop-types - contentContainerStyles: PropTypes.arrayOf(PropTypes.object).isRequired, + contentContainerStyles: PropTypes.oneOfType([PropTypes.arrayOf(PropTypes.object), PropTypes.object]).isRequired, /** Sections for the section list */ data: PropTypes.arrayOf(PropTypes.string).isRequired, @@ -99,28 +99,6 @@ function LHNOptionsList({ currentReportID, }) { const styles = useThemeStyles(); - /** - * This function is used to compute the layout of any given item in our list. Since we know that each item will have the exact same height, this is a performance optimization - * so that the heights can be determined before the options are rendered. Otherwise, the heights are determined when each option is rendering and it causes a lot of overhead on large - * lists. - * - * @param {Array} itemData - This is the same as the data we pass into the component - * @param {Number} index the current item's index in the set of data - * - * @returns {Object} - */ - const getItemLayout = useCallback( - (itemData, index) => { - const optionHeight = optionMode === CONST.OPTION_MODE.COMPACT ? variables.optionRowHeightCompact : variables.optionRowHeight; - return { - length: optionHeight, - offset: index * optionHeight, - index, - }; - }, - [optionMode], - ); - /** * Function which renders a row in the list * @@ -164,20 +142,15 @@ function LHNOptionsList({ return ( - ); diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index 5e69be266342..e8d011c6da29 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -1,7 +1,7 @@ /* eslint-disable rulesdir/onyx-props-must-have-default */ import PropTypes from 'prop-types'; import React, {useCallback, useEffect, useRef} from 'react'; -import {InteractionManager, View} from 'react-native'; +import {InteractionManager, View, StyleSheet} from 'react-native'; import _ from 'underscore'; import LogoComponent from '@assets/images/expensify-wordmark.svg'; import Header from '@components/Header'; @@ -180,7 +180,7 @@ function SidebarLinks({onLinkClick, insets, optionListItems, isLoading, priority }, sidebarListContainer: { - scrollbarWidth: 'none', paddingBottom: 4, }, From b52995e95f8d23e97bda5ef1bdcbf5fa742d1cb4 Mon Sep 17 00:00:00 2001 From: Tomasz Misiukiewicz Date: Wed, 22 Nov 2023 15:34:58 +0100 Subject: [PATCH 02/10] migrate LHNOptionsList to FlashList --- src/components/LHNOptionsList/LHNOptionsList.js | 2 +- src/pages/home/sidebar/SidebarLinks.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/LHNOptionsList/LHNOptionsList.js b/src/components/LHNOptionsList/LHNOptionsList.js index 5bc08a91fc82..bdcea888b20f 100644 --- a/src/components/LHNOptionsList/LHNOptionsList.js +++ b/src/components/LHNOptionsList/LHNOptionsList.js @@ -1,9 +1,9 @@ +import {FlashList} from '@shopify/flash-list'; import lodashGet from 'lodash/get'; import PropTypes from 'prop-types'; import React, {useCallback} from 'react'; import {View} from 'react-native'; import {withOnyx} from 'react-native-onyx'; -import {FlashList} from '@shopify/flash-list'; import _ from 'underscore'; import participantPropTypes from '@components/participantPropTypes'; import transactionPropTypes from '@components/transactionPropTypes'; diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index e8d011c6da29..1c57ba0ba157 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -1,7 +1,7 @@ /* eslint-disable rulesdir/onyx-props-must-have-default */ import PropTypes from 'prop-types'; import React, {useCallback, useEffect, useRef} from 'react'; -import {InteractionManager, View, StyleSheet} from 'react-native'; +import {InteractionManager, StyleSheet, View} from 'react-native'; import _ from 'underscore'; import LogoComponent from '@assets/images/expensify-wordmark.svg'; import Header from '@components/Header'; From c94289a26d432f770710d184fd5bf0d824be9191 Mon Sep 17 00:00:00 2001 From: Tomasz Misiukiewicz Date: Thu, 16 Nov 2023 16:20:48 +0100 Subject: [PATCH 03/10] add currentReportID to extraData --- src/components/LHNOptionsList/LHNOptionsList.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/LHNOptionsList/LHNOptionsList.js b/src/components/LHNOptionsList/LHNOptionsList.js index bdcea888b20f..8a3481ef31e0 100644 --- a/src/components/LHNOptionsList/LHNOptionsList.js +++ b/src/components/LHNOptionsList/LHNOptionsList.js @@ -151,6 +151,7 @@ function LHNOptionsList({ keyExtractor={keyExtractor} renderItem={renderItem} estimatedItemSize={optionMode === CONST.OPTION_MODE.COMPACT ? variables.optionRowHeightCompact : variables.optionRowHeight} + extraData={currentReportID} /> ); From 3904226690c31d4c09633d15a31995c004b81bb5 Mon Sep 17 00:00:00 2001 From: Tomasz Misiukiewicz Date: Thu, 16 Nov 2023 16:20:48 +0100 Subject: [PATCH 04/10] render list under the skeleton --- .../LHNOptionsList/LHNOptionsList.js | 2 +- src/pages/home/sidebar/SidebarLinks.js | 25 +++++++++++-------- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/src/components/LHNOptionsList/LHNOptionsList.js b/src/components/LHNOptionsList/LHNOptionsList.js index 8a3481ef31e0..13f8513299e7 100644 --- a/src/components/LHNOptionsList/LHNOptionsList.js +++ b/src/components/LHNOptionsList/LHNOptionsList.js @@ -21,7 +21,7 @@ import OptionRowLHNData from './OptionRowLHNData'; const propTypes = { /** Wrapper style for the section list */ // eslint-disable-next-line react/forbid-prop-types - style: PropTypes.arrayOf(PropTypes.object), + style: PropTypes.oneOfType([PropTypes.arrayOf(PropTypes.object), PropTypes.object]), /** Extra styles for the section list container */ contentContainerStyles: PropTypes.oneOfType([PropTypes.arrayOf(PropTypes.object), PropTypes.object]).isRequired, diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index 1c57ba0ba157..a59dc81aad54 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -177,16 +177,21 @@ function SidebarLinks({onLinkClick, insets, optionListItems, isLoading, priority - - - {isLoading && } + + + {isLoading && ( + + + + )} + ); } From a5f09fe6f3ad7112655c66fd90f7aaba331a092d Mon Sep 17 00:00:00 2001 From: Tomasz Misiukiewicz Date: Thu, 16 Nov 2023 16:21:57 +0100 Subject: [PATCH 05/10] use stylePropTypes --- src/components/LHNOptionsList/LHNOptionsList.js | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/components/LHNOptionsList/LHNOptionsList.js b/src/components/LHNOptionsList/LHNOptionsList.js index 13f8513299e7..e4e412a78649 100644 --- a/src/components/LHNOptionsList/LHNOptionsList.js +++ b/src/components/LHNOptionsList/LHNOptionsList.js @@ -13,6 +13,8 @@ import * as OptionsListUtils from '@libs/OptionsListUtils'; import reportActionPropTypes from '@pages/home/report/reportActionPropTypes'; import reportPropTypes from '@pages/reportPropTypes'; import useThemeStyles from '@styles/useThemeStyles'; +import stylePropTypes from '@styles/stylePropTypes'; +import styles from '@styles/styles'; import variables from '@styles/variables'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; @@ -20,11 +22,10 @@ import OptionRowLHNData from './OptionRowLHNData'; const propTypes = { /** Wrapper style for the section list */ - // eslint-disable-next-line react/forbid-prop-types - style: PropTypes.oneOfType([PropTypes.arrayOf(PropTypes.object), PropTypes.object]), + style: stylePropTypes, /** Extra styles for the section list container */ - contentContainerStyles: PropTypes.oneOfType([PropTypes.arrayOf(PropTypes.object), PropTypes.object]).isRequired, + contentContainerStyles: stylePropTypes.isRequired, /** Sections for the section list */ data: PropTypes.arrayOf(PropTypes.string).isRequired, @@ -98,7 +99,7 @@ function LHNOptionsList({ draftComments, currentReportID, }) { - const styles = useThemeStyles(); + const themeStyles = useThemeStyles(); /** * Function which renders a row in the list * @@ -141,7 +142,7 @@ function LHNOptionsList({ ); return ( - + ); From 6a393f2982a61d8b45751debbac9257ad5e3fd14 Mon Sep 17 00:00:00 2001 From: Tomasz Misiukiewicz Date: Thu, 16 Nov 2023 16:21:58 +0100 Subject: [PATCH 06/10] update keyExtractor --- src/components/LHNOptionsList/LHNOptionsList.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/LHNOptionsList/LHNOptionsList.js b/src/components/LHNOptionsList/LHNOptionsList.js index e4e412a78649..344fc0dcc0b4 100644 --- a/src/components/LHNOptionsList/LHNOptionsList.js +++ b/src/components/LHNOptionsList/LHNOptionsList.js @@ -81,7 +81,7 @@ const defaultProps = { ...withCurrentReportIDDefaultProps, }; -const keyExtractor = (item) => item; +const keyExtractor = (item) => `report_${item}`; function LHNOptionsList({ style, From da1626cf7b7d393cebad1fbe70d9d2883182930c Mon Sep 17 00:00:00 2001 From: Tomasz Misiukiewicz Date: Thu, 16 Nov 2023 16:21:58 +0100 Subject: [PATCH 07/10] fix reassure tests --- src/components/LHNOptionsList/LHNOptionsList.js | 7 +++---- tests/perf-test/SidebarLinks.perf-test.js | 6 +++--- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/components/LHNOptionsList/LHNOptionsList.js b/src/components/LHNOptionsList/LHNOptionsList.js index 344fc0dcc0b4..0679267bfdf9 100644 --- a/src/components/LHNOptionsList/LHNOptionsList.js +++ b/src/components/LHNOptionsList/LHNOptionsList.js @@ -12,9 +12,8 @@ import compose from '@libs/compose'; import * as OptionsListUtils from '@libs/OptionsListUtils'; import reportActionPropTypes from '@pages/home/report/reportActionPropTypes'; import reportPropTypes from '@pages/reportPropTypes'; -import useThemeStyles from '@styles/useThemeStyles'; import stylePropTypes from '@styles/stylePropTypes'; -import styles from '@styles/styles'; +import useThemeStyles from '@styles/useThemeStyles'; import variables from '@styles/variables'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; @@ -99,7 +98,7 @@ function LHNOptionsList({ draftComments, currentReportID, }) { - const themeStyles = useThemeStyles(); + const styles = useThemeStyles(); /** * Function which renders a row in the list * @@ -142,7 +141,7 @@ function LHNOptionsList({ ); return ( - + { expect(lhnOptionsList).toBeDefined(); fireEvent.scroll(lhnOptionsList, eventData); - - const button1 = await screen.findByTestId('1'); - const button2 = await screen.findByTestId('2'); + // find elements that are currently visible in the viewport + const button1 = await screen.findByTestId('7'); + const button2 = await screen.findByTestId('8'); fireEvent.press(button1); fireEvent.press(button2); }; From 963b4f16ae59bc074fd265c2cf00d2402c5d0fd3 Mon Sep 17 00:00:00 2001 From: Tomasz Misiukiewicz Date: Fri, 17 Nov 2023 08:45:41 +0100 Subject: [PATCH 08/10] hide vertical scroll indicator --- src/components/LHNOptionsList/LHNOptionsList.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/LHNOptionsList/LHNOptionsList.js b/src/components/LHNOptionsList/LHNOptionsList.js index 0679267bfdf9..0d300c5e2179 100644 --- a/src/components/LHNOptionsList/LHNOptionsList.js +++ b/src/components/LHNOptionsList/LHNOptionsList.js @@ -152,6 +152,7 @@ function LHNOptionsList({ renderItem={renderItem} estimatedItemSize={optionMode === CONST.OPTION_MODE.COMPACT ? variables.optionRowHeightCompact : variables.optionRowHeight} extraData={[currentReportID]} + showsVerticalScrollIndicator={false} /> ); From 48cddcab04861fe7b50da5916de04d400cff794d Mon Sep 17 00:00:00 2001 From: Tomasz Misiukiewicz Date: Wed, 22 Nov 2023 15:44:42 +0100 Subject: [PATCH 09/10] prevent displaying skeleton when list items are ready --- src/pages/home/sidebar/SidebarLinks.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index a59dc81aad54..b4128ed34caa 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -186,7 +186,7 @@ function SidebarLinks({onLinkClick, insets, optionListItems, isLoading, priority shouldDisableFocusOptions={isSmallScreenWidth} optionMode={viewMode} /> - {isLoading && ( + {isLoading && optionListItems.length === 0 && ( From e18243fb2d605da5f25f331a5d1490217834c4a1 Mon Sep 17 00:00:00 2001 From: Tomasz Misiukiewicz Date: Wed, 22 Nov 2023 15:44:57 +0100 Subject: [PATCH 10/10] fix bg color in skeleton --- src/pages/home/sidebar/SidebarLinks.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index b4128ed34caa..2aba742f157f 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -187,7 +187,7 @@ function SidebarLinks({onLinkClick, insets, optionListItems, isLoading, priority optionMode={viewMode} /> {isLoading && optionListItems.length === 0 && ( - + )}