From 5de34fba8a874c66abed081a6e94509d1bb2d3bf Mon Sep 17 00:00:00 2001 From: hurali97 Date: Tue, 14 Nov 2023 17:50:11 +0500 Subject: [PATCH 01/14] perf: add memoization This memoizes relevant functions and values to not re-render LHNOptionsList and ReportActionsList when there's some update in react tree which is not relevant --- src/components/LHNOptionsList/LHNOptionsList.js | 4 ++-- src/pages/home/report/ReportActionsList.js | 4 ++-- src/pages/home/report/ReportActionsView.js | 14 +++++++------- src/pages/home/sidebar/SidebarLinks.js | 8 +++++--- 4 files changed, 16 insertions(+), 14 deletions(-) diff --git a/src/components/LHNOptionsList/LHNOptionsList.js b/src/components/LHNOptionsList/LHNOptionsList.js index ef1954aeb948..ec031c041c0e 100644 --- a/src/components/LHNOptionsList/LHNOptionsList.js +++ b/src/components/LHNOptionsList/LHNOptionsList.js @@ -1,6 +1,6 @@ import lodashGet from 'lodash/get'; import PropTypes from 'prop-types'; -import React, {useCallback} from 'react'; +import React, {memo, useCallback} from 'react'; import {FlatList, View} from 'react-native'; import {withOnyx} from 'react-native-onyx'; import _ from 'underscore'; @@ -211,4 +211,4 @@ export default compose( key: ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT, }, }), -)(LHNOptionsList); +)(memo(LHNOptionsList)); diff --git a/src/pages/home/report/ReportActionsList.js b/src/pages/home/report/ReportActionsList.js index 759e73aa90e5..51dce09610d4 100644 --- a/src/pages/home/report/ReportActionsList.js +++ b/src/pages/home/report/ReportActionsList.js @@ -1,7 +1,7 @@ import {useRoute} from '@react-navigation/native'; import lodashGet from 'lodash/get'; import PropTypes from 'prop-types'; -import React, {useCallback, useEffect, useMemo, useRef, useState} from 'react'; +import React, {memo, useCallback, useEffect, useMemo, useRef, useState} from 'react'; import Animated, {useAnimatedStyle, useSharedValue, withTiming} from 'react-native-reanimated'; import _ from 'underscore'; import InvertedFlatList from '@components/InvertedFlatList'; @@ -443,4 +443,4 @@ ReportActionsList.propTypes = propTypes; ReportActionsList.defaultProps = defaultProps; ReportActionsList.displayName = 'ReportActionsList'; -export default compose(withWindowDimensions, withPersonalDetails(), withCurrentUserPersonalDetails)(ReportActionsList); +export default compose(withWindowDimensions, withPersonalDetails(), withCurrentUserPersonalDetails)(memo(ReportActionsList)); diff --git a/src/pages/home/report/ReportActionsView.js b/src/pages/home/report/ReportActionsView.js index 01ec967d76b1..761c6933ff3f 100755 --- a/src/pages/home/report/ReportActionsView.js +++ b/src/pages/home/report/ReportActionsView.js @@ -1,7 +1,7 @@ import {useIsFocused} from '@react-navigation/native'; import lodashGet from 'lodash/get'; import PropTypes from 'prop-types'; -import React, {useContext, useEffect, useMemo, useRef} from 'react'; +import React, {useCallback, useContext, useEffect, useMemo, useRef} from 'react'; import {withOnyx} from 'react-native-onyx'; import _ from 'underscore'; import networkPropTypes from '@components/networkPropTypes'; @@ -172,25 +172,25 @@ function ReportActionsView(props) { } }, [props.report, didSubscribeToReportTypingEvents, reportID]); + const oldestReportAction = useMemo(() => _.last(props.reportActions), [props.reportActions]); + /** * Retrieves the next set of report actions for the chat once we are nearing the end of what we are currently * displaying. */ - const loadOlderChats = () => { + const loadOlderChats = useCallback(() => { // Only fetch more if we are neither already fetching (so that we don't initiate duplicate requests) nor offline. if (props.network.isOffline || props.isLoadingOlderReportActions) { return; } - const oldestReportAction = _.last(props.reportActions); - // Don't load more chats if we're already at the beginning of the chat history if (oldestReportAction.actionName === CONST.REPORT.ACTIONS.TYPE.CREATED) { return; } // Retrieve the next REPORT.ACTIONS.LIMIT sized page of comments Report.getOlderActions(reportID, oldestReportAction.reportActionID); - }; + }, [props.network.isOffline, props.isLoadingOlderReportActions, oldestReportAction.actionName, oldestReportAction.reportActionID, reportID]); /** * Retrieves the next set of report actions for the chat once we are nearing the end of what we are currently @@ -227,7 +227,7 @@ function ReportActionsView(props) { /** * Runs when the FlatList finishes laying out */ - const recordTimeToMeasureItemLayout = () => { + const recordTimeToMeasureItemLayout = useCallback(() => { if (didLayout.current) { return; } @@ -242,7 +242,7 @@ function ReportActionsView(props) { } else { Performance.markEnd(CONST.TIMING.SWITCH_REPORT); } - }; + }, [hasCachedActions]); // Comments have not loaded at all yet do nothing if (!_.size(props.reportActions)) { diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index ad981a190a70..e6dee6f213d4 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -1,6 +1,6 @@ /* eslint-disable rulesdir/onyx-props-must-have-default */ import PropTypes from 'prop-types'; -import React, {useCallback, useEffect, useRef} from 'react'; +import React, {useCallback, useEffect, useMemo, useRef} from 'react'; import {InteractionManager, View} from 'react-native'; import _ from 'underscore'; import LogoComponent from '@assets/images/expensify-wordmark.svg'; @@ -145,6 +145,8 @@ function SidebarLinks({onLinkClick, insets, optionListItems, isLoading, priority ); const viewMode = priorityMode === CONST.PRIORITY_MODE.GSD ? CONST.OPTION_MODE.COMPACT : CONST.OPTION_MODE.DEFAULT; + const listStyle = useMemo(() => [isLoading ? styles.flexShrink1 : styles.flex1], [isLoading]); + const contentContainerStyles = useMemo(() => [styles.sidebarListContainer, {paddingBottom: StyleUtils.getSafeAreaMargins(insets).marginBottom}], [insets]); return ( @@ -177,8 +179,8 @@ function SidebarLinks({onLinkClick, insets, optionListItems, isLoading, priority Date: Wed, 15 Nov 2023 15:05:32 +0500 Subject: [PATCH 02/14] perf: add memoization --- src/components/OptionsList/BaseOptionsList.js | 22 ++++++++----------- src/components/OptionsList/index.js | 4 ++-- src/components/OptionsList/index.native.js | 6 ++--- 3 files changed, 14 insertions(+), 18 deletions(-) diff --git a/src/components/OptionsList/BaseOptionsList.js b/src/components/OptionsList/BaseOptionsList.js index e0acc2534fbf..cecf983ff989 100644 --- a/src/components/OptionsList/BaseOptionsList.js +++ b/src/components/OptionsList/BaseOptionsList.js @@ -1,5 +1,5 @@ import PropTypes from 'prop-types'; -import React, {forwardRef, memo, useEffect, useRef} from 'react'; +import React, {forwardRef, memo, useEffect, useMemo, useRef} from 'react'; import {View} from 'react-native'; import _ from 'underscore'; import OptionRow from '@components/OptionRow'; @@ -35,7 +35,7 @@ const defaultProps = { ...optionsListDefaultProps, }; -function BaseOptionsList({ +const BaseOptionsList = forwardRef(({ keyboardDismissMode, onScrollBeginDrag, onScroll, @@ -65,16 +65,18 @@ function BaseOptionsList({ onSelectRow, boldStyle, isDisabled, - innerRef, isRowMultilineSupported, isLoadingNewOptions, nestedScrollEnabled, bounces, -}) { + safeAreaPaddingBottomStyle, +}, innerRef) => { const flattenedData = useRef(); const previousSections = usePrevious(sections); const didLayout = useRef(false); + const listContentContainerStyle = useMemo(() => [contentContainerStyles, safeAreaPaddingBottomStyle], [contentContainerStyles, safeAreaPaddingBottomStyle]) + /** * This helper function is used to memoize the computation needed for getItemLayout. It is run whenever section data changes. * @@ -270,7 +272,7 @@ function BaseOptionsList({ scrollEnabled={nestedScrollEnabled} onScrollBeginDrag={onScrollBeginDrag} onScroll={onScroll} - contentContainerStyle={contentContainerStyles} + contentContainerStyle={listContentContainerStyle} showsVerticalScrollIndicator={showScrollIndicator} sections={sections} keyExtractor={extractKey} @@ -290,7 +292,7 @@ function BaseOptionsList({ )} ); -} +}); BaseOptionsList.propTypes = propTypes; BaseOptionsList.defaultProps = defaultProps; @@ -298,13 +300,7 @@ BaseOptionsList.displayName = 'BaseOptionsList'; // using memo to avoid unnecessary rerenders when parents component rerenders (thus causing this component to rerender because shallow comparison is used for some props). export default memo( - forwardRef((props, ref) => ( - - )), + BaseOptionsList, (prevProps, nextProps) => nextProps.focusedIndex === prevProps.focusedIndex && nextProps.selectedOptions.length === prevProps.selectedOptions.length && diff --git a/src/components/OptionsList/index.js b/src/components/OptionsList/index.js index 36b8e7fccf12..6046a6124ccc 100644 --- a/src/components/OptionsList/index.js +++ b/src/components/OptionsList/index.js @@ -1,4 +1,4 @@ -import React, {forwardRef, useCallback, useEffect, useRef} from 'react'; +import React, {forwardRef, memo, useCallback, useEffect, useRef} from 'react'; import {Keyboard} from 'react-native'; import _ from 'underscore'; import withWindowDimensions from '@components/withWindowDimensions'; @@ -64,4 +64,4 @@ const OptionsListWithRef = forwardRef((props, ref) => ( OptionsListWithRef.displayName = 'OptionsListWithRef'; -export default withWindowDimensions(OptionsListWithRef); +export default withWindowDimensions(memo(OptionsListWithRef)); diff --git a/src/components/OptionsList/index.native.js b/src/components/OptionsList/index.native.js index ab2db4f20967..8a70e1e060b1 100644 --- a/src/components/OptionsList/index.native.js +++ b/src/components/OptionsList/index.native.js @@ -1,4 +1,4 @@ -import React, {forwardRef} from 'react'; +import React, {forwardRef, memo} from 'react'; import {Keyboard} from 'react-native'; import BaseOptionsList from './BaseOptionsList'; import {defaultProps, propTypes} from './optionsListPropTypes'; @@ -8,7 +8,7 @@ const OptionsList = forwardRef((props, ref) => ( // eslint-disable-next-line react/jsx-props-no-spreading {...props} ref={ref} - onScrollBeginDrag={() => Keyboard.dismiss()} + onScrollBeginDrag={Keyboard.dismiss} /> )); @@ -16,4 +16,4 @@ OptionsList.propTypes = propTypes; OptionsList.defaultProps = defaultProps; OptionsList.displayName = 'OptionsList'; -export default OptionsList; +export default memo(OptionsList); From bbb216dd728eb135709a7892b793247c5cdb820d Mon Sep 17 00:00:00 2001 From: hurali97 Date: Wed, 15 Nov 2023 15:07:28 +0500 Subject: [PATCH 03/14] perf: add navigation listeners and remove inline functions --- .../OptionsSelector/BaseOptionsSelector.js | 109 ++++++++++-------- 1 file changed, 62 insertions(+), 47 deletions(-) diff --git a/src/components/OptionsSelector/BaseOptionsSelector.js b/src/components/OptionsSelector/BaseOptionsSelector.js index 8c480c27f20f..cdf2b83b6215 100755 --- a/src/components/OptionsSelector/BaseOptionsSelector.js +++ b/src/components/OptionsSelector/BaseOptionsSelector.js @@ -1,7 +1,7 @@ import lodashGet from 'lodash/get'; import PropTypes from 'prop-types'; import React, {Component} from 'react'; -import {ScrollView, View} from 'react-native'; +import {InteractionManager, ScrollView, View} from 'react-native'; import _ from 'underscore'; import ArrowKeyFocusManager from '@components/ArrowKeyFocusManager'; import Button from '@components/Button'; @@ -10,7 +10,7 @@ import FormHelpMessage from '@components/FormHelpMessage'; import OptionsList from '@components/OptionsList'; import TextInput from '@components/TextInput'; import withLocalize, {withLocalizePropTypes} from '@components/withLocalize'; -import withNavigationFocus from '@components/withNavigationFocus'; +import withNavigation from '@components/withNavigation'; import compose from '@libs/compose'; import getPlatform from '@libs/getPlatform'; import KeyboardShortcut from '@libs/KeyboardShortcut'; @@ -32,9 +32,6 @@ const propTypes = { /** List styles for OptionsList */ listStyles: PropTypes.oneOfType([PropTypes.arrayOf(PropTypes.object), PropTypes.object]), - /** Whether navigation is focused */ - isFocused: PropTypes.bool.isRequired, - ...optionsSelectorPropTypes, ...withLocalizePropTypes, }; @@ -58,49 +55,59 @@ class BaseOptionsSelector extends Component { this.selectFocusedOption = this.selectFocusedOption.bind(this); this.addToSelection = this.addToSelection.bind(this); this.updateSearchValue = this.updateSearchValue.bind(this); + this.onLayout = this.onLayout.bind(this); + this.setListRef = this.setListRef.bind(this); this.relatedTarget = null; - const allOptions = this.flattenSections(); - const focusedIndex = this.getInitiallyFocusedIndex(allOptions); - + this.focusListener = null; + this.blurListener = null; + this.isFocused = false; this.state = { - allOptions, - focusedIndex, + allOptions: [], + focusedIndex: 0, shouldDisableRowSelection: false, errorMessage: '', }; } componentDidMount() { - this.subscribeToKeyboardShortcut(); + this.focusListener = this.props.navigation.addListener('focus', () => { + this.subscribeToKeyboardShortcut(); + + // Screen coming back into focus, for example + // when doing Cmd+Shift+K, then Cmd+K, then Cmd+Shift+K. + // Only applies to platforms that support keyboard shortcuts + if ([CONST.PLATFORM.DESKTOP, CONST.PLATFORM.WEB].includes(getPlatform()) && this.props.autoFocus && this.textInput) { + this.focusTimeout = setTimeout(() => { + this.textInput.focus(); + }, CONST.ANIMATED_TRANSITION); + } - if (this.props.isFocused && this.props.autoFocus && this.textInput) { - this.focusTimeout = setTimeout(() => { - this.textInput.focus(); - }, CONST.ANIMATED_TRANSITION); - } + this.isFocused = true; + }); + this.blurListener = this.props.navigation.addListener('blur', () => { + this.unSubscribeFromKeyboardShortcut(); + this.isFocused = false; + }); this.scrollToIndex(this.props.selectedOptions.length ? 0 : this.state.focusedIndex, false); + + /** + * Execute the following code after all interactions have been completed. + * Which means once we are sure that all navigation animations are done, + * we will execute the callback passed to `runAfterInteractions`. + */ + this.interactionTask = InteractionManager.runAfterInteractions(() => { + const allOptions = this.flattenSections(); + const focusedIndex = this.getInitiallyFocusedIndex(allOptions); + this.setState({ + allOptions, + focusedIndex, + }); + }); } componentDidUpdate(prevProps) { - if (prevProps.isFocused !== this.props.isFocused) { - if (this.props.isFocused) { - this.subscribeToKeyboardShortcut(); - } else { - this.unSubscribeFromKeyboardShortcut(); - } - } - - // Screen coming back into focus, for example - // when doing Cmd+Shift+K, then Cmd+K, then Cmd+Shift+K. - // Only applies to platforms that support keyboard shortcuts - if ([CONST.PLATFORM.DESKTOP, CONST.PLATFORM.WEB].includes(getPlatform()) && !prevProps.isFocused && this.props.isFocused && this.props.autoFocus && this.textInput) { - setTimeout(() => { - this.textInput.focus(); - }, CONST.ANIMATED_TRANSITION); - } - if (_.isEqual(this.props.sections, prevProps.sections)) { return; } @@ -139,11 +146,22 @@ class BaseOptionsSelector extends Component { } componentWillUnmount() { + this.interactionTask.cancel(); + this.focusListener(); + this.blurListener(); if (this.focusTimeout) { clearTimeout(this.focusTimeout); } + } - this.unSubscribeFromKeyboardShortcut(); + onLayout() { + if (this.props.selectedOptions.length === 0) { + this.scrollToIndex(this.state.focusedIndex, false); + } + + if (this.props.onLayout) { + this.props.onLayout(); + } } /** @@ -172,6 +190,10 @@ class BaseOptionsSelector extends Component { return defaultIndex; } + setListRef(ref) { + this.list = ref; + } + updateSearchValue(value) { this.setState({ errorMessage: value.length > this.props.maxLength ? this.props.translate('common.error.characterLimitExceedCounter', {length: value.length, limit: this.props.maxLength}) : '', @@ -226,7 +248,7 @@ class BaseOptionsSelector extends Component { selectFocusedOption() { const focusedOption = this.state.allOptions[this.state.focusedIndex]; - if (!focusedOption || !this.props.isFocused) { + if (!focusedOption || !this.isFocused) { return; } @@ -400,7 +422,7 @@ class BaseOptionsSelector extends Component { ); const optionsList = ( (this.list = el)} + ref={this.setListRef} optionHoveredStyle={this.props.optionHoveredStyle} onSelectRow={this.props.onSelectRow ? this.selectRow : undefined} sections={this.props.sections} @@ -417,16 +439,9 @@ class BaseOptionsSelector extends Component { isDisabled={this.props.isDisabled} shouldHaveOptionSeparator={this.props.shouldHaveOptionSeparator} highlightSelectedOptions={this.props.highlightSelectedOptions} - onLayout={() => { - if (this.props.selectedOptions.length === 0) { - this.scrollToIndex(this.state.focusedIndex, false); - } - - if (this.props.onLayout) { - this.props.onLayout(); - } - }} - contentContainerStyles={[safeAreaPaddingBottomStyle, ...this.props.contentContainerStyles]} + onLayout={this.onLayout} + safeAreaPaddingBottomStyle={safeAreaPaddingBottomStyle} + contentContainerStyles={this.props.contentContainerStyles} sectionHeaderStyle={this.props.sectionHeaderStyle} listContainerStyles={this.props.listContainerStyles} listStyles={this.props.listStyles} @@ -518,4 +533,4 @@ class BaseOptionsSelector extends Component { BaseOptionsSelector.defaultProps = defaultProps; BaseOptionsSelector.propTypes = propTypes; -export default compose(withLocalize, withNavigationFocus)(BaseOptionsSelector); +export default compose(withLocalize, withNavigation)(BaseOptionsSelector); From 15147c6e7c6a09ca0e54a3c3351cfbebe9e18d6c Mon Sep 17 00:00:00 2001 From: hurali97 Date: Wed, 15 Nov 2023 15:09:45 +0500 Subject: [PATCH 04/14] refactor: use personalDetails from utils and add Interaction Manager --- src/libs/PersonalDetailsUtils.js | 11 ++++++- src/pages/SearchPage.js | 49 ++++++++++++++++++-------------- 2 files changed, 38 insertions(+), 22 deletions(-) diff --git a/src/libs/PersonalDetailsUtils.js b/src/libs/PersonalDetailsUtils.js index c99adc32a56a..3a1038700537 100644 --- a/src/libs/PersonalDetailsUtils.js +++ b/src/libs/PersonalDetailsUtils.js @@ -177,4 +177,13 @@ function getFormattedAddress(privatePersonalDetails) { return formattedAddress.trim().replace(/,$/, ''); } -export {getDisplayNameOrDefault, getPersonalDetailsByIDs, getAccountIDsByLogins, getLoginsByAccountIDs, getNewPersonalDetailsOnyxData, getFormattedAddress}; +/** + * get personal details + * + * @returns {Object} + */ +function getPersonalDetails() { + return allPersonalDetails || {}; +} + +export {getPersonalDetails, getDisplayNameOrDefault, getPersonalDetailsByIDs, getAccountIDsByLogins, getLoginsByAccountIDs, getNewPersonalDetailsOnyxData, getFormattedAddress}; diff --git a/src/pages/SearchPage.js b/src/pages/SearchPage.js index 3e7731efc7b2..7d9f9818c309 100755 --- a/src/pages/SearchPage.js +++ b/src/pages/SearchPage.js @@ -1,6 +1,6 @@ import PropTypes from 'prop-types'; import React, {Component} from 'react'; -import {View} from 'react-native'; +import {InteractionManager, View} from 'react-native'; import {withOnyx} from 'react-native-onyx'; import _ from 'underscore'; import HeaderWithBackButton from '@components/HeaderWithBackButton'; @@ -14,13 +14,13 @@ import compose from '@libs/compose'; import Navigation from '@libs/Navigation/Navigation'; import * as OptionsListUtils from '@libs/OptionsListUtils'; import Performance from '@libs/Performance'; +import * as PersonalDetailsUtils from '@libs/PersonalDetailsUtils'; import * as ReportUtils from '@libs/ReportUtils'; import styles from '@styles/styles'; import * as Report from '@userActions/Report'; import Timing from '@userActions/Timing'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; -import personalDetailsPropType from './personalDetailsPropType'; import reportPropTypes from './reportPropTypes'; const propTypes = { @@ -29,9 +29,6 @@ const propTypes = { /** Beta features list */ betas: PropTypes.arrayOf(PropTypes.string), - /** All of the personal details for everyone */ - personalDetails: PropTypes.objectOf(personalDetailsPropType), - /** All reports shared with the user */ reports: PropTypes.objectOf(reportPropTypes), @@ -49,7 +46,6 @@ const propTypes = { const defaultProps = { betas: [], - personalDetails: {}, reports: {}, network: {}, isSearchingForReports: false, @@ -76,12 +72,16 @@ class SearchPage extends Component { } componentDidUpdate(prevProps) { - if (_.isEqual(prevProps.reports, this.props.reports) && _.isEqual(prevProps.personalDetails, this.props.personalDetails)) { + if (_.isEqual(prevProps.reports, this.props.reports)) { return; } this.updateOptions(); } + componentWillUnmount() { + this.interactionTask.cancel(); + } + onChangeText(searchValue = '') { if (searchValue.length) { Report.searchInServer(searchValue); @@ -134,16 +134,26 @@ class SearchPage extends Component { } updateOptions() { - const {recentReports, personalDetails, userToInvite} = OptionsListUtils.getSearchOptions( - this.props.reports, - this.props.personalDetails, - this.state.searchValue.trim(), - this.props.betas, - ); - this.setState({ - userToInvite, - recentReports, - personalDetails, + if (this.interactionTask) { + this.interactionTask.cancel(); + } + + /** + * Execute the callback after all interactions are done, which means + * after all animations have finished. + */ + this.interactionTask = InteractionManager.runAfterInteractions(() => { + const {recentReports, personalDetails, userToInvite} = OptionsListUtils.getSearchOptions( + this.props.reports, + PersonalDetailsUtils.getPersonalDetails(), + this.state.searchValue.trim(), + this.props.betas, + ); + this.setState({ + userToInvite, + recentReports, + personalDetails, + }); }); } @@ -173,7 +183,7 @@ class SearchPage extends Component { render() { const sections = this.getSections(); - const isOptionsDataReady = ReportUtils.isReportDataReady() && OptionsListUtils.isPersonalDetailsReady(this.props.personalDetails); + const isOptionsDataReady = ReportUtils.isReportDataReady() && OptionsListUtils.isPersonalDetailsReady(this.state.personalDetails); const headerMessage = OptionsListUtils.getHeaderMessage( this.state.recentReports.length + this.state.personalDetails.length !== 0, Boolean(this.state.userToInvite), @@ -228,9 +238,6 @@ export default compose( reports: { key: ONYXKEYS.COLLECTION.REPORT, }, - personalDetails: { - key: ONYXKEYS.PERSONAL_DETAILS_LIST, - }, betas: { key: ONYXKEYS.BETAS, }, From 15e62f92aa9eb07d2ed3ac9ff9b71717395fca47 Mon Sep 17 00:00:00 2001 From: hurali97 Date: Wed, 15 Nov 2023 17:31:22 +0500 Subject: [PATCH 05/14] fix: linting --- src/components/OptionsList/BaseOptionsList.js | 489 +++++++++--------- 1 file changed, 247 insertions(+), 242 deletions(-) diff --git a/src/components/OptionsList/BaseOptionsList.js b/src/components/OptionsList/BaseOptionsList.js index cecf983ff989..d303c6f58073 100644 --- a/src/components/OptionsList/BaseOptionsList.js +++ b/src/components/OptionsList/BaseOptionsList.js @@ -35,264 +35,269 @@ const defaultProps = { ...optionsListDefaultProps, }; -const BaseOptionsList = forwardRef(({ - keyboardDismissMode, - onScrollBeginDrag, - onScroll, - listStyles, - focusedIndex, - selectedOptions, - headerMessage, - isLoading, - sections, - onLayout, - hideSectionHeaders, - shouldHaveOptionSeparator, - showTitleTooltip, - optionHoveredStyle, - contentContainerStyles, - sectionHeaderStyle, - showScrollIndicator, - listContainerStyles, - shouldDisableRowInnerPadding, - shouldPreventDefaultFocusOnSelectRow, - disableFocusOptions, - canSelectMultipleOptions, - shouldShowMultipleOptionSelectorAsButton, - multipleOptionSelectorButtonText, - onAddToSelection, - highlightSelectedOptions, - onSelectRow, - boldStyle, - isDisabled, - isRowMultilineSupported, - isLoadingNewOptions, - nestedScrollEnabled, - bounces, - safeAreaPaddingBottomStyle, -}, innerRef) => { - const flattenedData = useRef(); - const previousSections = usePrevious(sections); - const didLayout = useRef(false); - - const listContentContainerStyle = useMemo(() => [contentContainerStyles, safeAreaPaddingBottomStyle], [contentContainerStyles, safeAreaPaddingBottomStyle]) - - /** - * This helper function is used to memoize the computation needed for getItemLayout. It is run whenever section data changes. - * - * @returns {Array} - */ - const buildFlatSectionArray = () => { - let offset = 0; - - // Start with just an empty list header - const flatArray = [{length: 0, offset}]; - - // Build the flat array - for (let sectionIndex = 0; sectionIndex < sections.length; sectionIndex++) { - const section = sections[sectionIndex]; - - // Add the section header - const sectionHeaderHeight = section.title && !hideSectionHeaders ? variables.optionsListSectionHeaderHeight : 0; - flatArray.push({length: sectionHeaderHeight, offset}); - offset += sectionHeaderHeight; - - // Add section items - for (let i = 0; i < section.data.length; i++) { - let fullOptionHeight = variables.optionRowHeight; - if (i > 0 && shouldHaveOptionSeparator) { - fullOptionHeight += variables.borderTopWidth; +const BaseOptionsList = forwardRef( + ( + { + keyboardDismissMode, + onScrollBeginDrag, + onScroll, + listStyles, + focusedIndex, + selectedOptions, + headerMessage, + isLoading, + sections, + onLayout, + hideSectionHeaders, + shouldHaveOptionSeparator, + showTitleTooltip, + optionHoveredStyle, + contentContainerStyles, + sectionHeaderStyle, + showScrollIndicator, + listContainerStyles, + shouldDisableRowInnerPadding, + shouldPreventDefaultFocusOnSelectRow, + disableFocusOptions, + canSelectMultipleOptions, + shouldShowMultipleOptionSelectorAsButton, + multipleOptionSelectorButtonText, + onAddToSelection, + highlightSelectedOptions, + onSelectRow, + boldStyle, + isDisabled, + isRowMultilineSupported, + isLoadingNewOptions, + nestedScrollEnabled, + bounces, + safeAreaPaddingBottomStyle, + }, + innerRef, + ) => { + const flattenedData = useRef(); + const previousSections = usePrevious(sections); + const didLayout = useRef(false); + + const listContentContainerStyle = useMemo(() => [contentContainerStyles, safeAreaPaddingBottomStyle], [contentContainerStyles, safeAreaPaddingBottomStyle]); + + /** + * This helper function is used to memoize the computation needed for getItemLayout. It is run whenever section data changes. + * + * @returns {Array} + */ + const buildFlatSectionArray = () => { + let offset = 0; + + // Start with just an empty list header + const flatArray = [{length: 0, offset}]; + + // Build the flat array + for (let sectionIndex = 0; sectionIndex < sections.length; sectionIndex++) { + const section = sections[sectionIndex]; + + // Add the section header + const sectionHeaderHeight = section.title && !hideSectionHeaders ? variables.optionsListSectionHeaderHeight : 0; + flatArray.push({length: sectionHeaderHeight, offset}); + offset += sectionHeaderHeight; + + // Add section items + for (let i = 0; i < section.data.length; i++) { + let fullOptionHeight = variables.optionRowHeight; + if (i > 0 && shouldHaveOptionSeparator) { + fullOptionHeight += variables.borderTopWidth; + } + flatArray.push({length: fullOptionHeight, offset}); + offset += fullOptionHeight; } - flatArray.push({length: fullOptionHeight, offset}); - offset += fullOptionHeight; + + // Add the section footer + flatArray.push({length: 0, offset}); } - // Add the section footer + // Then add the list footer flatArray.push({length: 0, offset}); - } - - // Then add the list footer - flatArray.push({length: 0, offset}); - return flatArray; - }; - - useEffect(() => { - if (_.isEqual(sections, previousSections)) { - return; - } - flattenedData.current = buildFlatSectionArray(); - }); - - const onViewableItemsChanged = () => { - if (didLayout.current || !onLayout) { - return; - } - - didLayout.current = true; - onLayout(); - }; - - /** - * This function is used to compute the layout of any given item in our list. - * We need to implement it so that we can programmatically scroll to items outside the virtual render window of the SectionList. - * - * @param {Array} data - This is the same as the data we pass into the component - * @param {Number} flatDataArrayIndex - This index is provided by React Native, and refers to a flat array with data from all the sections. This flat array has some quirks: - * - * 1. It ALWAYS includes a list header and a list footer, even if we don't provide/render those. - * 2. Each section includes a header, even if we don't provide/render one. - * - * For example, given a list with two sections, two items in each section, no header, no footer, and no section headers, the flat array might look something like this: - * - * [{header}, {sectionHeader}, {item}, {item}, {sectionHeader}, {item}, {item}, {footer}] - * - * @returns {Object} - */ - const getItemLayout = (data, flatDataArrayIndex) => { - if (!_.has(flattenedData.current, flatDataArrayIndex)) { + return flatArray; + }; + + useEffect(() => { + if (_.isEqual(sections, previousSections)) { + return; + } flattenedData.current = buildFlatSectionArray(); - } + }); + + const onViewableItemsChanged = () => { + if (didLayout.current || !onLayout) { + return; + } - const targetItem = flattenedData.current[flatDataArrayIndex]; - return { - length: targetItem.length, - offset: targetItem.offset, - index: flatDataArrayIndex, + didLayout.current = true; + onLayout(); }; - }; - - /** - * Returns the key used by the list - * @param {Object} option - * @return {String} - */ - const extractKey = (option) => option.keyForList; - - /** - * Function which renders a row in the list - * - * @param {Object} params - * @param {Object} params.item - * @param {Number} params.index - * @param {Object} params.section - * - * @return {Component} - */ - const renderItem = ({item, index, section}) => { - const isItemDisabled = isDisabled || section.isDisabled || !!item.isDisabled; - const isSelected = _.some(selectedOptions, (option) => { - if (option.accountID && option.accountID === item.accountID) { - return true; + + /** + * This function is used to compute the layout of any given item in our list. + * We need to implement it so that we can programmatically scroll to items outside the virtual render window of the SectionList. + * + * @param {Array} data - This is the same as the data we pass into the component + * @param {Number} flatDataArrayIndex - This index is provided by React Native, and refers to a flat array with data from all the sections. This flat array has some quirks: + * + * 1. It ALWAYS includes a list header and a list footer, even if we don't provide/render those. + * 2. Each section includes a header, even if we don't provide/render one. + * + * For example, given a list with two sections, two items in each section, no header, no footer, and no section headers, the flat array might look something like this: + * + * [{header}, {sectionHeader}, {item}, {item}, {sectionHeader}, {item}, {item}, {footer}] + * + * @returns {Object} + */ + const getItemLayout = (data, flatDataArrayIndex) => { + if (!_.has(flattenedData.current, flatDataArrayIndex)) { + flattenedData.current = buildFlatSectionArray(); } - if (option.reportID && option.reportID === item.reportID) { - return true; + const targetItem = flattenedData.current[flatDataArrayIndex]; + return { + length: targetItem.length, + offset: targetItem.offset, + index: flatDataArrayIndex, + }; + }; + + /** + * Returns the key used by the list + * @param {Object} option + * @return {String} + */ + const extractKey = (option) => option.keyForList; + + /** + * Function which renders a row in the list + * + * @param {Object} params + * @param {Object} params.item + * @param {Number} params.index + * @param {Object} params.section + * + * @return {Component} + */ + const renderItem = ({item, index, section}) => { + const isItemDisabled = isDisabled || section.isDisabled || !!item.isDisabled; + const isSelected = _.some(selectedOptions, (option) => { + if (option.accountID && option.accountID === item.accountID) { + return true; + } + + if (option.reportID && option.reportID === item.reportID) { + return true; + } + + if (_.isEmpty(option.name)) { + return false; + } + + return option.name === item.searchText; + }); + + return ( + 0 && shouldHaveOptionSeparator} + shouldDisableRowInnerPadding={shouldDisableRowInnerPadding} + shouldPreventDefaultFocusOnSelectRow={shouldPreventDefaultFocusOnSelectRow} + isMultilineSupported={isRowMultilineSupported} + /> + ); + }; + + /** + * Function which renders a section header component + * + * @param {Object} params + * @param {Object} params.section + * @param {String} params.section.title + * @param {Boolean} params.section.shouldShow + * + * @return {Component} + */ + const renderSectionHeader = ({section: {title, shouldShow}}) => { + if (!title && shouldShow && !hideSectionHeaders && sectionHeaderStyle) { + return ; } - if (_.isEmpty(option.name)) { - return false; + if (title && shouldShow && !hideSectionHeaders) { + return ( + // Note: The `optionsListSectionHeader` style provides an explicit height to section headers. + // We do this so that we can reference the height in `getItemLayout` – + // we need to know the heights of all list items up-front in order to synchronously compute the layout of any given list item. + // So be aware that if you adjust the content of the section header (for example, change the font size), you may need to adjust this explicit height as well. + + {title} + + ); } - return option.name === item.searchText; - }); + return ; + }; return ( - 0 && shouldHaveOptionSeparator} - shouldDisableRowInnerPadding={shouldDisableRowInnerPadding} - shouldPreventDefaultFocusOnSelectRow={shouldPreventDefaultFocusOnSelectRow} - isMultilineSupported={isRowMultilineSupported} - /> + + {isLoading ? ( + + ) : ( + <> + {/* If we are loading new options we will avoid showing any header message. This is mostly because one of the header messages says there are no options. */} + {/* This is misleading because we might be in the process of loading fresh options from the server. */} + {!isLoadingNewOptions && headerMessage ? ( + + {headerMessage} + + ) : null} + + + )} + ); - }; - - /** - * Function which renders a section header component - * - * @param {Object} params - * @param {Object} params.section - * @param {String} params.section.title - * @param {Boolean} params.section.shouldShow - * - * @return {Component} - */ - const renderSectionHeader = ({section: {title, shouldShow}}) => { - if (!title && shouldShow && !hideSectionHeaders && sectionHeaderStyle) { - return ; - } - - if (title && shouldShow && !hideSectionHeaders) { - return ( - // Note: The `optionsListSectionHeader` style provides an explicit height to section headers. - // We do this so that we can reference the height in `getItemLayout` – - // we need to know the heights of all list items up-front in order to synchronously compute the layout of any given list item. - // So be aware that if you adjust the content of the section header (for example, change the font size), you may need to adjust this explicit height as well. - - {title} - - ); - } - - return ; - }; - - return ( - - {isLoading ? ( - - ) : ( - <> - {/* If we are loading new options we will avoid showing any header message. This is mostly because one of the header messages says there are no options. */} - {/* This is misleading because we might be in the process of loading fresh options from the server. */} - {!isLoadingNewOptions && headerMessage ? ( - - {headerMessage} - - ) : null} - - - )} - - ); -}); + }, +); BaseOptionsList.propTypes = propTypes; BaseOptionsList.defaultProps = defaultProps; From aab633b9bb74d783f42d96c30e2d0242567e5f01 Mon Sep 17 00:00:00 2001 From: hurali97 Date: Thu, 16 Nov 2023 12:14:34 +0500 Subject: [PATCH 06/14] refactor: focus text input --- .../OptionsSelector/BaseOptionsSelector.js | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/components/OptionsSelector/BaseOptionsSelector.js b/src/components/OptionsSelector/BaseOptionsSelector.js index cdf2b83b6215..682743ec7f01 100755 --- a/src/components/OptionsSelector/BaseOptionsSelector.js +++ b/src/components/OptionsSelector/BaseOptionsSelector.js @@ -72,12 +72,11 @@ class BaseOptionsSelector extends Component { componentDidMount() { this.focusListener = this.props.navigation.addListener('focus', () => { - this.subscribeToKeyboardShortcut(); + if ([CONST.PLATFORM.DESKTOP, CONST.PLATFORM.WEB].includes(getPlatform())) { + this.subscribeToKeyboardShortcut(); + } - // Screen coming back into focus, for example - // when doing Cmd+Shift+K, then Cmd+K, then Cmd+Shift+K. - // Only applies to platforms that support keyboard shortcuts - if ([CONST.PLATFORM.DESKTOP, CONST.PLATFORM.WEB].includes(getPlatform()) && this.props.autoFocus && this.textInput) { + if (this.props.autoFocus && this.textInput) { this.focusTimeout = setTimeout(() => { this.textInput.focus(); }, CONST.ANIMATED_TRANSITION); @@ -87,7 +86,9 @@ class BaseOptionsSelector extends Component { }); this.blurListener = this.props.navigation.addListener('blur', () => { - this.unSubscribeFromKeyboardShortcut(); + if ([CONST.PLATFORM.DESKTOP, CONST.PLATFORM.WEB].includes(getPlatform())) { + this.unSubscribeFromKeyboardShortcut(); + } this.isFocused = false; }); this.scrollToIndex(this.props.selectedOptions.length ? 0 : this.state.focusedIndex, false); From 0e2833010b3be51885059123e6275d6341b3d4d8 Mon Sep 17 00:00:00 2001 From: hurali97 Date: Tue, 28 Nov 2023 13:07:10 +0500 Subject: [PATCH 07/14] fix: skeleton being shown when typing --- src/pages/SearchPage.js | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/pages/SearchPage.js b/src/pages/SearchPage.js index a6323729a86d..6759b08060d7 100755 --- a/src/pages/SearchPage.js +++ b/src/pages/SearchPage.js @@ -52,6 +52,18 @@ const defaultProps = { isSearchingForReports: false, }; +function isSectionsEmpty(sections) { + if (!sections.length) { + return true; + } + + if (!sections[0].data.length) { + return true; + } + + return _.isEmpty(sections[0].data[0]); +} + class SearchPage extends Component { constructor(props) { super(props); @@ -184,7 +196,7 @@ class SearchPage extends Component { render() { const sections = this.getSections(); - const isOptionsDataReady = ReportUtils.isReportDataReady() && OptionsListUtils.isPersonalDetailsReady(this.state.personalDetails); + const isOptionsDataReady = ReportUtils.isReportDataReady() && OptionsListUtils.isPersonalDetailsReady(PersonalDetailsUtils.getPersonalDetails()); const headerMessage = OptionsListUtils.getHeaderMessage( this.state.recentReports.length + this.state.personalDetails.length !== 0, Boolean(this.state.userToInvite), @@ -209,7 +221,7 @@ class SearchPage extends Component { headerMessage={headerMessage} hideSectionHeaders showTitleTooltip - shouldShowOptions={didScreenTransitionEnd && isOptionsDataReady} + shouldShowOptions={didScreenTransitionEnd && isOptionsDataReady && !isSectionsEmpty(sections)} textInputLabel={this.props.translate('optionsSelector.nameEmailOrPhoneNumber')} textInputAlert={ this.props.network.isOffline ? `${this.props.translate('common.youAppearToBeOffline')} ${this.props.translate('search.resultsAreLimited')}` : '' From 44cc8fe2bdbe7bca7920d4ca032a891bdae9d8b7 Mon Sep 17 00:00:00 2001 From: hurali97 Date: Mon, 4 Dec 2023 16:48:38 +0500 Subject: [PATCH 08/14] refactor: remove unnecessary useMemo --- src/pages/home/sidebar/SidebarLinks.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index 52dbe879d218..09d6c1f2de62 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -148,8 +148,6 @@ function SidebarLinks({onLinkClick, insets, optionListItems, isLoading, priority const viewMode = priorityMode === CONST.PRIORITY_MODE.GSD ? CONST.OPTION_MODE.COMPACT : CONST.OPTION_MODE.DEFAULT; // eslint-disable-next-line react-hooks/exhaustive-deps - const listStyle = useMemo(() => [isLoading ? styles.flexShrink1 : styles.flex1], [isLoading]); - // eslint-disable-next-line react-hooks/exhaustive-deps const contentContainerStyles = useMemo(() => [styles.sidebarListContainer, {paddingBottom: StyleUtils.getSafeAreaMargins(insets).marginBottom}], [insets]); return ( @@ -183,7 +181,7 @@ function SidebarLinks({onLinkClick, insets, optionListItems, isLoading, priority Date: Wed, 6 Dec 2023 17:06:50 +0500 Subject: [PATCH 09/14] fix: personalDetails not updating --- src/libs/PersonalDetailsUtils.js | 10 ---------- src/pages/SearchPage.js | 15 +++++++++++---- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/src/libs/PersonalDetailsUtils.js b/src/libs/PersonalDetailsUtils.js index bbe4df529ade..560480dcec9d 100644 --- a/src/libs/PersonalDetailsUtils.js +++ b/src/libs/PersonalDetailsUtils.js @@ -197,17 +197,7 @@ function getFormattedAddress(privatePersonalDetails) { return formattedAddress.trim().replace(/,$/, ''); } -/** - * get personal details - * - * @returns {Object} - */ -function getPersonalDetails() { - return allPersonalDetails || {}; -} - export { - getPersonalDetails, getDisplayNameOrDefault, getPersonalDetailsByIDs, getAccountIDsByLogins, diff --git a/src/pages/SearchPage.js b/src/pages/SearchPage.js index 34afe5e3c3b5..5dfa3a0cacc4 100755 --- a/src/pages/SearchPage.js +++ b/src/pages/SearchPage.js @@ -15,12 +15,12 @@ import compose from '@libs/compose'; import Navigation from '@libs/Navigation/Navigation'; import * as OptionsListUtils from '@libs/OptionsListUtils'; import Performance from '@libs/Performance'; -import * as PersonalDetailsUtils from '@libs/PersonalDetailsUtils'; import * as ReportUtils from '@libs/ReportUtils'; import * as Report from '@userActions/Report'; import Timing from '@userActions/Timing'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; +import personalDetailsPropType from './personalDetailsPropType'; import reportPropTypes from './reportPropTypes'; const propTypes = { @@ -29,6 +29,9 @@ const propTypes = { /** Beta features list */ betas: PropTypes.arrayOf(PropTypes.string), + /** All of the personal details for everyone */ + personalDetails: PropTypes.objectOf(personalDetailsPropType), + /** All reports shared with the user */ reports: PropTypes.objectOf(reportPropTypes), @@ -47,6 +50,7 @@ const propTypes = { const defaultProps = { betas: [], + personalDetails: {}, reports: {}, network: {}, isSearchingForReports: false, @@ -85,7 +89,7 @@ class SearchPage extends Component { } componentDidUpdate(prevProps) { - if (_.isEqual(prevProps.reports, this.props.reports)) { + if (_.isEqual(prevProps.reports, this.props.reports) && _.isEqual(prevProps.personalDetails, this.props.personalDetails)) { return; } this.updateOptions(); @@ -155,7 +159,7 @@ class SearchPage extends Component { this.interactionTask = InteractionManager.runAfterInteractions(() => { const {recentReports, personalDetails, userToInvite} = OptionsListUtils.getSearchOptions( this.props.reports, - PersonalDetailsUtils.getPersonalDetails(), + this.props.personalDetails, this.state.searchValue.trim(), this.props.betas, ); @@ -193,7 +197,7 @@ class SearchPage extends Component { render() { const sections = this.getSections(); - const isOptionsDataReady = ReportUtils.isReportDataReady() && OptionsListUtils.isPersonalDetailsReady(PersonalDetailsUtils.getPersonalDetails()); + const isOptionsDataReady = ReportUtils.isReportDataReady() && OptionsListUtils.isPersonalDetailsReady(this.props.personalDetails); const headerMessage = OptionsListUtils.getHeaderMessage( this.state.recentReports.length + this.state.personalDetails.length !== 0, Boolean(this.state.userToInvite), @@ -257,6 +261,9 @@ export default compose( key: ONYXKEYS.IS_SEARCHING_FOR_REPORTS, initWithStoredValues: false, }, + personalDetails: { + key: ONYXKEYS.PERSONAL_DETAILS_LIST, + }, }), withThemeStyles, )(SearchPage); From 3f057546a3a808ecc77355ac452132d9b7899889 Mon Sep 17 00:00:00 2001 From: hurali97 Date: Fri, 22 Dec 2023 12:11:11 +0500 Subject: [PATCH 10/14] fix: skeleton not hiding when no result found for input --- src/pages/SearchPage.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/SearchPage.js b/src/pages/SearchPage.js index 69448f3a52e1..20d00fab662d 100755 --- a/src/pages/SearchPage.js +++ b/src/pages/SearchPage.js @@ -222,7 +222,7 @@ class SearchPage extends Component { headerMessage={headerMessage} hideSectionHeaders showTitleTooltip - shouldShowOptions={didScreenTransitionEnd && isOptionsDataReady && !isSectionsEmpty(sections)} + shouldShowOptions={didScreenTransitionEnd && isOptionsDataReady && (!isSectionsEmpty(sections) || this.state.searchValue.length)} textInputLabel={this.props.translate('optionsSelector.nameEmailOrPhoneNumber')} textInputAlert={ this.props.network.isOffline ? `${this.props.translate('common.youAppearToBeOffline')} ${this.props.translate('search.resultsAreLimited')}` : '' From 82e9c7ce918bfc20e96ab2dd7bbbfe3948398eaf Mon Sep 17 00:00:00 2001 From: hurali97 Date: Thu, 28 Dec 2023 16:47:24 +0500 Subject: [PATCH 11/14] fix: PR feedbacks --- src/components/OptionsList/BaseOptionsList.js | 2 +- src/components/OptionsSelector/BaseOptionsSelector.js | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/components/OptionsList/BaseOptionsList.js b/src/components/OptionsList/BaseOptionsList.js index 1d7faf9ae0f9..1699b68b2348 100644 --- a/src/components/OptionsList/BaseOptionsList.js +++ b/src/components/OptionsList/BaseOptionsList.js @@ -85,7 +85,7 @@ const BaseOptionsList = forwardRef( const didLayout = useRef(false); const listContainerStyles = useMemo(() => listContainerStylesProp || [styles.flex1], [listContainerStylesProp, styles.flex1]); - const contentContainerStyles = useMemo(() => [contentContainerStylesProp, safeAreaPaddingBottomStyle], [contentContainerStylesProp, safeAreaPaddingBottomStyle]); + const contentContainerStyles = useMemo(() => [safeAreaPaddingBottomStyle, ...contentContainerStylesProp], [contentContainerStylesProp, safeAreaPaddingBottomStyle]); /** * This helper function is used to memoize the computation needed for getItemLayout. It is run whenever section data changes. diff --git a/src/components/OptionsSelector/BaseOptionsSelector.js b/src/components/OptionsSelector/BaseOptionsSelector.js index 22e302448f29..b44c5375d720 100755 --- a/src/components/OptionsSelector/BaseOptionsSelector.js +++ b/src/components/OptionsSelector/BaseOptionsSelector.js @@ -100,6 +100,9 @@ class BaseOptionsSelector extends Component { componentDidMount() { this.focusListener = this.props.navigation.addListener('focus', () => { + // Screen coming back into focus, for example + // when doing Cmd+Shift+K, then Cmd+K, then Cmd+Shift+K. + // Only applies to platforms that support keyboard shortcuts if ([CONST.PLATFORM.DESKTOP, CONST.PLATFORM.WEB].includes(getPlatform())) { this.subscribeToKeyboardShortcut(); } @@ -188,7 +191,9 @@ class BaseOptionsSelector extends Component { } componentWillUnmount() { - this.interactionTask.cancel(); + if (this.interactionTask) { + this.interactionTask.cancel(); + } this.focusListener(); this.blurListener(); if (this.focusTimeout) { From 94d9a50b8a00b43a28ed9eb90f33cf38d423b6dc Mon Sep 17 00:00:00 2001 From: hurali97 Date: Thu, 28 Dec 2023 16:47:50 +0500 Subject: [PATCH 12/14] test: fix reassure test --- tests/perf-test/OptionsSelector.perf-test.js | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/tests/perf-test/OptionsSelector.perf-test.js b/tests/perf-test/OptionsSelector.perf-test.js index da706d7bb629..2152767b1df0 100644 --- a/tests/perf-test/OptionsSelector.perf-test.js +++ b/tests/perf-test/OptionsSelector.perf-test.js @@ -36,6 +36,17 @@ jest.mock('../../src/components/withNavigationFocus', () => (Component) => { return WithNavigationFocus; }); +jest.mock('@react-navigation/native', () => { + const actualNav = jest.requireActual('@react-navigation/native'); + return { + ...actualNav, + useNavigation: () => ({ + navigate: jest.fn(), + addListener: () => jest.fn(), + }), + }; +}); + const generateSections = (sectionConfigs) => _.map(sectionConfigs, ({numItems, indexOffset, shouldShow = true}) => ({ data: Array.from({length: numItems}, (_v, i) => ({ @@ -118,10 +129,10 @@ test('[OptionsSelector] should scroll and press few items', () => { const eventData = generateEventData(100, variables.optionRowHeight); const eventData2 = generateEventData(200, variables.optionRowHeight); - const scenario = (screen) => { + const scenario = async (screen) => { fireEvent.press(screen.getByText('Item 10')); fireEvent.scroll(screen.getByTestId('options-list'), eventData); - fireEvent.press(screen.getByText('Item 100')); + fireEvent.press(await screen.findByText('Item 100')); fireEvent.scroll(screen.getByTestId('options-list'), eventData2); fireEvent.press(screen.getByText('Item 200')); }; From 56f5f4948fed0e6d57a6aa72a224de550a3414ea Mon Sep 17 00:00:00 2001 From: hurali97 Date: Thu, 28 Dec 2023 19:58:54 +0500 Subject: [PATCH 13/14] test: mock withNavigation for options selector reassure test --- tests/perf-test/OptionsSelector.perf-test.js | 25 +++++++------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/tests/perf-test/OptionsSelector.perf-test.js b/tests/perf-test/OptionsSelector.perf-test.js index 2152767b1df0..557a0baf1ba4 100644 --- a/tests/perf-test/OptionsSelector.perf-test.js +++ b/tests/perf-test/OptionsSelector.perf-test.js @@ -20,31 +20,22 @@ jest.mock('../../src/components/withLocalize', () => (Component) => { return WrappedComponent; }); -jest.mock('../../src/components/withNavigationFocus', () => (Component) => { - function WithNavigationFocus(props) { +jest.mock('../../src/components/withNavigation', () => (Component) => { + function withNavigation(props) { return ( jest.fn(), + }} /> ); } - WithNavigationFocus.displayName = 'WithNavigationFocus'; - - return WithNavigationFocus; -}); - -jest.mock('@react-navigation/native', () => { - const actualNav = jest.requireActual('@react-navigation/native'); - return { - ...actualNav, - useNavigation: () => ({ - navigate: jest.fn(), - addListener: () => jest.fn(), - }), - }; + withNavigation.displayName = 'withNavigation'; + return withNavigation; }); const generateSections = (sectionConfigs) => From c4859f4996667471218286632fa32d1ef3a19fa1 Mon Sep 17 00:00:00 2001 From: hurali97 Date: Fri, 29 Dec 2023 14:16:58 +0500 Subject: [PATCH 14/14] fix: linting --- src/components/OptionsList/BaseOptionsList.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/components/OptionsList/BaseOptionsList.js b/src/components/OptionsList/BaseOptionsList.js index 1699b68b2348..8b24066af969 100644 --- a/src/components/OptionsList/BaseOptionsList.js +++ b/src/components/OptionsList/BaseOptionsList.js @@ -101,6 +101,7 @@ const BaseOptionsList = forwardRef( // Build the flat array for (let sectionIndex = 0; sectionIndex < sections.length; sectionIndex++) { const section = sections[sectionIndex]; + // Add the section header const sectionHeaderHeight = section.title && !hideSectionHeaders ? variables.optionsListSectionHeaderHeight : 0; flatArray.push({length: sectionHeaderHeight, offset}); @@ -172,6 +173,7 @@ const BaseOptionsList = forwardRef( /** * Returns the key used by the list + * * @param {Object} option * @return {String} */