Skip to content

feat: show all icons alphabetically on empty search#210

Open
nanoraptor wants to merge 3 commits intovicinaehq:mainfrom
nanoraptor:feat/show-all-icons-on-empty-search
Open

feat: show all icons alphabetically on empty search#210
nanoraptor wants to merge 3 commits intovicinaehq:mainfrom
nanoraptor:feat/show-all-icons-on-empty-search

Conversation

@nanoraptor
Copy link
Copy Markdown

  • Modified filterIconIndex to return all icons sorted alphabetically when search is empty
  • Updated display logic to show all icons instead of 'Start searching' message
  • Updated test to verify new behavior
  • All 11 tests passing

   - Modified filterIconIndex to return all icons sorted alphabetically when
  search is empty
   - Updated display logic to show all icons instead of 'Start searching'
  message
   - Updated test to verify new behavior
   - All 11 tests passing

   Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR changes Nerd Font Search so that when the search box is empty (and “All icon packs” is selected), the UI shows an alphabetical icon list instead of an empty “Start searching” state, and updates tests accordingly.

Changes:

  • Updated filterIconIndex to return an alphabetically sorted list for short/empty search in the “all packs” mode.
  • Updated the grid display/empty-state logic to show icons (or a loading message) rather than a “Start searching” prompt.
  • Updated the pack filter behavior test to validate the new empty-search behavior.

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 4 comments.

File Description
extensions/nerdfont-search/src/filtering.ts Changes short/empty-search behavior to return a sorted icon list for all packs.
extensions/nerdfont-search/src/nerdfonts-search.tsx Adjusts which icons are displayed when search is empty and updates empty-state messaging.
extensions/nerdfont-search/test/pack-filter-behavior.test.ts Updates assertions for the new empty-search behavior and sorting expectation.
extensions/nerdfont-search/package-lock.json Lockfile metadata changes.
Files not reviewed (1)
  • extensions/nerdfont-search/package-lock.json: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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.
Comment on lines 33 to 38
if (searchText.length < 3) {
if (selectedPack === PACK_FILTER_ALL) {
return [];
return iconIndex
.sort((a, b) => a.displayName.localeCompare(b.displayName))
.slice(0, SEARCH_RESULT_LIMIT);
}
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.
Comment on lines +34 to +36
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),
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.
Comment on lines +162 to +176
: "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.
@SiriusCrain
Copy link
Copy Markdown

@nanoraptor is it ok for you if I will push some perfomance fixes to your PR?

@nanoraptor
Copy link
Copy Markdown
Author

@nanoraptor is it ok for you if I will push some perfomance fixes to your PR?

Okay

@SiriusCrain
Copy link
Copy Markdown

SiriusCrain commented Mar 25, 2026

I cannot push since it's your fork, here is patch, apply it please:
nerdfont-presort-optimization.patch

What was fixed:

  • Icons are pre-sorted, before they were sorted on each filterIconIndex call
  • map lookup + slice instead of filtering 10K items
  • removed array mutation
  • fixed search

5 runs median result

Before fix:

Scenario Median Mean P95 Min Max
empty-search-all 3.42ms 3.45ms 3.77ms 3.38ms 3.93ms
empty-search-pack 0.44ms 0.44ms 0.45ms 0.43ms 0.56ms
short-search-all 3.28ms 3.28ms 3.44ms 3.25ms 3.68ms
fuse-search-all 113.20ms 113.58ms 116.92ms 110.56ms 129.55ms
fuse-search-pack 144.04ms 144.88ms 147.92ms 141.80ms 167.30ms

After fix:

Scenario Median Mean P95 Min Max
empty-search-all 0.00ms 0.01ms 0.01ms 0.00ms 0.03ms
empty-search-pack 0.13ms 0.13ms 0.15ms 0.12ms 0.30ms
short-search-all 0.00ms 0.00ms 0.01ms 0.00ms 0.01ms
fuse-search-all 114.86ms 116.00ms 125.70ms 112.07ms 135.63ms
fuse-search-pack 145.82ms 146.05ms 147.83ms 143.79ms 164.66ms

@fbosch
Copy link
Copy Markdown
Contributor

fbosch commented Mar 25, 2026

I cannot push since it's your fork, here is patch, apply it please: nerdfont-presort-optimization.patch

What was fixed:

* Icons are pre-sorted, before they were sorted on each `filterIconIndex` call

* map lookup + slice instead of filtering 10K items

* removed array mutation

* fixed search

5 runs median result

