Skip to content

Fix/move preselected items to top #80322

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
marufsharifi wants to merge 14 commits into
base: main
Choose a base branch
from
+706 −327
Open

Fix/move preselected items to top #80322

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
722464c
Fix sorting to move pre-selected items to the top of lists
marufsharifi Jan 18, 2026
79913be
Fix sorting to move pre-selected items to the top of lists second lists
marufsharifi Jan 18, 2026
82578b6
fixed for the attendees selector
marufsharifi Jan 18, 2026
3c359bc
Merge branch 'main' into fix/move-preselected-items-to-top
marufsharifi Jan 23, 2026
6a442af
revert changes for SearchColumnsPage
marufsharifi Jan 23, 2026
05b5547
removed unnecessary props
marufsharifi Jan 23, 2026
8f88a11
Merge branch 'main' into fix/move-preselected-items-to-top
marufsharifi Jan 24, 2026
9d36163
applied ai optimization suggestions.
marufsharifi Jan 25, 2026
673dce1
moving the loop outside of searchOptions
marufsharifi Jan 25, 2026
c230c97
fixed the attendees issues.
marufsharifi Jan 25, 2026
4cb6618
fixed workspaces dropdown in reports
marufsharifi Jan 25, 2026
f85c1cc
fixed InviteReportParticipantsPage to works
marufsharifi Jan 25, 2026
5d4f1e6
fixed Country selection
marufsharifi Jan 25, 2026
0db9c28
fixed type errors.
marufsharifi Jan 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/CONST/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1985,6 +1985,8 @@ const CONST = {
GLOBAL_CREATE: '\uE100',
},

MOVE_SELECTED_ITEMS_TO_TOP_OF_LIST_THRESHOLD: 8,

INVISIBLE_CODEPOINTS: ['fe0f', '200d', '2066'],

