fix: address image picker review

apps/docs/docs/advanced/icons/index.mdx

@@ -12,7 +12,7 @@ tags:
 
 Icons in Homarr are automatically requested from multiple sources:
 
-- [Dashboard Icons](https://dashboardicons.com/) - Our recommended source with over 1888+ curated, high-quality icons
+- [Dashboard Icons](https://dashboardicons.com/) - Our recommended source with more than 1,888 curated, high-quality icons
 - [selfh.st icons](https://github.com/selfhst/icons)
 - [Simple icons](https://github.com/simple-icons/simple-icons)
 - [Tabler icons](https://tabler-icons.io/)

packages/api/src/router/icons.ts

@@ -1,4 +1,4 @@
-import { and, like } from "@homarr/db";
+import { getImageMatchRank, normalizeImageName } from "@homarr/common";
 import { icons } from "@homarr/db/schema";
 import { iconsFindSchema } from "@homarr/validation/icons";
 
@@ -15,39 +15,41 @@ export const iconsRouter = createTRPCRouter({
     })
     .input(iconsFindSchema)
     .query(async ({ ctx, input }) => {
-      const term = input.searchText?.toLowerCase().trim() ?? "";
-      const keywords = (input.searchText ?? "").split(" ").filter((keyword) => keyword.length > 0);
-
-      let whereCondition = undefined;
-      if (term.length > 0) {
-        whereCondition = and(...keywords.map((keyword) => like(icons.name, `%${keyword}%`)));
-      }
+      const term = normalizeImageName(input.searchText ?? "");
+      const repositories = await ctx.db.query.iconRepositories.findMany({
+        orderBy: (table, { asc, sql }) => [sql`CASE WHEN ${table.slug} = 'local' THEN 0 ELSE 1 END`, asc(table.slug)],
+        with: {
+          icons: {
+            columns: { id: true, name: true, url: true },
+            orderBy: (table, { asc, sql }) => [
+              sql`CASE WHEN ${table.name} LIKE '%.svg' THEN 0 ELSE 1 END`,
+              asc(table.name),
+            ],
+            limit: term.length === 0 ? input.limitPerGroup : undefined,
+          },
+        },
+      });
 
       return {
-        icons: await ctx.db.query.iconRepositories.findMany({
-          orderBy: (table, { asc, sql }) => [sql`CASE WHEN ${table.slug} = 'local' THEN 0 ELSE 1 END`, asc(table.slug)],
-          with: {
-            icons: {
-              columns: { id: true, name: true, url: true },
-              where: whereCondition,
-              orderBy: (table, { asc, sql }) => {
-                const svgFirst = sql`CASE WHEN ${table.name} LIKE '%.svg' THEN 0 ELSE 1 END`;
-                const nameAsc = asc(table.name);
-                if (term.length === 0) {
-                  return [svgFirst, nameAsc];
-                }
-                const tier = sql`CASE
-                  WHEN LOWER(${table.name}) = ${term} THEN 0
-                  WHEN LOWER(${table.name}) LIKE ${term + "%"} THEN 1
-                  WHEN LOWER(${table.name}) LIKE ${`%${term}%`} THEN 2
-                  ELSE 3
-                END`;
-                return [tier, svgFirst, nameAsc];
-              },
-              limit: input.limitPerGroup,
-            },
-          },
-        }),
+        icons:
+          term.length === 0
+            ? repositories
+            : repositories.map((repository) => ({
+                ...repository,
+                icons: repository.icons
+                  .flatMap((icon) => {
+                    const rank = getImageMatchRank(term, icon.name || icon.url);
+                    return rank === null ? [] : [{ icon, rank }];
+                  })
+                  .toSorted((a, b) =>
+                    a.rank !== b.rank
+                      ? a.rank - b.rank
+                      : Number(b.icon.name.toLowerCase().endsWith(".svg")) -
+                          Number(a.icon.name.toLowerCase().endsWith(".svg")) || a.icon.name.localeCompare(b.icon.name),
+                  )
+                  .slice(0, input.limitPerGroup)
+                  .map(({ icon }) => icon),
+              })),
         countIcons: await ctx.db.$count(icons),
       };
     }),

packages/api/src/router/test/icons.spec.ts

@@ -0,0 +1,44 @@
+import { describe, expect, test } from "vitest";
+
+import { iconRepositories, icons } from "@homarr/db/schema";
+import { createDb } from "@homarr/db/test";
+
+import { iconsRouter } from "../icons";
+
+describe("findIcons", () => {
+  test.each(["homeassistant", "home_assistant", "home assistant.png"])(
+    "matches normalized search %s before applying the group limit",
+    async (searchText) => {
+      const db = createDb();
+      const caller = iconsRouter.createCaller({ db, deviceType: undefined, session: null });
+
+      await db.insert(iconRepositories).values({ id: "repository", slug: "dashboard-icons" });
+      await db.insert(icons).values([
+        {
+          id: "irrelevant",
+          name: "000-unrelated.svg",
+          url: "https://example.com/000-unrelated.svg",
+          checksum: "irrelevant",
+          iconRepositoryId: "repository",
+        },
+        {
+          id: "home-assistant",
+          name: "home-assistant.svg",
+          url: "https://example.com/home-assistant.svg",
+          checksum: "home-assistant",
+          iconRepositoryId: "repository",
+        },
+      ]);
+
+      const result = await caller.findIcons({ searchText, limitPerGroup: 1 });
+
+      expect(result.icons[0]?.icons).toEqual([
+        {
+          id: "home-assistant",
+          name: "home-assistant.svg",
+          url: "https://example.com/home-assistant.svg",
+        },
+      ]);
+    },
+  );
+});

packages/common/src/string.ts

@@ -28,3 +28,34 @@ export const bestMatch = <TItem extends Record<string, unknown>>(
     return best;
   }, null);
 };
+
+export const normalizeImageName = (value: string) => {
+  const fileName = value.split(/[?#]/, 1)[0]?.split("/").pop() ?? value;
+  const withoutExtension = fileName.replace(/\.(?:avif|gif|jpe?g|png|svg|webp)$/i, "");
+
+  return withoutExtension
+    .normalize("NFKD")
+    .replace(/[\u0300-\u036f]/g, "")
+    .replace(/&/g, " and ")
+    .replace(/[^a-zA-Z0-9]+/g, " ")
+    .trim()
+    .toLowerCase();
+};
+
+export const getImageMatchRank = (normalizedSearch: string, candidateValue: string): number | null => {
+  const candidate = normalizeImageName(candidateValue);
+  if (!candidate) return null;
+
+  const compactSearch = normalizedSearch.replaceAll(" ", "");
+  const compactCandidate = candidate.replaceAll(" ", "");
+  if (candidate === normalizedSearch) return 0;
+  if (compactCandidate === compactSearch) return 1;
+  if (candidate.startsWith(`${normalizedSearch} `)) return 2;
+
+  const searchTokens = normalizedSearch.split(" ");
+  const candidateTokens = candidate.split(" ");
+  if (searchTokens.every((term) => candidateTokens.some((token) => token === term))) return 3;
+  if (searchTokens.every((term) => candidateTokens.some((token) => token.startsWith(term)))) return 4;
+  if (candidate.includes(normalizedSearch)) return 5;
+  return null;
+};

packages/common/src/test/string.spec.ts

@@ -1,6 +1,6 @@
 import { describe, expect, test } from "vitest";
 
-import { bestMatch, capitalize } from "../string";
+import { bestMatch, capitalize, getImageMatchRank, normalizeImageName } from "../string";
 
 const capitalizeTestCases = [
   ["hello", "Hello"],
@@ -54,3 +54,23 @@ describe("bestMatch should find the best match in an array of options", () => {
     expect(result).toBeNull();
   });
 });
+
+describe("image name matching", () => {
+  test.each([
+    ["home-assistant.svg", "home assistant"],
+    ["https://example.com/Home_Assistant.PNG?v=1", "home assistant"],
+    ["jelly.fin.webp", "jelly fin"],
+    ["Caf\u00e9.svg", "cafe"],
+  ])("normalizes %s", (value, expected) => {
+    expect(normalizeImageName(value)).toBe(expected);
+  });
+
+  test.each([
+    ["homeassistant", "home-assistant.svg", 1],
+    ["home assistant", "home_assistant.png", 0],
+    ["home assistant", "Home Assistant.webp", 0],
+    ["home assistant", "unrelated.svg", null],
+  ])("ranks %s against %s", (search, candidate, expected) => {
+    expect(getImageMatchRank(normalizeImageName(search), candidate)).toBe(expected);
+  });
+});

packages/forms-collection/src/icon-picker/icon-picker.tsx

@@ -76,7 +76,8 @@ export const IconPicker = ({
   const [debouncedDraft] = useDebouncedValue(draft, 250);
 
   const directUrl = isDirectImageUrl(draft) ? draft.trim() : null;
-  const searchText = directUrl || debouncedDraft === (value ?? "") ? "" : debouncedDraft.trim();
+  const isCommittedValueDraft = debouncedDraft === (value ?? "");
+  const searchText = directUrl !== null || isCommittedValueDraft ? "" : debouncedDraft.trim();
   const canUpload = session?.user.permissions.includes("media-upload") ?? false;
 
   const query = clientApi.icon.findIcons.useQuery(
@@ -283,92 +284,91 @@ export const IconPicker = ({
       }}
     >
       <Combobox.Target withExpandedAttribute>
-        <Group wrap="nowrap" gap="xs" w="100%" align="start">
-          <InputBase
-            flex={1}
-            leftSection={leftSection}
-            rightSection={
-              <Group gap={2} wrap="nowrap">
-                {(draft || value) && (
-                  <CloseButton
-                    size="sm"
-                    aria-label={tCommon("iconPicker.clear")}
-                    onMouseDown={(event) => event.preventDefault()}
-                    onClick={clearValue}
-                  />
-                )}
-                {query.isFetching && !directUrl ? <Loader size={14} /> : <Combobox.Chevron />}
-              </Group>
-            }
-            rightSectionWidth={56}
-            rightSectionPointerEvents="all"
-            value={draft}
-            onChange={(event) => {
-              const nextValue = event.currentTarget.value;
-              setDraft(nextValue);
-              setHasEdited(true);
-              setKeyboardIndex(-1);
-              combobox.openDropdown("keyboard");
-              combobox.updateSelectedOptionIndex();
-
-              if (!nextValue) {
-                setValue("");
-              } else if (isImageSource(nextValue)) {
-                setValue(nextValue.trim());
-              }
-            }}
-            onKeyDown={handleKeyDown}
-            onClick={openWithSuggestedSearch}
-            onFocus={(event) => {
-              onFocus?.(event);
-              openWithSuggestedSearch();
-            }}
-            onBlur={(event) => {
-              onBlur?.(event);
-              combobox.closeDropdown();
-              setDraft(value ?? "");
-              setHasEdited(false);
-            }}
-            withAsterisk={withAsterisk}
-            error={error}
-            label={label ?? tCommon("iconPicker.label")}
-            description={tCommon("iconPicker.description")}
-            styles={{
-              description: { color: "var(--mantine-color-text)" },
-              input: { color: "var(--mantine-color-text)" },
-              error: { color: "light-dark(var(--mantine-color-red-9), var(--mantine-color-red-2))" },
-            }}
-            placeholder={placeholderText}
-            autoComplete="off"
-            spellCheck={false}
-          />
-
-          {canUpload && (
-            <UploadMedia
-              onSuccess={(medias) => {
-                const first = medias.at(0);
-                if (!first) return;
-                commitValue(first.url);
-                combobox.closeDropdown();
-              }}
-            >
-              {({ onClick, loading }) => (
-                <Tooltip label={tCommon("iconPicker.uploadImage")}>
-                  <ActionIcon
-                    onClick={onClick}
-                    loading={loading}
-                    mt={46}
-                    size={36}
-                    variant="default"
-                    aria-label={tCommon("iconPicker.uploadImage")}
-                  >
-                    <IconUpload size={17} stroke={1.5} />
-                  </ActionIcon>
-                </Tooltip>
+        <InputBase
+          flex={1}
+          leftSection={leftSection}
+          rightSection={
+            <Group gap={2} wrap="nowrap">
+              {canUpload && (
+                <UploadMedia
+                  onSuccess={(medias) => {
+                    const first = medias.at(0);
+                    if (!first) return;
+                    commitValue(first.url);
+                    combobox.closeDropdown();
+                  }}
+                >
+                  {({ onClick, loading }) => (
+                    <Tooltip label={tCommon("iconPicker.uploadImage")}>
+                      <ActionIcon
+                        onClick={(event) => {
+                          event.stopPropagation();
+                          onClick();
+                        }}
+                        loading={loading}
+                        size="sm"
+                        variant="subtle"
+                        aria-label={tCommon("iconPicker.uploadImage")}
+                      >
+                        <IconUpload size={16} stroke={1.5} />
+                      </ActionIcon>
+                    </Tooltip>
+                  )}
+                </UploadMedia>
               )}
-            </UploadMedia>
-          )}
-        </Group>
+              {(draft || value) && (
+                <CloseButton
+                  size="sm"
+                  aria-label={tCommon("iconPicker.clear")}
+                  onMouseDown={(event) => event.preventDefault()}
+                  onClick={clearValue}
+                />
+              )}
+              {query.isFetching && !directUrl ? <Loader size={14} /> : <Combobox.Chevron />}
+            </Group>
+          }
+          rightSectionWidth={canUpload ? 86 : 56}
+          rightSectionPointerEvents="all"
+          value={draft}
+          onChange={(event) => {
+            const nextValue = event.currentTarget.value;
+            setDraft(nextValue);
+            setHasEdited(true);
+            setKeyboardIndex(-1);
+            combobox.openDropdown("keyboard");
+            combobox.updateSelectedOptionIndex();
+
+            if (!nextValue) {
+              setValue("");
+            } else if (isImageSource(nextValue)) {
+              setValue(nextValue.trim());
+            }
+          }}
+          onKeyDown={handleKeyDown}
+          onClick={openWithSuggestedSearch}
+          onFocus={(event) => {
+            onFocus?.(event);
+            openWithSuggestedSearch();
+          }}
+          onBlur={(event) => {
+            onBlur?.(event);
+            combobox.closeDropdown();
+            setDraft(value ?? "");
+            setHasEdited(false);
+          }}
+          withAsterisk={withAsterisk}
+          error={error}
+          label={label ?? tCommon("iconPicker.label")}
+          description={tCommon("iconPicker.description")}
+          styles={{
+            description: { color: "var(--mantine-color-text)" },
+            input: { color: "var(--mantine-color-text)" },
+            error: { color: "light-dark(var(--mantine-color-red-9), var(--mantine-color-red-2))" },
+          }}
+          placeholder={placeholderText}
+          autoComplete="off"
+          spellCheck={false}
+        />
       </Combobox.Target>
 
       <Combobox.Dropdown className={classes.dropdown}>

packages/forms-collection/src/icon-picker/icon-picker.utils.spec.ts

@@ -1,13 +1,7 @@
 import { describe, expect, test } from "vitest";
 
 import type { IconGroup } from "./icon-picker.utils";
-import {
-  arrangeIconPickerSections,
-  findBestIconMatch,
-  isDirectImageUrl,
-  isImageSource,
-  isSvgImage,
-} from "./icon-picker.utils";
+import { arrangeIconPickerSections, isDirectImageUrl, isImageSource, isSvgImage } from "./icon-picker.utils";
 
 const groups = [
   {
@@ -35,26 +29,6 @@ describe("icon picker utilities", () => {
     expect(result.other.map((icon) => icon.id)).toEqual(["raster"]);
   });
 
-  test("normalizes punctuation and prefers a user's exact local image", () => {
-    expect(findBestIconMatch("home_assistant", groups)).toBe("/api/user-medias/home-assistant");
-  });
-
-  test("prefers SVG over raster when equally matched outside local images", () => {
-    expect(findBestIconMatch("Home Assistant", [groups[0] as IconGroup])).toBe(
-      "https://cdn.example.com/home-assistant.svg?v=1",
-    );
-  });
-
-  test("prefers an exact normalized name over a longer partial match", () => {
-    expect(findBestIconMatch("home assistant", [groups[0] as IconGroup])).toBe(
-      "https://cdn.example.com/home-assistant.svg?v=1",
-    );
-  });
-
-  test("does not return unrelated icons", () => {
-    expect(findBestIconMatch("jellyfin", groups)).toBeNull();
-  });
-
   test.each([
     ["https://example.com/icon.png", true],
     ["http://localhost:3000/icon", true],

packages/forms-collection/src/icon-picker/icon-picker.utils.ts

@@ -1,6 +1,7 @@
 import type { inferRouterOutputs } from "@trpc/server";
 
 import type { AppRouter } from "@homarr/api";
+import { getImageMatchRank, normalizeImageName } from "@homarr/common";
 
 type RouterOutput = inferRouterOutputs<AppRouter>;
 export type IconGroup = RouterOutput["icon"]["findIcons"]["icons"][number];
@@ -31,8 +32,8 @@ export const arrangeIconPickerSections = (iconGroups: IconGroup[], searchTerm =
   const normalizedSearch = normalizeImageName(searchTerm);
   if (normalizedSearch) {
     const compareMatches = (a: PickerIcon, b: PickerIcon) => {
-      const aRank = getMatchRank(normalizedSearch, a.name || a.url) ?? Number.MAX_SAFE_INTEGER;
-      const bRank = getMatchRank(normalizedSearch, b.name || b.url) ?? Number.MAX_SAFE_INTEGER;
+      const aRank = getImageMatchRank(normalizedSearch, a.name || a.url) ?? Number.MAX_SAFE_INTEGER;
+      const bRank = getImageMatchRank(normalizedSearch, b.name || b.url) ?? Number.MAX_SAFE_INTEGER;
       return aRank - bRank || a.name.localeCompare(b.name);
     };
     return {
@@ -45,37 +46,6 @@ export const arrangeIconPickerSections = (iconGroups: IconGroup[], searchTerm =
   return sections;
 };
 
-export const findBestIconMatch = (searchTerm: string, iconGroups: IconGroup[]): string | null => {
-  const normalizedSearch = normalizeImageName(searchTerm);
-  if (!normalizedSearch) return null;
-
-  const rankedIcons = iconGroups
-    .flatMap((group) =>
-      group.icons.map((icon) => ({
-        icon,
-        group,
-        rank: getMatchRank(normalizedSearch, icon.name || icon.url),
-      })),
-    )
-    .filter((entry) => entry.rank !== null)
-    .toSorted((a, b) => {
-      const quality = (a.rank ?? Number.MAX_SAFE_INTEGER) - (b.rank ?? Number.MAX_SAFE_INTEGER);
-      if (quality !== 0) return quality;
-
-      const local = Number(b.group.slug === "local") - Number(a.group.slug === "local");
-      if (local !== 0) return local;
-
-      const svg = Number(isSvgImage(b.icon.url)) - Number(isSvgImage(a.icon.url));
-      if (svg !== 0) return svg;
-
-      return (
-        normalizeImageName(a.icon.name || a.icon.url).length - normalizeImageName(b.icon.name || b.icon.url).length
-      );
-    });
-
-  return rankedIcons.at(0)?.icon.url ?? null;
-};
-
 export const isDirectImageUrl = (value: string) => {
   try {
     const url = new URL(value.trim());
@@ -95,34 +65,3 @@ export const isSvgImage = (value: string) => {
     return value.toLowerCase().split(/[?#]/, 1)[0]?.endsWith(".svg") ?? false;
   }
 };
-
-const normalizeImageName = (value: string) => {
-  const fileName = value.split(/[?#]/, 1)[0]?.split("/").pop() ?? value;
-  const withoutExtension = fileName.replace(/\.(?:avif|gif|jpe?g|png|svg|webp)$/i, "");
-
-  return withoutExtension
-    .normalize("NFKD")
-    .replace(/[\u0300-\u036f]/g, "")
-    .replace(/&/g, " and ")
-    .replace(/[^a-zA-Z0-9]+/g, " ")
-    .trim()
-    .toLowerCase();
-};
-
-const getMatchRank = (search: string, candidateValue: string): number | null => {
-  const candidate = normalizeImageName(candidateValue);
-  if (!candidate) return null;
-
-  const compactSearch = search.replaceAll(" ", "");
-  const compactCandidate = candidate.replaceAll(" ", "");
-  if (candidate === search) return 0;
-  if (compactCandidate === compactSearch) return 1;
-  if (candidate.startsWith(`${search} `)) return 2;
-
-  const searchTokens = search.split(" ");
-  const candidateTokens = candidate.split(" ");
-  if (searchTokens.every((term) => candidateTokens.some((token) => token === term))) return 3;
-  if (searchTokens.every((term) => candidateTokens.some((token) => token.startsWith(term)))) return 4;
-  if (candidate.includes(search)) return 5;
-  return null;
-};

packages/forms-collection/src/index.tsx

@@ -2,7 +2,6 @@ export * from "./new-app/_app-new-form";
 export * from "./new-app/_form";
 
 export * from "./icon-picker/icon-picker";
-export * from "./new-app/icon-matcher";
 
 export * from "./upload-media/upload-media";
 

packages/forms-collection/src/new-app/icon-matcher.ts

@@ -1 +0,0 @@
-export { findBestIconMatch } from "../icon-picker/icon-picker.utils";