Skip to content

feat: show all icons alphabetically on empty search #210

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
nanoraptor wants to merge 3 commits into
base: main
Choose a base branch
from
Open

feat: show all icons alphabetically on empty search #210

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
80b2852
feat: show all icons alphabetically on empty search
nanoraptor Mar 23, 2026
4a32023
Apply nerdfont-presort-optimization patch
nanoraptor Mar 27, 2026
e1e0df1
Merge main branch with patch changes
nanoraptor Mar 27, 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
4 changes: 0 additions & 4 deletions extensions/nerdfont-search/package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion extensions/nerdfont-search/src/filtering.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@ function filterIconIndex({

if (searchText.length < 3) {
if (selectedPack === PACK_FILTER_ALL) {
return [];
return iconIndex
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

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

iconIndex.sort(...) mutates the shared iconIndex array in-place. Since the same array is cached (cachedIconIndex) and also passed into the Fuse instance (new Fuse(decodedIndex, ...)), this can reorder the underlying list after Fuse has been constructed and can lead to incorrect search results or hard-to-debug state issues. Use a non-mutating approach (e.g., sort a shallow copy) so the cached index and Fuse’s backing array aren’t modified.

Suggested change
return iconIndex
return [...iconIndex]

Copilot uses AI. Check for mistakes.
.sort((a, b) => a.displayName.localeCompare(b.displayName))
.slice(0, SEARCH_RESULT_LIMIT);
}
Comment on lines 37 to 45
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

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

The new behavior is described as “show all icons when search is empty”, but this branch runs for any searchText.length < 3 and also applies SEARCH_RESULT_LIMIT via slice(0, SEARCH_RESULT_LIMIT). That means 1–2 character queries will ignore the query and just show the default alphabetical list, and empty search will only show the first 200 icons—not “all” icons. Consider handling searchText.length === 0 separately from 1–2 characters, and clarify whether the limit should apply for the empty-search listing.

Copilot uses AI. Check for mistakes.

return iconIndex
Expand Down
33 changes: 14 additions & 19 deletions extensions/nerdfont-search/src/nerdfonts-search.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,14 +102,15 @@ export default function NerdFontSearch() {

const displayIcons = useMemo(() => {
if (searchText.length === 0 && activePack === PACK_FILTER_ALL) {
return recentIcons.map((icon) => ({
...icon,
keywords: [],
markdown: "",
}));
}
if (searchText.length < 3 && activePack === PACK_FILTER_ALL) {
return [];
// Show recent icons if available, otherwise show all icons
if (recentIcons.length > 0) {
return recentIcons.map((icon) => ({
...icon,
keywords: [],
markdown: "",
}));
}
return searchResults;
}
return searchResults;
}, [searchText, activePack, recentIcons, searchResults]);
Expand Down Expand Up @@ -163,22 +164,16 @@ export default function NerdFontSearch() {
title={
searchText.length > 0 && searchText.length < 3
? "Keep typing..."
: searchText.length === 0 && activePack !== PACK_FILTER_ALL
: searchText.length >= 3
? "No icons found"
: searchText.length >= 3
? "No icons found"
: "Start searching"
: "Loading icons..."
}
description={
searchText.length > 0 && searchText.length < 3
? "Enter at least 3 characters to search"
: searchText.length === 0 && activePack !== PACK_FILTER_ALL
? "Try selecting another icon pack"
: searchText.length >= 3
? "Try a different search term or pick another icon pack"
: recentIcons.length > 0
? "Your recently copied icons will appear here"
: "Enter at least 3 characters to search for icons"
: searchText.length >= 3
? "Try a different search term or pick another icon pack"
: "Your icon library is loading. Please wait..."
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

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

The empty-state copy now defaults to “Loading icons...” / “Your icon library is loading...” whenever displayIcons.length === 0 and searchText.length < 3. But isLoading can be false while displayIcons is still empty (e.g., the index load failed and iconIndex stayed empty in useIconData’s catch). In that case the UI would misleadingly claim it’s still loading. Consider keying the empty-state messaging off isLoading (and possibly adding an explicit error/failed-to-load state) instead of assuming empty results always mean loading.

Copilot uses AI. Check for mistakes.
}
icon={Icon.MagnifyingGlass}
/>
Expand Down
6 changes: 5 additions & 1 deletion extensions/nerdfont-search/test/pack-filter-behavior.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,11 @@ test("pack filter behavior for short and full search terms", () => {
searchText: "",
selectedPack: PACK_FILTER_ALL,
});
assert.equal(noSearchAll.length, 0, "all-pack short search should return []");
assert.ok(noSearchAll.length > 0, "all-pack short search should return all icons");
assert.ok(
noSearchAll.every((a, i, arr) => i === 0 || a.displayName >= arr[i - 1].displayName),
Comment on lines +34 to +36
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

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

The alphabetical-order assertion uses a.displayName >= arr[i - 1].displayName, which is a different ordering than the production code’s localeCompare and can behave differently with case/locale-specific characters. Also, the message says “return all icons”, but the implementation applies SEARCH_RESULT_LIMIT, so the assertion text is misleading. Align the test’s ordering check and expected wording with the production sorting and any intentional result limiting.

Suggested change
assert.ok(noSearchAll.length > 0, "all-pack short search should return all icons");
assert.ok(
noSearchAll.every((a, i, arr) => i === 0 || a.displayName >= arr[i - 1].displayName),
assert.ok(noSearchAll.length > 0, "all-pack short search should return icons");
assert.ok(
noSearchAll.every(
(icon, i, arr) =>
i === 0 || arr[i - 1].displayName.localeCompare(icon.displayName) <= 0
),

Copilot uses AI. Check for mistakes.
"all-pack short search should return icons sorted alphabetically"
);

const noSearchPack = filterIconIndex({
iconIndex: decodedIndex,
Expand Down
Loading