From eef89c99cfa596048146e20d458df5942f144dfd Mon Sep 17 00:00:00 2001 From: Situ Chandra Shil <108292595+situchan@users.noreply.github.com> Date: Tue, 21 Nov 2023 22:02:11 +0600 Subject: [PATCH] Revert "Migrate LHNOptionsList to Flashlist" --- .../LHNOptionsList/LHNOptionsList.js | 45 ++++++++++++++----- src/pages/home/sidebar/SidebarLinks.js | 27 +++++------ src/styles/styles.ts | 1 + tests/perf-test/SidebarLinks.perf-test.js | 6 +-- 4 files changed, 50 insertions(+), 29 deletions(-) diff --git a/src/components/LHNOptionsList/LHNOptionsList.js b/src/components/LHNOptionsList/LHNOptionsList.js index 0d300c5e2179..5e77947187e9 100644 --- a/src/components/LHNOptionsList/LHNOptionsList.js +++ b/src/components/LHNOptionsList/LHNOptionsList.js @@ -1,8 +1,7 @@ -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 {FlatList, View} from 'react-native'; import {withOnyx} from 'react-native-onyx'; import _ from 'underscore'; import participantPropTypes from '@components/participantPropTypes'; @@ -12,7 +11,6 @@ import compose from '@libs/compose'; import * as OptionsListUtils from '@libs/OptionsListUtils'; import reportActionPropTypes from '@pages/home/report/reportActionPropTypes'; import reportPropTypes from '@pages/reportPropTypes'; -import stylePropTypes from '@styles/stylePropTypes'; import useThemeStyles from '@styles/useThemeStyles'; import variables from '@styles/variables'; import CONST from '@src/CONST'; @@ -21,10 +19,12 @@ import OptionRowLHNData from './OptionRowLHNData'; const propTypes = { /** Wrapper style for the section list */ - style: stylePropTypes, + // eslint-disable-next-line react/forbid-prop-types + style: PropTypes.arrayOf(PropTypes.object), /** Extra styles for the section list container */ - contentContainerStyles: stylePropTypes.isRequired, + // eslint-disable-next-line react/forbid-prop-types + contentContainerStyles: PropTypes.arrayOf(PropTypes.object).isRequired, /** Sections for the section list */ data: PropTypes.arrayOf(PropTypes.string).isRequired, @@ -80,7 +80,7 @@ const defaultProps = { ...withCurrentReportIDDefaultProps, }; -const keyExtractor = (item) => `report_${item}`; +const keyExtractor = (item) => item; function LHNOptionsList({ style, @@ -99,6 +99,28 @@ 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 * @@ -142,17 +164,20 @@ function LHNOptionsList({ return ( - ); diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index a59dc81aad54..5e69be266342 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, StyleSheet, View} from 'react-native'; +import {InteractionManager, View} from 'react-native'; import _ from 'underscore'; import LogoComponent from '@assets/images/expensify-wordmark.svg'; import Header from '@components/Header'; @@ -177,21 +177,16 @@ function SidebarLinks({onLinkClick, insets, optionListItems, isLoading, priority - - - {isLoading && ( - - - - )} - + + + {isLoading && } ); } diff --git a/src/styles/styles.ts b/src/styles/styles.ts index c1b78a224eb3..e597f0ec874e 100644 --- a/src/styles/styles.ts +++ b/src/styles/styles.ts @@ -1367,6 +1367,7 @@ const styles = (theme: ThemeColors) => }, sidebarListContainer: { + scrollbarWidth: 'none', paddingBottom: 4, }, diff --git a/tests/perf-test/SidebarLinks.perf-test.js b/tests/perf-test/SidebarLinks.perf-test.js index 5601c588bb93..f6819d40a48f 100644 --- a/tests/perf-test/SidebarLinks.perf-test.js +++ b/tests/perf-test/SidebarLinks.perf-test.js @@ -105,9 +105,9 @@ test('should scroll and click some of the items', () => { expect(lhnOptionsList).toBeDefined(); fireEvent.scroll(lhnOptionsList, eventData); - // find elements that are currently visible in the viewport - const button1 = await screen.findByTestId('7'); - const button2 = await screen.findByTestId('8'); + + const button1 = await screen.findByTestId('1'); + const button2 = await screen.findByTestId('2'); fireEvent.press(button1); fireEvent.press(button2); };