Resolve namespaced Lucide icons in the admin lookup and picker
The API now returns definition icons as lucide:<name>, 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:<name> and a legacy bare <name> give <name>; fa:, emoji: and unknown namespaces give null (the fallback tile). Tapping a Lucide choice now stores the canonical lucide:<name>; No icon still clears. Task 707994b7. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -1,7 +1,7 @@
|
|||||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||||
import { act, cleanup, fireEvent, render, screen, waitFor, within } from "@testing-library/react";
|
import { act, cleanup, fireEvent, render, screen, waitFor, within } from "@testing-library/react";
|
||||||
import DefinitionsTab from "./DefinitionsTab";
|
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 type { AdminChild, DefinitionRule, QuestDefinition } from "../api/definitions";
|
||||||
import { storeTokens } from "../auth/oidc";
|
import { storeTokens } from "../auth/oidc";
|
||||||
|
|
||||||
@@ -1052,7 +1052,7 @@ describe("DefinitionsTab icon picker", () => {
|
|||||||
|
|
||||||
expect(api.writes()).toHaveLength(1);
|
expect(api.writes()).toHaveLength(1);
|
||||||
expect(api.writes()[0].method).toBe("POST");
|
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");
|
expect(rowIcon(await screen.findByTestId("definition-99"))).toBe("bed");
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -1073,7 +1073,7 @@ describe("DefinitionsTab icon picker", () => {
|
|||||||
const { method, path, body } = api.writes()[0];
|
const { method, path, body } = api.writes()[0];
|
||||||
expect(method).toBe("PATCH");
|
expect(method).toBe("PATCH");
|
||||||
expect(path).toBe("/api/v1/admin/quest-definitions/11");
|
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.
|
// Inactive "Old" stays assigned: the unchanged selection is still omitted.
|
||||||
expect(body).not.toHaveProperty("assignee_child_ids");
|
expect(body).not.toHaveProperty("assignee_child_ids");
|
||||||
await waitFor(() => expect(rowIcon(screen.getByTestId("definition-11"))).toBe("trash-2"));
|
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:<name>, 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 ─────────────────────────────────────────────────────── */
|
/* ── Start date ─────────────────────────────────────────────────────── */
|
||||||
|
|
||||||
function startDateInput(s: HTMLElement): HTMLInputElement {
|
function startDateInput(s: HTMLElement): HTMLInputElement {
|
||||||
|
|||||||
@@ -43,7 +43,12 @@ import {
|
|||||||
PlusGlyph,
|
PlusGlyph,
|
||||||
TaskGlyph,
|
TaskGlyph,
|
||||||
} from "./glyphs";
|
} from "./glyphs";
|
||||||
import { DEFINITION_ICONS, DefinitionIcon } from "./definitionIcons";
|
import {
|
||||||
|
DEFINITION_ICONS,
|
||||||
|
DefinitionIcon,
|
||||||
|
lucideIconKey,
|
||||||
|
normalizeDefinitionIconName,
|
||||||
|
} from "./definitionIcons";
|
||||||
import "./DefinitionsTab.css";
|
import "./DefinitionsTab.css";
|
||||||
|
|
||||||
/** ADMIN-SPEC §1: every scroll column ends with 92px so content clears the tab bar. */
|
/** ADMIN-SPEC §1: every scroll column ends with 92px so content clears the tab bar. */
|
||||||
@@ -576,20 +581,23 @@ function EditSheet({
|
|||||||
>
|
>
|
||||||
<TaskGlyph size={18} />
|
<TaskGlyph size={18} />
|
||||||
</button>
|
</button>
|
||||||
{DEFINITION_ICONS.map(({ name, label, Icon }) => (
|
{DEFINITION_ICONS.map(({ name, label, Icon }) => {
|
||||||
<button
|
const on = normalizeDefinitionIconName(draft.icon) === name;
|
||||||
key={name}
|
return (
|
||||||
type="button"
|
<button
|
||||||
role="radio"
|
key={name}
|
||||||
aria-checked={draft.icon === name}
|
type="button"
|
||||||
aria-label={label}
|
role="radio"
|
||||||
data-icon-option={name}
|
aria-checked={on}
|
||||||
className={draft.icon === name ? "defs-icon defs-icon--on" : "defs-icon"}
|
aria-label={label}
|
||||||
onClick={() => update({ icon: name })}
|
data-icon-option={name}
|
||||||
>
|
className={on ? "defs-icon defs-icon--on" : "defs-icon"}
|
||||||
<Icon size={18} aria-hidden="true" focusable="false" />
|
onClick={() => update({ icon: lucideIconKey(name) })}
|
||||||
</button>
|
>
|
||||||
))}
|
<Icon size={18} aria-hidden="true" focusable="false" />
|
||||||
|
</button>
|
||||||
|
);
|
||||||
|
})}
|
||||||
</div>
|
</div>
|
||||||
</Field>
|
</Field>
|
||||||
|
|
||||||
|
|||||||
@@ -1,7 +1,8 @@
|
|||||||
/**
|
/**
|
||||||
* The shipped Lucide subset a definition's `icon` may name (ADMIN-SPEC §3.3).
|
* 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
|
* 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:<kebab-name>`; a legacy bare `<kebab-name>`
|
||||||
|
* still names the same Lucide entry.
|
||||||
*/
|
*/
|
||||||
import {
|
import {
|
||||||
Apple,
|
Apple,
|
||||||
@@ -91,7 +92,26 @@ export const DEFINITION_ICONS: readonly DefinitionIconEntry[] = [
|
|||||||
|
|
||||||
const BY_NAME = new Map(DEFINITION_ICONS.map((entry) => [entry.name, entry]));
|
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);
|
return name === null ? null : (BY_NAME.get(name) ?? null);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user