From 7b63c60675ce10d68bdae477d6549506205134f2 Mon Sep 17 00:00:00 2001 From: ajnart Date: Tue, 11 Aug 2026 11:58:45 +0200 Subject: [PATCH] fix: address image picker review --- apps/docs/docs/advanced/icons/index.mdx | 2 +- packages/api/src/router/icons.ts | 66 +++---- packages/api/src/router/test/icons.spec.ts | 44 +++++ packages/common/src/string.ts | 31 ++++ packages/common/src/test/string.spec.ts | 22 ++- .../src/icon-picker/icon-picker.tsx | 172 +++++++++--------- .../src/icon-picker/icon-picker.utils.spec.ts | 28 +-- .../src/icon-picker/icon-picker.utils.ts | 67 +------ packages/forms-collection/src/index.tsx | 1 - .../src/new-app/icon-matcher.ts | 1 - 10 files changed, 221 insertions(+), 213 deletions(-) create mode 100644 packages/api/src/router/test/icons.spec.ts delete mode 100644 packages/forms-collection/src/new-app/icon-matcher.ts diff --git a/apps/docs/docs/advanced/icons/index.mdx b/apps/docs/docs/advanced/icons/index.mdx index 01cd4b166..0931c6e43 100644 --- a/apps/docs/docs/advanced/icons/index.mdx +++ b/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/) diff --git a/packages/api/src/router/icons.ts b/packages/api/src/router/icons.ts index fc1897a36..fcd4c8c3f 100644 --- a/packages/api/src/router/icons.ts +++ b/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), }; }), diff --git a/packages/api/src/router/test/icons.spec.ts b/packages/api/src/router/test/icons.spec.ts new file mode 100644 index 000000000..d4b85f6f1 --- /dev/null +++ b/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", + }, + ]); + }, + ); +}); diff --git a/packages/common/src/string.ts b/packages/common/src/string.ts index 178cb9a47..246e2ac4a 100644 --- a/packages/common/src/string.ts +++ b/packages/common/src/string.ts @@ -28,3 +28,34 @@ export const bestMatch = >( 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; +}; diff --git a/packages/common/src/test/string.spec.ts b/packages/common/src/test/string.spec.ts index d1bc1f9d0..acc8bb907 100644 --- a/packages/common/src/test/string.spec.ts +++ b/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); + }); +}); diff --git a/packages/forms-collection/src/icon-picker/icon-picker.tsx b/packages/forms-collection/src/icon-picker/icon-picker.tsx index 940a1673f..b0d43f0ae 100644 --- a/packages/forms-collection/src/icon-picker/icon-picker.tsx +++ b/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 = ({ }} > - - - {(draft || value) && ( - event.preventDefault()} - onClick={clearValue} - /> - )} - {query.isFetching && !directUrl ? : } - - } - 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 && ( - { - const first = medias.at(0); - if (!first) return; - commitValue(first.url); - combobox.closeDropdown(); - }} - > - {({ onClick, loading }) => ( - - - - - + + {canUpload && ( + { + const first = medias.at(0); + if (!first) return; + commitValue(first.url); + combobox.closeDropdown(); + }} + > + {({ onClick, loading }) => ( + + { + event.stopPropagation(); + onClick(); + }} + loading={loading} + size="sm" + variant="subtle" + aria-label={tCommon("iconPicker.uploadImage")} + > + + + + )} + )} - - )} - + {(draft || value) && ( + event.preventDefault()} + onClick={clearValue} + /> + )} + {query.isFetching && !directUrl ? : } + + } + 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} + /> diff --git a/packages/forms-collection/src/icon-picker/icon-picker.utils.spec.ts b/packages/forms-collection/src/icon-picker/icon-picker.utils.spec.ts index 33a251683..562b43cba 100644 --- a/packages/forms-collection/src/icon-picker/icon-picker.utils.spec.ts +++ b/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], diff --git a/packages/forms-collection/src/icon-picker/icon-picker.utils.ts b/packages/forms-collection/src/icon-picker/icon-picker.utils.ts index 83690b43b..e71558f8e 100644 --- a/packages/forms-collection/src/icon-picker/icon-picker.utils.ts +++ b/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; 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; -}; diff --git a/packages/forms-collection/src/index.tsx b/packages/forms-collection/src/index.tsx index 7621bed43..0dfaf90ca 100644 --- a/packages/forms-collection/src/index.tsx +++ b/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"; diff --git a/packages/forms-collection/src/new-app/icon-matcher.ts b/packages/forms-collection/src/new-app/icon-matcher.ts deleted file mode 100644 index 68c54726f..000000000 --- a/packages/forms-collection/src/new-app/icon-matcher.ts +++ /dev/null @@ -1 +0,0 @@ -export { findBestIconMatch } from "../icon-picker/icon-picker.utils";