UNICODE: {
Expand Down
2 changes: 1 addition & 1 deletion src/components/CountryPicker/CountrySelectorModal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ function CountrySelectorModal({isVisible, currentCountry, onCountrySelected, onC
[translate, currentCountry],
);

const searchResults = searchOptions(debouncedSearchValue, countries);
const searchResults = useMemo(() => searchOptions(debouncedSearchValue, countries, currentCountry ? [currentCountry] : []), [countries, debouncedSearchValue, currentCountry]);
const headerMessage = debouncedSearchValue.trim() && !searchResults.length ? translate('common.noResultsFound') : '';

const styles = useThemeStyles();
Expand Down
5 changes: 4 additions & 1 deletion src/components/PushRowWithModal/PushRowModal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,10 @@ function PushRowModal({isVisible, selectedOption, onOptionChange, onClose, optio
setSearchValue('');
};

const searchResults = searchOptions(debouncedSearchValue, options);
const searchResults = useMemo(
() => searchOptions(debouncedSearchValue, options, selectedOption ? [selectedOption] : []),
[debouncedSearchValue, options, selectedOption],
);

const textInputOptions = useMemo(
() => ({
Expand Down
46 changes: 22 additions & 24 deletions src/components/Search/FilterDropdowns/UserSelectPopup.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import {accountIDSelector} from '@selectors/Session';
import isEmpty from 'lodash/isEmpty';
import React, {memo, useCallback, useMemo, useRef, useState} from 'react';
import {View} from 'react-native';
Expand Down Expand Up @@ -59,7 +58,6 @@
const personalDetails = usePersonalDetails();
const {windowHeight} = useWindowDimensions();
const {shouldUseNarrowLayout} = useResponsiveLayout();
const [accountID] = useOnyx(ONYXKEYS.SESSION, {canBeMissing: true, selector: accountIDSelector});
const shouldFocusInputOnScreenFocus = canFocusInputOnScreenFocus();
const [countryCode = CONST.DEFAULT_COUNTRY_CODE] = useOnyx(ONYXKEYS.COUNTRY_CODE, {canBeMissing: false});
const [loginList] = useOnyx(ONYXKEYS.LOGIN_LIST, {canBeMissing: true});
Expand Down Expand Up @@ -92,6 +90,10 @@
return new Set(selectedOptions.map((option) => option.accountID).filter(Boolean));
}, [selectedOptions]);

const initialSelectedAccountIDs = useMemo(() => {
return new Set(initialSelectedOptions.map((option) => option.accountID).filter(Boolean));
}, [initialSelectedOptions]);

const optionsList = useMemo(() => {
return memoizedGetValidOptions(
{
Expand Down Expand Up @@ -119,41 +121,38 @@
}, [optionsList, cleanSearchTerm, countryCode, loginList]);

const listData = useMemo(() => {
const personalDetailList = filteredOptions.personalDetails.map((participant) => ({
const initialOptions: Array<Option & {keyForList: string}> = [];
const remainingOptions: Array<Option & {keyForList: string}> = [];

const personalDetailOptions = filteredOptions.personalDetails.map((participant) => ({
...participant,
isSelected: selectedAccountIDs.has(participant.accountID),
keyForList: String(participant.accountID),
}));

const recentReportsList = filteredOptions.recentReports.map((report) => ({
const recentReportOptions = filteredOptions.recentReports.map((report) => ({
...report,
isSelected: selectedAccountIDs.has(report.accountID),
keyForList: String(report.reportID),
}));

const combined = [...personalDetailList, ...recentReportsList];
const totalOptions = personalDetailOptions.length + recentReportOptions.length;
const reordered = [...personalDetailOptions, ...recentReportOptions];

combined.sort((a, b) => {
// selected items first
if (a.isSelected && !b.isSelected) {
return -1;
}
if (!a.isSelected && b.isSelected) {
return 1;
}
if (totalOptions <= CONST.MOVE_SELECTED_ITEMS_TO_TOP_OF_LIST_THRESHOLD) {
return reordered;
}

// Put the current user at the top of the list
if (a.accountID === accountID) {
return -1;
}
if (b.accountID === accountID) {
return 1;
for (const option of reordered) {
if (option.accountID && initialSelectedAccountIDs.has(option.accountID)) {
initialOptions.push(option);
} else {
remainingOptions.push(option);
}
return 0;
});
}

return combined;
}, [filteredOptions, accountID, selectedAccountIDs]);
return [...initialOptions, ...remainingOptions];
}, [filteredOptions, initialSelectedAccountIDs, selectedAccountIDs]);

const headerMessage = useMemo(() => {
const noResultsFound = isEmpty(listData);
Expand All @@ -165,7 +164,6 @@
const isSelected = selectedOptions.some((selected) => optionsMatch(selected, option));

setSelectedOptions((prev) => (isSelected ? prev.filter((selected) => !optionsMatch(selected, option)) : [...prev, getSelectedOptionData(option)]));
selectionListRef?.current?.scrollToIndex(0);
},
[selectedOptions],
);
Expand Down Expand Up @@ -208,7 +206,7 @@
canSelectMultiple
ListItem={UserSelectionListItem}
style={{containerStyle: [!shouldUseNarrowLayout && styles.pt4], listStyle: styles.pb2}}
onSelectRow={selectUser}

Check failure on line 209 in src/components/Search/FilterDropdowns/UserSelectPopup.tsx

View workflow job for this annotation

GitHub Actions / typecheck 

Type '(option: Option) => void' is not assignable to type '(item: ListItem) => void'.
isLoadingNewOptions={isLoadingNewOptions}
/>

Expand Down
85 changes: 59 additions & 26 deletions src/components/Search/SearchFiltersParticipantsSelector.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ import useOnyx from '@hooks/useOnyx';
import useScreenWrapperTransitionStatus from '@hooks/useScreenWrapperTransitionStatus';
import {canUseTouchScreen} from '@libs/DeviceCapabilities';
import memoize from '@libs/memoize';
import {filterAndOrderOptions, filterSelectedOptions, formatSectionsFromSearchTerm, getValidOptions} from '@libs/OptionsListUtils';
import type {Option, Section} from '@libs/OptionsListUtils';
import {filterAndOrderOptions, filterSelectedOptions, formatSectionsFromSearchTerm, getParticipantsOption, getPolicyExpenseReportOption, getValidOptions} from '@libs/OptionsListUtils';
import type {Option} from '@libs/OptionsListUtils';
import type {OptionData} from '@libs/ReportUtils';
import {getDisplayNameForParticipant} from '@libs/ReportUtils';
import Navigation from '@navigation/Navigation';
Expand Down Expand Up @@ -41,7 +41,7 @@ type SearchFiltersParticipantsSelectorProps = {
};

function SearchFiltersParticipantsSelector({initialAccountIDs, onFiltersUpdate}: SearchFiltersParticipantsSelectorProps) {
const {translate, formatPhoneNumber} = useLocalize();
const {translate, formatPhoneNumber, localeCompare} = useLocalize();
const personalDetails = usePersonalDetails();
const {didScreenTransitionEnd} = useScreenWrapperTransitionStatus();
const {options, areOptionsInitialized} = useOptionsList({
Expand Down Expand Up @@ -79,13 +79,28 @@ function SearchFiltersParticipantsSelector({initialAccountIDs, onFiltersUpdate}:
);
}, [areOptionsInitialized, options.reports, options.personalDetails, allPolicies, draftComments, nvpDismissedProductTraining, loginList, countryCode]);

const initialSelectedOptions = useMemo(() => {
if (!initialAccountIDs || initialAccountIDs.length === 0 || !personalDetails) {
return [];
}

const preSelectedOptions = initialAccountIDs.reduce<OptionData[]>((acc, accountID) => {
const participant = personalDetails[accountID];
if (participant) {
acc.push(getSelectedOptionData(participant));
}
return acc;
}, []);

return preSelectedOptions;
}, [initialAccountIDs, personalDetails]);

const unselectedOptions = useMemo(() => {
return filterSelectedOptions(defaultOptions, new Set(selectedOptions.map((option) => option.accountID)));
}, [defaultOptions, selectedOptions]);

const chatOptions = useMemo(() => {
const filteredOptions = filterAndOrderOptions(unselectedOptions, cleanSearchTerm, countryCode, loginList, {
selectedOptions,
excludeLogins: CONST.EXPENSIFY_EMAILS_OBJECT,
maxRecentReportsToShow: CONST.IOU.MAX_RECENT_REPORTS_TO_SHOW,
canInviteUser: false,
Expand All @@ -99,10 +114,10 @@ function SearchFiltersParticipantsSelector({initialAccountIDs, onFiltersUpdate}:
}

return filteredOptions;
}, [unselectedOptions, cleanSearchTerm, countryCode, loginList, selectedOptions]);
}, [unselectedOptions, cleanSearchTerm, countryCode, loginList]);

const {sections, headerMessage} = useMemo(() => {
const newSections: Section[] = [];
const sectionData: OptionData[] = [];
if (!areOptionsInitialized) {
return {sections: [], headerMessage: undefined};
}
Expand All @@ -127,44 +142,62 @@ function SearchFiltersParticipantsSelector({initialAccountIDs, onFiltersUpdate}:
}

// If the current user is not selected, add them to the top of the list
if (!selectedCurrentUser && chatOptions.currentUserOption) {
if (!selectedCurrentUser && chatOptions.currentUserOption && !initialSelectedOptions.some((option) => option.accountID === chatOptions.currentUserOption?.accountID)) {
Copy link
Contributor

@github-actions github-actions bot Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ PERF-13 (docs)

The .some() call inside the conditional on line 145 iterates through initialSelectedOptions but doesn't use any iterator-dependent values from the conditional context. Since this is inside a conditional (not an array method callback), this should be optimized.

Suggested fix: Create a Set of account IDs before this conditional:

const initialSelectedAccountIDs = new Set(initialSelectedOptions.map((option) => option.accountID));

// Then use in conditional:
if (!selectedCurrentUser && chatOptions.currentUserOption && !initialSelectedAccountIDs.has(chatOptions.currentUserOption?.accountID)) {

This Set can be reused for the filter operations on lines 157 and 162 as well.

Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.

const formattedName = getDisplayNameForParticipant({
accountID: chatOptions.currentUserOption.accountID,
shouldAddCurrentUserPostfix: true,
personalDetailsData: personalDetails,
formatPhoneNumber,
});
chatOptions.currentUserOption.text = formattedName;

newSections.push({
title: '',
data: [chatOptions.currentUserOption],
shouldShow: true,
});
sectionData.push(chatOptions.currentUserOption);
}

newSections.push(formattedResults.section);
const selectedIDsSet = new Set(initialSelectedOptions.map((option) => option.accountID));
const unselectedFormattedSectionData = formattedResults.section.data.filter((option) => !selectedIDsSet.has(option.accountID));
Copy link
Contributor

@github-actions github-actions bot Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ PERF-13 (docs)

Inside the .filter() on line 157, there is a .some() call that iterates through initialSelectedOptions for every item in chatOptions.recentReports. This creates O(n*m) complexity where both operations could be optimized.

Suggested fix: Create a Set of initial selected account IDs before the filter operations to enable O(1) lookups:

const initialSelectedAccountIDs = new Set(initialSelectedOptions.map((option) => option.accountID));
const unselectedRecentReports = chatOptions.recentReports.filter((report) => !initialSelectedAccountIDs.has(report.accountID));

This pattern should also be applied to line 162 with chatOptions.personalDetails.

Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.

if (unselectedFormattedSectionData.length) {
sectionData.push(...(unselectedFormattedSectionData as OptionData[]));
}

newSections.push({
title: '',
data: chatOptions.recentReports,
shouldShow: chatOptions.recentReports.length > 0,
});
const unselectedRecentReports = chatOptions.recentReports.filter((report) => !initialSelectedOptions.some((selectedOption) => selectedOption.accountID === report.accountID));
if (unselectedRecentReports) {
sectionData.push(...unselectedRecentReports);
}

newSections.push({
title: '',
data: chatOptions.personalDetails,
shouldShow: chatOptions.personalDetails.length > 0,
});
const unselectedPersonalDetails = chatOptions.personalDetails.filter((detail) => !initialSelectedOptions.some((selectedOption) => selectedOption.accountID === detail.accountID));
if (unselectedPersonalDetails) {
sectionData.push(...unselectedPersonalDetails);
}

const noResultsFound = chatOptions.personalDetails.length === 0 && chatOptions.recentReports.length === 0 && !chatOptions.currentUserOption;
const message = noResultsFound ? translate('common.noResultsFound') : undefined;
let sortedSectionData = sectionData.sort((a, b) => localeCompare(a?.login?.toLowerCase() ?? '', b?.login?.toLowerCase() ?? ''));

if (initialSelectedOptions.length && cleanSearchTerm === '') {
sortedSectionData = [
Copy link
Contributor

@github-actions github-actions bot Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ PERF-13 (docs)

Inside the .map() callback, the selectedOptions.some() call on line 177 doesn't use the iterator (participant) in its logic - it only checks if the participant.accountID matches any selected option. This function call should be hoisted outside the map.

Suggested fix: Create a Set of selected account IDs before the map:

const selectedAccountIDsSet = new Set(selectedOptions.map((option) => option.accountID));

const mappedParticipants = initialSelectedOptions.map((participant) => {
    const participantData = {
        ...participant,
        selected: selectedAccountIDsSet.has(participant.accountID),
    };
    // ... rest of logic
});

Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.

...(initialSelectedOptions.map((participant) => {
const participantData = {
...participant,
selected: selectedOptions.some((selectedOption) => selectedOption.accountID === participant.accountID),
};
const isReportPolicyExpenseChat = participant.isPolicyExpenseChat ?? false;
return isReportPolicyExpenseChat ? getPolicyExpenseReportOption(participantData, reportAttributesDerived) : getParticipantsOption(participantData, personalDetails);
}) as OptionData[]),
...sortedSectionData,
];
}

return {
sections: newSections,
sections: [
{
title: '',
data: sortedSectionData,
shouldShow: sortedSectionData.length > 0,
},
],
headerMessage: message,
};
}, [areOptionsInitialized, cleanSearchTerm, selectedOptions, chatOptions, personalDetails, reportAttributesDerived, translate, formatPhoneNumber]);
}, [areOptionsInitialized, cleanSearchTerm, selectedOptions, chatOptions, personalDetails, reportAttributesDerived, initialSelectedOptions, translate, formatPhoneNumber, localeCompare]);

const resetChanges = useCallback(() => {
setSelectedOptions([]);
Expand Down
55 changes: 28 additions & 27 deletions src/components/Search/SearchMultipleSelectionPicker.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,46 +34,47 @@ function SearchMultipleSelectionPicker({items, initiallySelectedItems, pickerTit
}, [initiallySelectedItems]);

const {sections, noResultsFound} = useMemo(() => {
const selectedItemsSection = selectedItems
.filter((item) => item?.name.toLowerCase().includes(debouncedSearchTerm?.toLowerCase()))
.sort((a, b) => sortOptionsWithEmptyValue(a.value.toString(), b.value.toString(), localeCompare))
.map((item) => ({
text: item.name,
keyForList: item.name,
isSelected: true,
value: item.value,
}));
const remainingItemsSection = items
.filter(
(item) =>
!selectedItems.some((selectedItem) => selectedItem.value.toString() === item.value.toString()) && item?.name?.toLowerCase().includes(debouncedSearchTerm?.toLowerCase()),
)
.sort((a, b) => sortOptionsWithEmptyValue(a.value.toString(), b.value.toString(), localeCompare))
.map((item) => ({
const filteredItems = items.filter((item) => item.name.toLowerCase().includes(debouncedSearchTerm.toLowerCase()));

const initialValues = new Set(initiallySelectedItems?.map((item) => item.value.toString()) ?? []);
const selectedValues = new Set(selectedItems.map((item) => item.value.toString()));

const initialItems: Array<{text: string; keyForList: string; isSelected: boolean; value: string | string[]}> = [];
const remainingItems: Array<{text: string; keyForList: string; isSelected: boolean; value: string | string[]}> = [];

const sortedItems = filteredItems.sort((a, b) => sortOptionsWithEmptyValue(a.value.toString(), b.value.toString(), localeCompare));

for (const item of sortedItems) {
const mapped = {
text: item.name,
keyForList: item.name,
isSelected: false,
isSelected: selectedValues.has(item.value.toString()),
value: item.value,
}));
const isEmpty = !selectedItemsSection.length && !remainingItemsSection.length;
};
if (!initialValues.size || !initialValues.has(item.value.toString())) {
remainingItems.push(mapped);
} else {
initialItems.push(mapped);
}
}

const shouldReorder = !debouncedSearchTerm.trim() && initialItems.length > 0;
const data = shouldReorder ? [...initialItems, ...remainingItems] : [...initialItems, ...remainingItems];

const isEmpty = data.length === 0;
return {
sections: isEmpty
? []
: [
{
title: undefined,
data: selectedItemsSection,
shouldShow: selectedItemsSection.length > 0,
},
{
title: pickerTitle,
data: remainingItemsSection,
shouldShow: remainingItemsSection.length > 0,
data,
shouldShow: true,
},
],
noResultsFound: isEmpty,
};
}, [selectedItems, items, pickerTitle, debouncedSearchTerm, localeCompare]);
}, [selectedItems, items, pickerTitle, debouncedSearchTerm, localeCompare, initiallySelectedItems]);

const onSelectItem = useCallback(
(item: Partial<OptionData & SearchMultipleSelectionPickerItem>) => {
Expand Down
Loading
Loading