Skip to content
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

Jump to bottom

fix: Selected category is not ticked when it appears in the search result #37479

Merged
merged 3 commits into from
Mar 12, 2024
Merged

fix: Selected category is not ticked when it appears in the search result #37479

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d897d2e
fix: Selected category is not ticked when it appears in the search re…
dukenv0307 Feb 28, 2024
bbcaad4
Optimize logic get search categories
dukenv0307 Mar 4, 2024
66f081b
Merge branch 'main' into fix/37287
dukenv0307 Mar 6, 2024
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
12 changes: 11 additions & 1 deletion src/libs/OptionsListUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -982,7 +982,17 @@ function getCategoryListSections(
}

if (searchInputValue) {
const searchCategories = enabledCategories.filter((category) => category.name.toLowerCase().includes(searchInputValue.toLowerCase()));
const searchCategories: Category[] = [];

enabledCategories.forEach((category) => {
if (!category.name.toLowerCase().includes(searchInputValue.toLowerCase())) {
return;
}
searchCategories.push({
...category,
isSelected: selectedOptions.some((selectedOption) => selectedOption.name === category.name),
Copy link
Member

@rushatgabhane rushatgabhane Feb 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@dukenv0307 @puneetlath should we optimize this? This is a O(N^2) call for every key press on searching.

N is the number of categories that match the search text.

Copy link
Member

@rushatgabhane rushatgabhane Feb 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

We have a list of selected options and a list unselected, why rebuild the list? What do you think? I hope im not prematurely optimizing

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, ideally there is a more performant way we could do this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rushatgabhane i just updated the code, please check again

Copy link
Member

@rushatgabhane rushatgabhane Mar 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@dukenv0307 we don't need to recalculate the selected options on every keypress.

eg: we are calculating the enabled categories only once over here -
https://github.com/Expensify/App/pull/37479/files#diff-7d43f3c3e4decdb0057928e895f456d51c0f4081837edd5e2c4ed7ddd570e274R947

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rushatgabhane Currently, we only calculate searchCategories based on searchInputValue and enabledCategories by adding isSelected = true field to the enabledCategories for all categories in the selectedOptions array

Copy link
Member

@rushatgabhane rushatgabhane Mar 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

ah okay, thanks! i was misunderstood

});
});

categorySections.push({
// "Search" section
Expand Down
Loading