Before fix:
Scenario Median Mean P95 Min Max
empty-search-all 3.42ms 3.45ms 3.77ms 3.38ms 3.93ms
empty-search-pack 0.44ms 0.44ms 0.45ms 0.43ms 0.56ms
short-search-all 3.28ms 3.28ms 3.44ms 3.25ms 3.68ms
fuse-search-all 113.20ms 113.58ms 116.92ms 110.56ms 129.55ms
fuse-search-pack 144.04ms 144.88ms 147.92ms 141.80ms 167.30ms

After fix:
Scenario Median Mean P95 Min Max
empty-search-all 0.00ms 0.01ms 0.01ms 0.00ms 0.03ms
empty-search-pack 0.13ms 0.13ms 0.15ms 0.12ms 0.30ms
short-search-all 0.00ms 0.00ms 0.01ms 0.00ms 0.01ms
fuse-search-all 114.86ms 116.00ms 125.70ms 112.07ms 135.63ms
fuse-search-pack 145.82ms 146.05ms 147.83ms 143.79ms 164.66ms

The decision of whether or not this is something we wanna go with still remains:
#209 (comment)

Maybe making it opt-in could be a solution?

@SiriusCrain
Copy link
Copy Markdown

I cannot push since it's your fork, here is patch, apply it please: nerdfont-presort-optimization.patch

What was fixed:

* Icons are pre-sorted, before they were sorted on each `filterIconIndex` call

* map lookup + slice instead of filtering 10K items

* removed array mutation

* fixed search

5 runs median result

Before fix:
Scenario Median Mean P95 Min Max
empty-search-all 3.42ms 3.45ms 3.77ms 3.38ms 3.93ms
empty-search-pack 0.44ms 0.44ms 0.45ms 0.43ms 0.56ms
short-search-all 3.28ms 3.28ms 3.44ms 3.25ms 3.68ms
fuse-search-all 113.20ms 113.58ms 116.92ms 110.56ms 129.55ms
fuse-search-pack 144.04ms 144.88ms 147.92ms 141.80ms 167.30ms

After fix:
Scenario Median Mean P95 Min Max
empty-search-all 0.00ms 0.01ms 0.01ms 0.00ms 0.03ms
empty-search-pack 0.13ms 0.13ms 0.15ms 0.12ms 0.30ms
short-search-all 0.00ms 0.00ms 0.01ms 0.00ms 0.01ms
fuse-search-all 114.86ms 116.00ms 125.70ms 112.07ms 135.63ms
fuse-search-pack 145.82ms 146.05ms 147.83ms 143.79ms 164.66ms

The decision of whether or not this is something we wanna go with still remains:
#209 (comment)

Maybe making it opt-in could be a solution?

I just fixed issues in current PR only following it's logic)
Of course if behavior will change we can adapt as well

nanoraptor and others added 2 commits March 28, 2026 03:59
Performance fixes for NerdFont search extension including:
- Add packIndex parameter for optimized pack filtering
- Update search configuration and Fuse options
- Enhance icon data handling and search functionality
- Add pack indexing for better performance
- Update UI logic for empty states and icon display
- Update tests

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@nanoraptor
Copy link
Copy Markdown
Author

I cannot push since it's your fork, here is patch, apply it please: nerdfont-presort-optimization.patch

What was fixed:

  • Icons are pre-sorted, before they were sorted on each filterIconIndex call
  • map lookup + slice instead of filtering 10K items
  • removed array mutation
  • fixed search

5 runs median result

Before fix:

Scenario Median Mean P95 Min Max
empty-search-all 3.42ms 3.45ms 3.77ms 3.38ms 3.93ms
empty-search-pack 0.44ms 0.44ms 0.45ms 0.43ms 0.56ms
short-search-all 3.28ms 3.28ms 3.44ms 3.25ms 3.68ms
fuse-search-all 113.20ms 113.58ms 116.92ms 110.56ms 129.55ms
fuse-search-pack 144.04ms 144.88ms 147.92ms 141.80ms 167.30ms
After fix:

Scenario Median Mean P95 Min Max
empty-search-all 0.00ms 0.01ms 0.01ms 0.00ms 0.03ms
empty-search-pack 0.13ms 0.13ms 0.15ms 0.12ms 0.30ms
short-search-all 0.00ms 0.00ms 0.01ms 0.00ms 0.01ms
fuse-search-all 114.86ms 116.00ms 125.70ms 112.07ms 135.63ms
fuse-search-pack 145.82ms 146.05ms 147.83ms 143.79ms 164.66ms

applied

@aurelleb
Copy link
Copy Markdown
Contributor

there are conflicts atm. besides, is this ready or not?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants