Skip to content

Commit

Permalink
Merge pull request #33297 from Expensify/revert-32962-perf/debounce-s…
Browse files Browse the repository at this point in the history
…earch

[CP Staging] Revert "(perf) Debounce search in OptionList"
  • Loading branch information
pecanoro authored Dec 19, 2023
2 parents b3ecf32 + 0b0e664 commit 777a52f
Show file tree
Hide file tree
Showing 15 changed files with 84 additions and 66 deletions.
2 changes: 1 addition & 1 deletion src/CONST.ts
Original file line number Diff line number Diff line change
Expand Up @@ -710,7 +710,7 @@ const CONST = {
TOOLTIP_SENSE: 1000,
TRIE_INITIALIZATION: 'trie_initialization',
COMMENT_LENGTH_DEBOUNCE_TIME: 500,
SEARCH_OPTION_LIST_DEBOUNCE_TIME: 300,
SEARCH_FOR_REPORTS_DEBOUNCE_TIME: 300,
},
PRIORITY_MODE: {
GSD: 'gsd',
Expand Down
1 change: 1 addition & 0 deletions src/components/CategoryPicker/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ function CategoryPicker({selectedCategory, policyCategories, policyRecentlyUsedC
sectionHeaderStyle={styles.mt5}
sections={sections}
selectedOptions={selectedOptions}
value={searchValue}
// Focus the first option when searching
focusedIndex={0}
// Focus the selected option on first load
Expand Down
1 change: 1 addition & 0 deletions src/components/MoneyRequestConfirmationList.js
Original file line number Diff line number Diff line change
Expand Up @@ -565,6 +565,7 @@ function MoneyRequestConfirmationList(props) {
return (
<OptionsSelector
sections={optionSelectorSections}
value=""
onSelectRow={canModifyParticipants ? selectParticipant : navigateToReportOrUserDetail}
onAddToSelection={selectParticipant}
onConfirmSelection={confirm}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -565,6 +565,7 @@ function MoneyTemporaryForRefactorRequestConfirmationList({
return (
<OptionsSelector
sections={optionSelectorSections}
value=""
onSelectRow={userCanModifyParticipants.current ? selectParticipant : navigateToReportOrUserDetail}
onAddToSelection={selectParticipant}
onConfirmSelection={confirm}
Expand Down
2 changes: 1 addition & 1 deletion src/components/OptionRow.js
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ export default React.memo(
prevProps.showSelectedState === nextProps.showSelectedState &&
prevProps.highlightSelected === nextProps.highlightSelected &&
prevProps.showTitleTooltip === nextProps.showTitleTooltip &&
_.isEqual(prevProps.option.icons, nextProps.option.icons) &&
!_.isEqual(prevProps.option.icons, nextProps.option.icons) &&
prevProps.optionIsFocused === nextProps.optionIsFocused &&
prevProps.option.text === nextProps.option.text &&
prevProps.option.alternateText === nextProps.option.alternateText &&
Expand Down
12 changes: 5 additions & 7 deletions src/components/OptionsSelector/BaseOptionsSelector.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ class BaseOptionsSelector extends Component {
this.incrementPage = this.incrementPage.bind(this);
this.sliceSections = this.sliceSections.bind(this);
this.calculateAllVisibleOptionsCount = this.calculateAllVisibleOptionsCount.bind(this);
this.debouncedUpdateSearchValue = _.debounce(this.updateSearchValue, CONST.TIMING.SEARCH_OPTION_LIST_DEBOUNCE_TIME);
this.relatedTarget = null;

const allOptions = this.flattenSections();
Expand All @@ -95,7 +94,6 @@ class BaseOptionsSelector extends Component {
shouldShowReferralModal: false,
errorMessage: '',
paginationPage: 1,
value: '',
};
}

Expand Down Expand Up @@ -163,7 +161,7 @@ class BaseOptionsSelector extends Component {
},
() => {
// If we just toggled an option on a multi-selection page or cleared the search input, scroll to top
if (this.props.selectedOptions.length !== prevProps.selectedOptions.length || (!!prevState.value && !this.state.value)) {
if (this.props.selectedOptions.length !== prevProps.selectedOptions.length || (!!prevProps.value && !this.props.value)) {
this.scrollToIndex(0);
return;
}
Expand Down Expand Up @@ -249,7 +247,6 @@ class BaseOptionsSelector extends Component {
this.setState({
paginationPage: 1,
errorMessage: value.length > this.props.maxLength ? this.props.translate('common.error.characterLimitExceedCounter', {length: value.length, limit: this.props.maxLength}) : '',
value,
});

this.props.onChangeText(value);
Expand Down Expand Up @@ -418,7 +415,7 @@ class BaseOptionsSelector extends Component {
this.relatedTarget = null;
}
if (this.textInput.isFocused()) {
setSelection(this.textInput, 0, this.state.value.length);
setSelection(this.textInput, 0, this.props.value.length);
}
}
const selectedOption = this.props.onSelectRow(option);
Expand All @@ -443,7 +440,7 @@ class BaseOptionsSelector extends Component {
if (this.props.shouldShowTextInput && this.props.shouldPreventDefaultFocusOnSelectRow) {
this.textInput.focus();
if (this.textInput.isFocused()) {
setSelection(this.textInput, 0, this.state.value.length);
setSelection(this.textInput, 0, this.props.value.length);
}
}
this.props.onAddToSelection(option);
Expand Down Expand Up @@ -471,10 +468,11 @@ class BaseOptionsSelector extends Component {
const textInput = (
<TextInput
ref={(el) => (this.textInput = el)}
value={this.props.value}
label={this.props.textInputLabel}
accessibilityLabel={this.props.textInputLabel}
role={CONST.ROLE.PRESENTATION}
onChangeText={this.debouncedUpdateSearchValue}
onChangeText={this.updateSearchValue}
errorText={this.state.errorMessage}
onSubmitEditing={this.selectFocusedOption}
placeholder={this.props.placeholderText}
Expand Down
3 changes: 3 additions & 0 deletions src/components/OptionsSelector/optionsSelectorPropTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ const propTypes = {
}),
).isRequired,

/** Value in the search input field */
value: PropTypes.string.isRequired,

/** Callback fired when text changes */
onChangeText: PropTypes.func,

Expand Down
1 change: 1 addition & 0 deletions src/components/TagPicker/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ function TagPicker({selectedTag, tag, policyTags, policyRecentlyUsedTags, should
highlightSelectedOptions
isRowMultilineSupported
shouldShowTextInput={shouldShowTextInput}
value={searchValue}
// Focus the first option when searching
focusedIndex={0}
// Focus the selected option on first load
Expand Down
5 changes: 4 additions & 1 deletion src/libs/actions/Report.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {format as timezoneFormat, utcToZonedTime} from 'date-fns-tz';
import ExpensiMark from 'expensify-common/lib/ExpensiMark';
import Str from 'expensify-common/lib/str';
import lodashDebounce from 'lodash/debounce';
import isEmpty from 'lodash/isEmpty';
import {DeviceEventEmitter, InteractionManager} from 'react-native';
import Onyx, {OnyxCollection, OnyxEntry, OnyxUpdate} from 'react-native-onyx';
Expand Down Expand Up @@ -2498,6 +2499,8 @@ function searchForReports(searchInput: string) {
API.read('SearchForReports', parameters, {successData, failureData});
}

const debouncedSearchInServer = lodashDebounce(searchForReports, CONST.TIMING.SEARCH_FOR_REPORTS_DEBOUNCE_TIME, {leading: false});

function searchInServer(searchInput: string) {
if (isNetworkOffline || !searchInput.trim().length) {
Onyx.set(ONYXKEYS.IS_SEARCHING_FOR_REPORTS, false);
Expand All @@ -2508,7 +2511,7 @@ function searchInServer(searchInput: string) {
// we want to show the loading state right away. Otherwise, we will see a flashing UI where the client options are sorted and
// tell the user there are no options, then we start searching, and tell them there are no options again.
Onyx.set(ONYXKEYS.IS_SEARCHING_FOR_REPORTS, true);
searchForReports(searchInput);
debouncedSearchInServer(searchInput);
}

function clearNewRoomFormError() {
Expand Down
1 change: 1 addition & 0 deletions src/pages/NewChatPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,7 @@ function NewChatPage({betas, isGroupChat, personalDetails, reports, translate, i
onAddToSelection={(option) => toggleOption(option)}
sections={sections}
selectedOptions={selectedOptions}
value={searchTerm}
onSelectRow={(option) => createChat(option)}
onChangeText={setSearchTermAndSearchInServer}
headerMessage={headerMessage}
Expand Down
13 changes: 11 additions & 2 deletions src/pages/SearchPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ class SearchPage extends Component {
this.selectReport = this.selectReport.bind(this);
this.onChangeText = this.onChangeText.bind(this);
this.updateOptions = this.updateOptions.bind(this);
this.debouncedUpdateOptions = _.debounce(this.updateOptions.bind(this), 75);
this.state = {
searchValue: '',
recentReports: {},
Expand All @@ -84,7 +85,7 @@ class SearchPage extends Component {

onChangeText(searchValue = '') {
Report.searchInServer(searchValue);
this.setState({searchValue}, this.updateOptions);
this.setState({searchValue}, this.debouncedUpdateOptions);
}

/**
Expand Down Expand Up @@ -155,7 +156,14 @@ class SearchPage extends Component {
}

if (option.reportID) {
Navigation.dismissModal(option.reportID);
this.setState(
{
searchValue: '',
},
() => {
Navigation.dismissModal(option.reportID);
},
);
} else {
Report.navigateToAndOpenReport([option.login]);
}
Expand All @@ -182,6 +190,7 @@ class SearchPage extends Component {
<View style={[this.props.themeStyles.flex1, this.props.themeStyles.w100, this.props.themeStyles.pRelative]}>
<OptionsSelector
sections={sections}
value={this.state.searchValue}
onSelectRow={this.selectReport}
onChangeText={this.onChangeText}
headerMessage={headerMessage}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,7 @@ function MoneyTemporaryForRefactorRequestParticipantsSelector({
onAddToSelection={addParticipantToSelection}
sections={sections}
selectedOptions={participants}
value={searchTerm}
onSelectRow={addSingleParticipant}
onChangeText={setSearchTermAndSearchInServer}
ref={forwardedRef}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import {withOnyx} from 'react-native-onyx';
import _ from 'underscore';
import Button from '@components/Button';
import FormHelpMessage from '@components/FormHelpMessage';
import {usePersonalDetails} from '@components/OnyxProvider';
import OptionsSelector from '@components/OptionsSelector';
import refPropTypes from '@components/refPropTypes';
import withLocalize, {withLocalizePropTypes} from '@components/withLocalize';
Expand All @@ -17,6 +16,7 @@ import * as Browser from '@libs/Browser';
import compose from '@libs/compose';
import * as OptionsListUtils from '@libs/OptionsListUtils';
import * as ReportUtils from '@libs/ReportUtils';
import personalDetailsPropType from '@pages/personalDetailsPropType';
import reportPropTypes from '@pages/reportPropTypes';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
Expand Down Expand Up @@ -48,6 +48,9 @@ const propTypes = {
}),
),

/** All of the personal details for everyone */
personalDetails: PropTypes.objectOf(personalDetailsPropType),

/** All reports shared with the user */
reports: PropTypes.objectOf(reportPropTypes),

Expand All @@ -70,6 +73,7 @@ const defaultProps = {
participants: [],
forwardedRef: undefined,
safeAreaPaddingBottomStyle: {},
personalDetails: {},
reports: {},
betas: [],
isDistanceRequest: false,
Expand All @@ -80,6 +84,7 @@ function MoneyRequestParticipantsSelector({
forwardedRef,
betas,
participants,
personalDetails,
reports,
translate,
navigateToRequest,
Expand All @@ -98,7 +103,7 @@ function MoneyRequestParticipantsSelector({
userToInvite: null,
});
const {isOffline} = useNetwork();
const personalDetails = usePersonalDetails();

const maxParticipantsReached = participants.length === CONST.REPORT.MAXIMUM_PARTICIPANTS;

/**
Expand Down Expand Up @@ -155,32 +160,20 @@ function MoneyRequestParticipantsSelector({
}

return newSections;
}, [maxParticipantsReached, newChatOptions.personalDetails, newChatOptions.recentReports, newChatOptions.userToInvite, participants, personalDetails, searchTerm, translate]);
}, [maxParticipantsReached, newChatOptions, participants, personalDetails, translate, searchTerm]);

/**
* Adds a single participant to the request
*
* @param {Object} option
*/
const addSingleParticipant = useCallback(
(option) => {
onAddParticipants(
[
{
accountID: option.accountID,
login: option.login,
isPolicyExpenseChat: option.isPolicyExpenseChat,
reportID: option.reportID,
selected: true,
searchText: option.searchText,
},
],
false,
);
navigateToRequest();
},
[onAddParticipants, navigateToRequest],
);
const addSingleParticipant = (option) => {
onAddParticipants(
[{accountID: option.accountID, login: option.login, isPolicyExpenseChat: option.isPolicyExpenseChat, reportID: option.reportID, selected: true, searchText: option.searchText}],
false,
);
navigateToRequest();
};

/**
* Removes a selected option from list if already selected. If not already selected add this option to the list.
Expand Down Expand Up @@ -222,16 +215,12 @@ function MoneyRequestParticipantsSelector({
[participants, onAddParticipants],
);

const headerMessage = useMemo(
() =>
OptionsListUtils.getHeaderMessage(
newChatOptions.personalDetails.length + newChatOptions.recentReports.length !== 0,
Boolean(newChatOptions.userToInvite),
searchTerm.trim(),
maxParticipantsReached,
_.some(participants, (participant) => participant.searchText.toLowerCase().includes(searchTerm.trim().toLowerCase())),
),
[maxParticipantsReached, newChatOptions.personalDetails.length, newChatOptions.recentReports.length, newChatOptions.userToInvite, participants, searchTerm],
const headerMessage = OptionsListUtils.getHeaderMessage(
newChatOptions.personalDetails.length + newChatOptions.recentReports.length !== 0,
Boolean(newChatOptions.userToInvite),
searchTerm.trim(),
maxParticipantsReached,
_.some(participants, (participant) => participant.searchText.toLowerCase().includes(searchTerm.trim().toLowerCase())),
);
const isOptionsDataReady = ReportUtils.isReportDataReady() && OptionsListUtils.isPersonalDetailsReady(personalDetails);

Expand Down Expand Up @@ -289,27 +278,23 @@ function MoneyRequestParticipantsSelector({
navigateToSplit();
}, [shouldShowSplitBillErrorMessage, navigateToSplit]);

const footerContent = useMemo(
() => (
<View>
{shouldShowSplitBillErrorMessage && (
<FormHelpMessage
style={[styles.ph1, styles.mb2]}
isError
message="iou.error.splitBillMultipleParticipantsErrorMessage"
/>
)}
<Button
success
text={translate('iou.addToSplit')}
onPress={handleConfirmSelection}
pressOnEnter
isDisabled={shouldShowSplitBillErrorMessage}
const footerContent = (
<View>
{shouldShowSplitBillErrorMessage && (
<FormHelpMessage
style={[styles.ph1, styles.mb2]}
isError
message="iou.error.splitBillMultipleParticipantsErrorMessage"
/>
</View>
),
// eslint-disable-next-line react-hooks/exhaustive-deps
[handleConfirmSelection, shouldShowSplitBillErrorMessage, translate],
)}
<Button
success
text={translate('iou.addToSplit')}
onPress={handleConfirmSelection}
pressOnEnter
isDisabled={shouldShowSplitBillErrorMessage}
/>
</View>
);

return (
Expand All @@ -321,6 +306,7 @@ function MoneyRequestParticipantsSelector({
onAddToSelection={addParticipantToSelection}
sections={sections}
selectedOptions={participants}
value={searchTerm}
onSelectRow={addSingleParticipant}
onChangeText={setSearchTermAndSearchInServer}
ref={forwardedRef}
Expand Down Expand Up @@ -360,6 +346,9 @@ MoneyRequestParticipantsSelectorWithRef.displayName = 'MoneyRequestParticipantsS
export default compose(
withLocalize,
withOnyx({
personalDetails: {
key: ONYXKEYS.PERSONAL_DETAILS_LIST,
},
reports: {
key: ONYXKEYS.COLLECTION.REPORT,
},
Expand Down
Loading

0 comments on commit 777a52f

Please sign in to comment.