Use tags and aliases when filtering icons in Icon Picker #10425
Conversation
src/components/ha-icon-picker.ts
Outdated
| const getItemScore = (item: IconItem): number => | ||
| item.icon.includes(filterString) ? 1 : 0; | ||
|
|
||
| filteredItems.sort((i1, i2) => getItemScore(i2) - getItemScore(i1)); |
There was a problem hiding this comment.
The items are sorted to have the icon containing the filterString first. If you search home, you want icons containing home word instead of home automation tagged icons.
piitaya
changed the title
src/components/ha-icon-picker.ts
Outdated
| aliases: string[]; | ||
| tags: string[]; |
|
So how is performance? 😃 |
src/components/ha-icon-picker.ts
Outdated
| name: string; | ||
| aliases: string[]; | ||
| tags: string[]; | ||
| tagsAndAliases: string[]; |
There was a problem hiding this comment.
Maybe name it keywords? Also for custom icons?
There was a problem hiding this comment.
Oh nice ! I did not find a good name for it but you find it !
Performance is good (~60 ms max for the search). I optimized the search in this commit : f349749. It's more efficient than the |
src/components/ha-icon-picker.ts
Outdated
| const filteredItems = iconItems.filter((item) => | ||
| item.icon.includes(filterString) | ||
| ); | ||
|
|
||
| const filteredItemsByTagsOrAliases = iconItems.filter( | ||
| (item) => | ||
| item.icon.includes(filterString) || | ||
| !item.icon.includes(filterString) && | ||
| item.tagsAndAliases.some((t) => t.includes(filterString)) | ||
| ); |
There was a problem hiding this comment.
You can combine this in 1 for loop, so we don't have to loop it twice.
iconItems.forEach((item) => {
if (item.icon.includes(filterString)) {
filteredItems.push(item);
return;
}
if (item.keywords.some((t) => t.includes(filterString)) {
filteredItemsByKeywords.push(item);
}
});There was a problem hiding this comment.
Why return instead of else if ? Just code style ?
There was a problem hiding this comment.
Yeah, we tend to return as quickly as possible
build-scripts/gulp/gen-icons-json.js
Outdated
| tags: icon.tags.map((t) => t.toLowerCase().replace(/\s\/\s/g, " ")), | ||
| aliases: icon.aliases, |
There was a problem hiding this comment.
Let's combine it to keywords here already, makes the json smaller, and saves work on the client :-)
What is the performance like now btw? |
|
I will update the PR for custom iconsets to support keywords. |
I didn't notice any difference but the code is more logic. |
Breaking change
No breaking change
Proposed change
Use the available tags and aliases in mdi icon meta to improve search experience in icon picker.
If we merge that PR, we can update this PR #10399 to allow custom iconsets proposing tags and aliases too.
Type of change
Example configuration
Additional information
Checklist
If user exposed functionality or configuration variables are added/changed: