Skip to content

Commit

Permalink
Merge pull request #31642 from situchan/revert-31052-feat/lhn-flashli…
Browse files Browse the repository at this point in the history
…st-migration

Revert "Migrate LHNOptionsList to Flashlist"
  • Loading branch information
Julesssss authored Nov 21, 2023
2 parents 6507621 + eef89c9 commit ed72f09
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 29 deletions.
45 changes: 35 additions & 10 deletions src/components/LHNOptionsList/LHNOptionsList.js
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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';
Expand All @@ -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,
Expand Down Expand Up @@ -80,7 +80,7 @@ const defaultProps = {
...withCurrentReportIDDefaultProps,
};

const keyExtractor = (item) => `report_${item}`;
const keyExtractor = (item) => item;

function LHNOptionsList({
style,
Expand All @@ -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
*
Expand Down Expand Up @@ -142,17 +164,20 @@ function LHNOptionsList({

return (
<View style={style || styles.flex1}>
<FlashList
<FlatList
indicatorStyle="white"
keyboardShouldPersistTaps="always"
contentContainerStyle={contentContainerStyles}
showsVerticalScrollIndicator={false}
data={data}
testID="lhn-options-list"
keyExtractor={keyExtractor}
stickySectionHeadersEnabled={false}
renderItem={renderItem}
estimatedItemSize={optionMode === CONST.OPTION_MODE.COMPACT ? variables.optionRowHeightCompact : variables.optionRowHeight}
extraData={[currentReportID]}
showsVerticalScrollIndicator={false}
getItemLayout={getItemLayout}
initialNumToRender={20}
maxToRenderPerBatch={5}
windowSize={5}
/>
</View>
);
Expand Down
27 changes: 11 additions & 16 deletions src/pages/home/sidebar/SidebarLinks.js
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -177,21 +177,16 @@ function SidebarLinks({onLinkClick, insets, optionListItems, isLoading, priority
</Tooltip>
<SignInOrAvatarWithOptionalStatus isCreateMenuOpen={isCreateMenuOpen} />
</View>
<View style={[styles.pRelative, styles.flex1]}>
<LHNOptionsList
style={styles.flex1}
contentContainerStyles={StyleSheet.flatten([styles.sidebarListContainer, {paddingBottom: StyleUtils.getSafeAreaMargins(insets).marginBottom}])}
data={optionListItems}
onSelectRow={showReportPage}
shouldDisableFocusOptions={isSmallScreenWidth}
optionMode={viewMode}
/>
{isLoading && (
<View style={[StyleSheet.absoluteFillObject, styles.appBG]}>
<OptionsListSkeletonView shouldAnimate />
</View>
)}
</View>

<LHNOptionsList
style={[isLoading ? styles.flexShrink1 : styles.flex1]}
contentContainerStyles={[styles.sidebarListContainer, {paddingBottom: StyleUtils.getSafeAreaMargins(insets).marginBottom}]}
data={optionListItems}
onSelectRow={showReportPage}
shouldDisableFocusOptions={isSmallScreenWidth}
optionMode={viewMode}
/>
{isLoading && <OptionsListSkeletonView shouldAnimate />}
</View>
);
}
Expand Down
1 change: 1 addition & 0 deletions src/styles/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1367,6 +1367,7 @@ const styles = (theme: ThemeColors) =>
},

sidebarListContainer: {
scrollbarWidth: 'none',
paddingBottom: 4,
},

Expand Down
6 changes: 3 additions & 3 deletions tests/perf-test/SidebarLinks.perf-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};
Expand Down

0 comments on commit ed72f09

Please sign in to comment.