From a64b619bee8879c2ac710a06af80763fcc77abb7 Mon Sep 17 00:00:00 2001 From: Joshua Date: Sat, 26 Sep 2026 07:33:18 -0400 Subject: [PATCH] Resolve namespaced Lucide icons in the admin lookup and picker The API now returns definition icons as lucide:, but the admin looked up and compared bare names only, so every icon rendered as the generic fallback and nothing was selected in the edit picker (NQ-241-1). Add normalizeDefinitionIconName as the one rule for both surfaces: lucide: and a legacy bare give ; fa:, emoji: and unknown namespaces give null (the fallback tile). Tapping a Lucide choice now stores the canonical lucide:; No icon still clears. Task 707994b7. Co-Authored-By: Claude Opus 5.5 --- admin/src/definitions/DefinitionsTab.test.tsx | 94 ++++++++++++++++++- admin/src/definitions/DefinitionsTab.tsx | 38 +++++--- admin/src/definitions/definitionIcons.tsx | 24 ++++- 3 files changed, 136 insertions(+), 20 deletions(-) diff --git a/admin/src/definitions/DefinitionsTab.test.tsx b/admin/src/definitions/DefinitionsTab.test.tsx index 0db0d5d..7f5ef3e 100644 --- a/admin/src/definitions/DefinitionsTab.test.tsx +++ b/admin/src/definitions/DefinitionsTab.test.tsx @@ -1,7 +1,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { act, cleanup, fireEvent, render, screen, waitFor, within } from "@testing-library/react"; import DefinitionsTab from "./DefinitionsTab"; -import { DEFINITION_ICONS, DefinitionIcon } from "./definitionIcons"; +import { DEFINITION_ICONS, DefinitionIcon, normalizeDefinitionIconName } from "./definitionIcons"; import type { AdminChild, DefinitionRule, QuestDefinition } from "../api/definitions"; import { storeTokens } from "../auth/oidc"; @@ -1052,7 +1052,7 @@ describe("DefinitionsTab icon picker", () => { expect(api.writes()).toHaveLength(1); expect(api.writes()[0].method).toBe("POST"); - expect((api.writes()[0].body as { icon: string }).icon).toBe("bed"); + expect((api.writes()[0].body as { icon: string }).icon).toBe("lucide:bed"); expect(rowIcon(await screen.findByTestId("definition-99"))).toBe("bed"); }); @@ -1073,7 +1073,7 @@ describe("DefinitionsTab icon picker", () => { const { method, path, body } = api.writes()[0]; expect(method).toBe("PATCH"); expect(path).toBe("/api/v1/admin/quest-definitions/11"); - expect((body as { icon: string }).icon).toBe("trash-2"); + expect((body as { icon: string }).icon).toBe("lucide:trash-2"); // Inactive "Old" stays assigned: the unchanged selection is still omitted. expect(body).not.toHaveProperty("assignee_child_ids"); await waitFor(() => expect(rowIcon(screen.getByTestId("definition-11"))).toBe("trash-2")); @@ -1119,6 +1119,94 @@ describe("DefinitionsTab icon picker", () => { }); }); +describe("DefinitionsTab namespaced icons", () => { + beforeEach(() => { + sessionStorage.clear(); + storeTokens({ access_token: "at-123", expires_in: 600 }); + }); + + afterEach(() => { + cleanup(); + vi.unstubAllGlobals(); + vi.useRealTimers(); + }); + + function rowIcon(row: HTMLElement): string | null { + const svg = row.querySelector(".defs-row-glyph svg"); + expect(svg).not.toBeNull(); + return svg!.getAttribute("data-icon"); + } + + function iconRadio(s: HTMLElement, label: string): HTMLElement { + return within(within(s).getByRole("radiogroup", { name: "Icon" })).getByRole("radio", { + name: label, + }); + } + + it("normalizes lucide-namespaced and legacy bare values to one Lucide name", () => { + expect(normalizeDefinitionIconName("lucide:dog")).toBe("dog"); + expect(normalizeDefinitionIconName("dog")).toBe("dog"); + expect(normalizeDefinitionIconName("fa:dog")).toBeNull(); + expect(normalizeDefinitionIconName("emoji:๐Ÿถ")).toBeNull(); + expect(normalizeDefinitionIconName("mystery:dog")).toBeNull(); + expect(normalizeDefinitionIconName(null)).toBeNull(); + }); + + for (const icon of ["lucide:dog", "dog"]) { + it(`renders and selects the Dog glyph for ${icon}`, async () => { + await renderReady([{ ...BRUSH, id: 30, icon }]); + const row = screen.getByTestId("definition-30"); + expect(rowIcon(row)).toBe("dog"); + + fireEvent.click(row); + const s = sheet(); + expect(iconRadio(s, "Dog").getAttribute("aria-checked")).toBe("true"); + expect(iconRadio(s, "Dog").className).toContain("defs-icon--on"); + expect(iconRadio(s, "No icon").getAttribute("aria-checked")).toBe("false"); + }); + } + + for (const icon of ["fa:dog", "emoji:๐Ÿถ", "mystery:dog"]) { + it(`renders the fallback tile and selects nothing for ${icon}`, async () => { + await renderReady([{ ...BRUSH, id: 31, icon }]); + const row = screen.getByTestId("definition-31"); + expect(rowIcon(row)).toBeNull(); + + fireEvent.click(row); + const s = sheet(); + const group = within(s).getByRole("radiogroup", { name: "Icon" }); + for (const radio of within(group).getAllByRole("radio")) { + expect(radio.getAttribute("aria-checked")).toBe("false"); + } + }); + } + + it("tapping a Lucide choice on a namespaced task PATCHes lucide:, and No icon clears it", async () => { + await renderReady([{ ...BRUSH, id: 32, icon: "lucide:dog" }]); + fireEvent.click(screen.getByTestId("definition-32")); + let s = sheet(); + fireEvent.click(iconRadio(s, "Cat")); + expect(iconRadio(s, "Cat").getAttribute("aria-checked")).toBe("true"); + expect(iconRadio(s, "Dog").getAttribute("aria-checked")).toBe("false"); + + await act(async () => { + fireEvent.click(within(s).getByRole("button", { name: "Save changes" })); + }); + expect((api.writes()[0].body as { icon: string }).icon).toBe("lucide:cat"); + await waitFor(() => expect(rowIcon(screen.getByTestId("definition-32"))).toBe("cat")); + + fireEvent.click(screen.getByTestId("definition-32")); + s = sheet(); + expect(iconRadio(s, "Cat").getAttribute("aria-checked")).toBe("true"); + fireEvent.click(iconRadio(s, "No icon")); + expect(iconRadio(s, "No icon").getAttribute("aria-checked")).toBe("true"); + await act(async () => { + fireEvent.click(within(s).getByRole("button", { name: "Save changes" })); + }); + expect((api.writes()[1].body as { icon: string | null }).icon).toBeNull(); + }); +}); + /* โ”€โ”€ Start date โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ */ function startDateInput(s: HTMLElement): HTMLInputElement { diff --git a/admin/src/definitions/DefinitionsTab.tsx b/admin/src/definitions/DefinitionsTab.tsx index dcbb698..bb5a7ea 100644 --- a/admin/src/definitions/DefinitionsTab.tsx +++ b/admin/src/definitions/DefinitionsTab.tsx @@ -43,7 +43,12 @@ import { PlusGlyph, TaskGlyph, } from "./glyphs"; -import { DEFINITION_ICONS, DefinitionIcon } from "./definitionIcons"; +import { + DEFINITION_ICONS, + DefinitionIcon, + lucideIconKey, + normalizeDefinitionIconName, +} from "./definitionIcons"; import "./DefinitionsTab.css"; /** ADMIN-SPEC ยง1: every scroll column ends with 92px so content clears the tab bar. */ @@ -576,20 +581,23 @@ function EditSheet({ > - {DEFINITION_ICONS.map(({ name, label, Icon }) => ( - - ))} + {DEFINITION_ICONS.map(({ name, label, Icon }) => { + const on = normalizeDefinitionIconName(draft.icon) === name; + return ( + + ); + })} diff --git a/admin/src/definitions/definitionIcons.tsx b/admin/src/definitions/definitionIcons.tsx index dbf0a3d..484a430 100644 --- a/admin/src/definitions/definitionIcons.tsx +++ b/admin/src/definitions/definitionIcons.tsx @@ -1,7 +1,8 @@ /** * The shipped Lucide subset a definition's `icon` may name (ADMIN-SPEC ยง3.3). * Only these named imports are bundled โ€” no CDN, no runtime fetch. The stored - * value is the Lucide kebab-case name, so it stays meaningful to any surface. + * value is the namespaced `lucide:`; a legacy bare `` + * still names the same Lucide entry. */ import { Apple, @@ -91,7 +92,26 @@ export const DEFINITION_ICONS: readonly DefinitionIconEntry[] = [ const BY_NAME = new Map(DEFINITION_ICONS.map((entry) => [entry.name, entry])); -export function findDefinitionIcon(name: string | null): DefinitionIconEntry | null { +const LUCIDE_PREFIX = "lucide:"; + +/** The canonical stored key for a Lucide choice, e.g. `lucide:dog`. */ +export function lucideIconKey(name: string): string { + return `${LUCIDE_PREFIX}${name}`; +} + +/** + * The Lucide name an icon value refers to: `lucide:dog` and a legacy bare + * `dog` both give `dog`. Any other namespace (`fa:`, `emoji:`, unknown) and + * null give null โ€” the admin has no Lucide entry for them. + */ +export function normalizeDefinitionIconName(value: string | null): string | null { + if (value === null) return null; + if (value.startsWith(LUCIDE_PREFIX)) return value.slice(LUCIDE_PREFIX.length); + return value.includes(":") ? null : value; +} + +export function findDefinitionIcon(value: string | null): DefinitionIconEntry | null { + const name = normalizeDefinitionIconName(value); return name === null ? null : (BY_NAME.get(name) ?? null); }