Skip to content

Commit

Permalink
Improve sorting of autocomplete options
Browse files Browse the repository at this point in the history
  • Loading branch information
Kicu committed Nov 20, 2024
1 parent 3c4e854 commit b7883d4
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 26 deletions.
4 changes: 2 additions & 2 deletions src/components/Search/SearchPageHeaderInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ function SearchPageHeaderInput({queryJSON, children}: SearchPageHeaderInputProps
const updatedSubstitutionsMap = getUpdatedSubstitutionsMap(userQuery, autocompleteSubstitutions);
setAutocompleteSubstitutions(updatedSubstitutionsMap);

if (updatedUserQuery) {
if (updatedUserQuery || textInputValue.length > 0) {
listRef.current?.updateAndScrollToFocusedIndex(0);
} else {
listRef.current?.updateAndScrollToFocusedIndex(-1);
Expand Down Expand Up @@ -214,7 +214,7 @@ function SearchPageHeaderInput({queryJSON, children}: SearchPageHeaderInputProps
}
: undefined;

const listTopPosition = variables.contentHeaderHeight;
const listTopPosition = variables.contentHeaderDesktopHeight;
const shouldShowAutocompleteList = isAutocompleteListVisible && !!textInputValue;

return (
Expand Down
2 changes: 1 addition & 1 deletion src/components/Search/SearchRouter/SearchRouter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ function SearchRouter({onRouterClose, shouldHideInputCaret}: SearchRouterProps)
const updatedSubstitutionsMap = getUpdatedSubstitutionsMap(userQuery, autocompleteSubstitutions);
setAutocompleteSubstitutions(updatedSubstitutionsMap);

if (updatedUserQuery) {
if (updatedUserQuery || textInputValue.length > 0) {
listRef.current?.updateAndScrollToFocusedIndex(0);
} else {
listRef.current?.updateAndScrollToFocusedIndex(-1);
Expand Down
71 changes: 52 additions & 19 deletions src/components/Search/SearchRouter/SearchRouterList.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
import {Str} from 'expensify-common';
import React, {forwardRef, useCallback, useMemo} from 'react';
import React, {forwardRef, useMemo} from 'react';
import type {ForwardedRef} from 'react';
import {useOnyx} from 'react-native-onyx';
import * as Expensicons from '@components/Icon/Expensicons';
import {usePersonalDetails} from '@components/OnyxProvider';
import {useOptionsList} from '@components/OptionListContextProvider';
import type {SearchFilterKey, SearchQueryString} from '@components/Search/types';
import type {SearchFilterKey} from '@components/Search/types';
import SelectionList from '@components/SelectionList';
import SearchQueryListItem, {isSearchQueryItem} from '@components/SelectionList/Search/SearchQueryListItem';
import type {SearchQueryItem, SearchQueryListItemProps} from '@components/SelectionList/Search/SearchQueryListItem';
Expand All @@ -18,6 +17,7 @@ import useResponsiveLayout from '@hooks/useResponsiveLayout';
import useThemeStyles from '@hooks/useThemeStyles';
import * as CardUtils from '@libs/CardUtils';
import * as OptionsListUtils from '@libs/OptionsListUtils';
import type {SearchOption} from '@libs/OptionsListUtils';
import Performance from '@libs/Performance';
import {getAllTaxRates} from '@libs/PolicyUtils';
import type {OptionData} from '@libs/ReportUtils';
Expand Down Expand Up @@ -60,6 +60,14 @@ type SearchRouterListProps = {
onListItemFocus: (item: SearchQueryItem) => void;
};

const defaultListOptions = {
userToInvite: null,
recentReports: [],
personalDetails: [],
currentUserOption: null,
categoryOptions: [],
};

const setPerformanceTimersEnd = () => {
Timing.end(CONST.TIMING.OPEN_SEARCH);
Performance.markEnd(CONST.TIMING.OPEN_SEARCH);
Expand Down Expand Up @@ -112,7 +120,7 @@ function SearchRouterList(
const {options, areOptionsInitialized} = useOptionsList();
const searchOptions = useMemo(() => {
if (!areOptionsInitialized) {
return {recentReports: [], personalDetails: [], userToInvite: null, currentUserOption: null, categoryOptions: [], tagOptions: [], taxRatesOptions: []};
return defaultListOptions;
}
return OptionsListUtils.getSearchOptions(options, '', betas ?? []);
}, [areOptionsInitialized, betas, options]);
Expand All @@ -123,19 +131,44 @@ function SearchRouterList(

const [cardList = {}] = useOnyx(ONYXKEYS.CARD_LIST);
const cardAutocompleteList = Object.values(cardList);
const personalDetailsForParticipants = usePersonalDetails();
const participantsAutocompleteList = useMemo(
() =>
Object.values(personalDetailsForParticipants)
.filter((details): details is NonNullable<PersonalDetails> => !!(details && details?.login))
.map((details) => ({

const participantsAutocompleteList = useMemo(() => {
if (!areOptionsInitialized) {
return [];
}

const filteredOptions = OptionsListUtils.getFilteredOptions({
reports: options.reports,
personalDetails: options.personalDetails,
excludeLogins: CONST.EXPENSIFY_EMAILS,
maxRecentReportsToShow: 0,
includeSelfDM: true,
});

// This cast is needed as something is incorrect in types OptionsListUtils.getOptions around l1490 and includeRecentReports types
const personalDetailsFromOptions = filteredOptions.personalDetails.map((option) => (option as SearchOption<PersonalDetails>).item);
const autocompleteOptions = Object.values(personalDetailsFromOptions)
.filter((details): details is NonNullable<PersonalDetails> => !!(details && details?.login))
.map((details) => {
return {
name: details.displayName ?? Str.removeSMSDomain(details.login ?? ''),
accountID: details?.accountID.toString(),
})),
[personalDetailsForParticipants],
);
accountID: details.accountID.toString(),
};
});
const currentUser = (filteredOptions.currentUserOption as SearchOption<PersonalDetails>).item;
if (currentUser) {
autocompleteOptions.push({
name: currentUser.displayName ?? Str.removeSMSDomain(currentUser.login ?? ''),
accountID: currentUser.accountID?.toString() ?? '-1',
});
}

return autocompleteOptions;
}, [areOptionsInitialized, options.personalDetails, options.reports]);

const taxRates = getAllTaxRates();
const taxAutocompleteList = useMemo(() => getAutocompleteTaxList(taxRates, policy), [policy, taxRates]);

const [allPolicyCategories] = useOnyx(ONYXKEYS.COLLECTION.POLICY_CATEGORIES);
const [allRecentCategories] = useOnyx(ONYXKEYS.COLLECTION.POLICY_RECENTLY_USED_CATEGORIES);
const categoryAutocompleteList = useMemo(() => {
Expand Down Expand Up @@ -219,7 +252,6 @@ function SearchRouterList(
case CONST.SEARCH.SYNTAX_FILTER_KEYS.FROM: {
const filteredParticipants = participantsAutocompleteList
.filter((participant) => participant.name.toLowerCase().includes(autocompleteValue.toLowerCase()) && !alreadyAutocompletedKeys.includes(participant.name.toLowerCase()))
.sort()
.slice(0, 10);

return filteredParticipants.map((participant) => ({
Expand All @@ -231,7 +263,6 @@ function SearchRouterList(
case CONST.SEARCH.SYNTAX_FILTER_KEYS.TO: {
const filteredParticipants = participantsAutocompleteList
.filter((participant) => participant.name.toLowerCase().includes(autocompleteValue.toLowerCase()) && !alreadyAutocompletedKeys.includes(participant.name.toLowerCase()))
.sort()
.slice(0, 10);

return filteredParticipants.map((participant) => ({
Expand All @@ -242,8 +273,7 @@ function SearchRouterList(
}
case CONST.SEARCH.SYNTAX_FILTER_KEYS.IN: {
const filteredChats = searchOptions.recentReports
.filter((chat) => chat.text?.toLowerCase()?.includes(autocompleteValue.toLowerCase()))
.sort((chatA, chatB) => (chatA > chatB ? 1 : -1))
.filter((chat) => chat.text?.toLowerCase()?.includes(autocompleteValue.toLowerCase()) && !alreadyAutocompletedKeys.includes(chat.text.toLowerCase()))
.slice(0, 10);

return filteredChats.map((chat) => ({
Expand Down Expand Up @@ -279,7 +309,10 @@ function SearchRouterList(
}
case CONST.SEARCH.SYNTAX_FILTER_KEYS.CARD_ID: {
const filteredCards = cardAutocompleteList
.filter((card) => card.bank.toLowerCase().includes(autocompleteValue.toLowerCase()) && !alreadyAutocompletedKeys.includes(card.bank.toLowerCase()))
.filter(
(card) =>
card.bank.toLowerCase().includes(autocompleteValue.toLowerCase()) && !alreadyAutocompletedKeys.includes(CardUtils.getCardDescription(card.cardID).toLowerCase()),
)
.sort()
.slice(0, 10);

Expand Down
2 changes: 1 addition & 1 deletion src/styles/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2571,7 +2571,7 @@ const styles = (theme: ThemeColors) =>
searchResultsHeaderBar: {
display: 'flex',
justifyContent: 'center',
height: variables.contentHeaderHeight,
height: variables.contentHeaderDesktopHeight,
zIndex: variables.popoverzIndex,
position: 'relative',
paddingHorizontal: 20,
Expand Down
6 changes: 3 additions & 3 deletions tests/unit/Search/buildSubstitutionsMapTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ jest.mock('@libs/ReportUtils', () => {
parseReportRouteParams: jest.fn(() => ({})),
// The `getReportName` method is quite complex, and we don't need to test it, we just want to test the logic around generating subsitutionsMap
getReportName(report: OnyxTypes.Report) {
return report.displayName;
return report.reportName;
},
};
});
Expand Down Expand Up @@ -52,11 +52,11 @@ const cardListMock = {
const reportsMock = {
[`${ONYXKEYS.COLLECTION.REPORT}rep123`]: {
reportID: 'rep123',
displayName: 'Report 1',
reportName: 'Report 1',
},
[`${ONYXKEYS.COLLECTION.REPORT}rep456`]: {
reportID: 'rep456',
displayName: 'Report 2',
reportName: 'Report 2',
},
} as OnyxCollection<OnyxTypes.Report>;

Expand Down

0 comments on commit b7883d4

Please sign in to comment.