From 9355bcedae46e23be69a566b6c9cc627310e01e0 Mon Sep 17 00:00:00 2001 From: Cedric Vidal Date: Tue, 20 Oct 2026 23:50:40 -0700 Subject: [PATCH] refactor(portal): share navigation and list section icons Adapt the generic navigation/header-icon changes from the preserved review branch onto main without the Experiment foundation dependency. Keep existing navigation behavior and generic fallbacks, and cover the registry, list headers, and stories. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 25ed0301-e18f-4fc8-a879-6eeb3879418c --- apps/portal/src/components/Layout.stories.tsx | 9 ++ apps/portal/src/components/Layout.test.tsx | 32 +++++++ apps/portal/src/components/Layout.tsx | 52 ++++-------- .../list-layout/ListLayout.stories.tsx | 66 +++++++++++++++ .../list-layout/ListLayout.test.tsx | 84 ++++++++++++++++++- .../src/components/list-layout/ListLayout.tsx | 6 +- .../src/components/navigation-icons.test.ts | 38 +++++++++ .../portal/src/components/navigation-icons.ts | 74 ++++++++++++++++ docs/architecture/app-design.md | 9 ++ 9 files changed, 331 insertions(+), 39 deletions(-) create mode 100644 apps/portal/src/components/list-layout/ListLayout.stories.tsx create mode 100644 apps/portal/src/components/navigation-icons.test.ts create mode 100644 apps/portal/src/components/navigation-icons.ts diff --git a/apps/portal/src/components/Layout.stories.tsx b/apps/portal/src/components/Layout.stories.tsx index fe59b9f18..c12ddcefb 100644 --- a/apps/portal/src/components/Layout.stories.tsx +++ b/apps/portal/src/components/Layout.stories.tsx @@ -41,6 +41,12 @@ export const Collapsed: Story = { await expect( canvas.getByText(/This is an AI evaluation platform\./), ).toBeVisible(); + await expect( + canvas.getByRole("link", { name: "Runs" }).querySelector(".lucide-list"), + ).toBeTruthy(); + await expect( + canvas.getByRole("link", { name: "Reports" }).querySelector(".lucide-file-text"), + ).toBeTruthy(); }, }; @@ -51,6 +57,9 @@ export const Expanded: Story = { await expect(canvas.getByRole("button", { name: "Collapse sidebar" })).toBeVisible(); // With a project selected, scoped nav is present. await expect(canvas.getByRole("link", { name: "Runs" })).toBeVisible(); + await expect( + canvas.getByRole("link", { name: "Runs" }).querySelector(".lucide-list"), + ).toBeTruthy(); }, }; diff --git a/apps/portal/src/components/Layout.test.tsx b/apps/portal/src/components/Layout.test.tsx index 755a9cdf9..e180794bc 100644 --- a/apps/portal/src/components/Layout.test.tsx +++ b/apps/portal/src/components/Layout.test.tsx @@ -78,6 +78,38 @@ afterEach(() => { }); describe("Layout", () => { + it.each(["0", "1"])("preserves section icons with sidebar expanded=%s", (expanded) => { + localStorage.setItem("scope:layout:sidebar-expanded", expanded); + renderLayout(); + + const sidebar = within(screen.getByLabelText("Primary navigation")); + const sections = [ + ["Projects", "folder-kanban"], + ["Runs", "list"], + ["Statistics", "chart-column"], + ["Reports", "file-text"], + ["Insights", "lightbulb"], + ["Prompts", "message-square-text"], + ["Criteria", "flask-conical"], + ["Features", "tags"], + ["Profiles", "sliders-horizontal"], + ["Skills", "book-open"], + ["Codebases", "folder-git2"], + ["Resources", "boxes"], + ["MCP", "server"], + ["Extensions", "puzzle"], + ["Agents", "bot"], + ["Models", "cpu"], + ["Secrets", "key-round"], + ]; + for (const [label, icon] of sections) { + expect( + sidebar.getByRole("link", { name: label }).querySelector(`.lucide-${icon}`), + label, + ).toBeTruthy(); + } + }); + it("keeps the full-bleed shell when opening and closing a key preview", () => { renderLayout("/secrets/keys"); diff --git a/apps/portal/src/components/Layout.tsx b/apps/portal/src/components/Layout.tsx index 2a08d36cd..1c2f7cc8b 100644 --- a/apps/portal/src/components/Layout.tsx +++ b/apps/portal/src/components/Layout.tsx @@ -7,27 +7,10 @@ import { cn } from "@/lib/utils"; import { Activity, Plus, - List, - FlaskConical, - BarChart3, - Tags, - FileText, - KeyRound, - Bot, - Server, - Lightbulb, - Cpu, - MessageSquareText, Settings, Menu, - BookOpen, - FolderGit2, - FolderKanban, GitBranch, Plug, - Puzzle, - Boxes, - SlidersHorizontal, PanelLeftClose, PanelLeftOpen, } from "lucide-react"; @@ -50,6 +33,7 @@ import { UserMenu } from "./UserMenu"; import { ProjectSwitcher } from "./ProjectSwitcher"; import { useFeatureFlags } from "@/contexts/FeatureFlagContext"; import { useProjectContext } from "@/contexts/ProjectContext"; +import { navigationIcons } from "./navigation-icons"; interface NavItem { to: string; @@ -94,10 +78,10 @@ const navGroups: NavGroup[] = [ label: "Activity", scoped: true, items: [ - { to: "/runs", label: "Runs", icon: List }, - { to: "/statistics", label: "Statistics", icon: BarChart3 }, - { to: "/reports", label: "Reports", icon: FileText }, - { to: "/insights", label: "Insights", icon: Lightbulb }, + { to: "/runs", label: "Runs", icon: navigationIcons.runs }, + { to: "/statistics", label: "Statistics", icon: navigationIcons.statistics }, + { to: "/reports", label: "Reports", icon: navigationIcons.reports }, + { to: "/insights", label: "Insights", icon: navigationIcons.insights }, ], }, { @@ -105,12 +89,12 @@ const navGroups: NavGroup[] = [ label: "Library", scoped: true, items: [ - { to: "/task-prompts", label: "Prompts", icon: MessageSquareText }, - { to: "/criteria", label: "Criteria", icon: FlaskConical }, - { to: "/prompt-features", label: "Features", icon: Tags }, - { to: "/profiles", label: "Profiles", icon: SlidersHorizontal, featureKey: "profiles" }, - { to: "/skills", label: "Skills", icon: BookOpen, featureKey: "skills" }, - { to: "/codebases", label: "Codebases", icon: FolderGit2 }, + { to: "/task-prompts", label: "Prompts", icon: navigationIcons.prompts }, + { to: "/criteria", label: "Criteria", icon: navigationIcons.criteria }, + { to: "/prompt-features", label: "Features", icon: navigationIcons.features }, + { to: "/profiles", label: "Profiles", icon: navigationIcons.profiles, featureKey: "profiles" }, + { to: "/skills", label: "Skills", icon: navigationIcons.skills, featureKey: "skills" }, + { to: "/codebases", label: "Codebases", icon: navigationIcons.codebases }, ], }, { @@ -118,9 +102,9 @@ const navGroups: NavGroup[] = [ label: "Integrations", scoped: true, items: [ - { to: "/resources", label: "Resources", icon: Boxes }, - { to: "/mcp-servers", label: "MCP", icon: Server, featureKey: "mcp" }, - { to: "/extensions", label: "Extensions", icon: Puzzle, featureKey: "extensions" }, + { to: "/resources", label: "Resources", icon: navigationIcons.resources }, + { to: "/mcp-servers", label: "MCP", icon: navigationIcons.mcp, featureKey: "mcp" }, + { to: "/extensions", label: "Extensions", icon: navigationIcons.extensions, featureKey: "extensions" }, ], }, { @@ -129,9 +113,9 @@ const navGroups: NavGroup[] = [ id: "platform", label: "Platform", items: [ - { to: "/agents", label: "Agents", icon: Bot, featureKey: "agents" }, - { to: "/models", label: "Models", icon: Cpu, featureKey: "models" }, - { to: "/secrets", label: "Secrets", icon: KeyRound, featureKey: "tokens" }, + { to: "/agents", label: "Agents", icon: navigationIcons.agents, featureKey: "agents" }, + { to: "/models", label: "Models", icon: navigationIcons.models, featureKey: "models" }, + { to: "/secrets", label: "Secrets", icon: navigationIcons.secrets, featureKey: "tokens" }, ], }, ]; @@ -394,7 +378,7 @@ export function Layout() { diff --git a/apps/portal/src/components/list-layout/ListLayout.stories.tsx b/apps/portal/src/components/list-layout/ListLayout.stories.tsx new file mode 100644 index 000000000..5f08407a0 --- /dev/null +++ b/apps/portal/src/components/list-layout/ListLayout.stories.tsx @@ -0,0 +1,66 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import type { Meta, StoryObj } from "@storybook/react-vite"; +import { expect, screen, userEvent } from "storybook/test"; +import { ListLayout } from "./ListLayout"; + +const meta = { + title: "Layout/ListLayout", + component: ListLayout, + tags: ["ai-generated", "needs-work"], + parameters: { + layout: "fullscreen", + }, + args: { + title: "Runs", + description: "Benchmark runs and processing history", + filterRail:
Run filters
, + railStorageKey: "runs", + children:
Run table
, + }, +} satisfies Meta; + +export default meta; +type Story = StoryObj; + +export const Runs: Story = { + play: async ({ canvas }) => { + const trigger = canvas.getByLabelText("Open filters"); + await expect(trigger.querySelector(".lucide-list")).toBeTruthy(); + await expect(trigger).toHaveAttribute("title", "Open filters"); + + if (window.matchMedia("(max-width: 1023.98px)").matches) { + await userEvent.click(trigger); + await expect(screen.getByRole("dialog", { name: "Filters" })).toBeVisible(); + await userEvent.click(screen.getByRole("button", { name: "Close" })); + } + }, +}; + +export const Reports: Story = { + args: { + title: "Reports", + description: "Generated benchmark reports", + filterRail:
Report filters
, + railStorageKey: "reports", + children:
Report table
, + }, + play: async ({ canvas }) => { + await expect( + canvas.getByLabelText("Open filters").querySelector(".lucide-file-text"), + ).toBeTruthy(); + }, +}; + +export const UnknownSection: Story = { + args: { + title: "Custom list", + railStorageKey: "custom-list", + }, + play: async ({ canvas }) => { + await expect( + canvas.getByLabelText("Open filters").querySelector(".lucide-sliders-horizontal"), + ).toBeTruthy(); + }, +}; diff --git a/apps/portal/src/components/list-layout/ListLayout.test.tsx b/apps/portal/src/components/list-layout/ListLayout.test.tsx index 3a2b68e60..7083f1ac7 100644 --- a/apps/portal/src/components/list-layout/ListLayout.test.tsx +++ b/apps/portal/src/components/list-layout/ListLayout.test.tsx @@ -2,14 +2,21 @@ // Licensed under the MIT License. // @vitest-environment happy-dom -import { cleanup, fireEvent, render, screen } from "@testing-library/react"; -import { afterEach, describe, expect, it, vi } from "vitest"; +import { cleanup, fireEvent, render, screen, within } from "@testing-library/react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { composeStories } from "@storybook/react-vite"; +import { useIsCompactViewport } from "@/hooks/useMediaQuery"; import { ListLayout } from "./ListLayout"; +import * as stories from "./ListLayout.stories"; vi.mock("@/hooks/useMediaQuery", () => ({ - useIsCompactViewport: () => false, + useIsCompactViewport: vi.fn(() => false), })); +beforeEach(() => { + vi.mocked(useIsCompactViewport).mockReturnValue(false); +}); + afterEach(() => { cleanup(); localStorage.clear(); @@ -30,12 +37,75 @@ describe("ListLayout", () => { expect(screen.getByText("Runs")).toBeTruthy(); expect(screen.getByText("table-content")).toBeTruthy(); + expect( + screen.getByRole("button", { name: "Open filters" }).querySelector(".lucide-list"), + ).toBeTruthy(); expect(localStorage.getItem("scope:list-layout:runs:rail-collapsed")).toBe("0"); fireEvent.click(screen.getByRole("button", { name: "Hide filters" })); expect(localStorage.getItem("scope:list-layout:runs:rail-collapsed")).toBe("1"); }); + it("updates the section icon when the list changes", () => { + const { rerender } = render( + filters} railStorageKey="runs"> +
table-content
+
, + ); + expect( + screen.getByRole("button", { name: "Open filters" }).querySelector(".lucide-list"), + ).toBeTruthy(); + + rerender( + filters} railStorageKey="reports"> +
table-content
+
, + ); + expect( + screen.getByRole("button", { name: "Open filters" }).querySelector(".lucide-file-text"), + ).toBeTruthy(); + }); + + it.each([undefined, "", "custom-list", "__proto__", "constructor", "toString"])( + "preserves the generic filter icon for storage key %s", + (railStorageKey) => { + render( + filters} railStorageKey={railStorageKey}> +
table-content
+
, + ); + expect( + screen.getByRole("button", { name: "Open filters" }).querySelector(".lucide-sliders-horizontal"), + ).toBeTruthy(); + }, + ); + + it("opens the compact filter sheet from the section icon", () => { + vi.mocked(useIsCompactViewport).mockReturnValue(true); + render( + filters} railStorageKey="reports"> +
table-content
+
, + ); + const trigger = screen.getByRole("button", { name: "Open filters" }); + expect(trigger.getAttribute("title")).toBe("Open filters"); + expect(trigger.querySelector(".lucide-file-text")).toBeTruthy(); + fireEvent.click(trigger); + expect(screen.getByRole("dialog", { name: "Filters" })).toBeTruthy(); + fireEvent.click(screen.getByRole("button", { name: "Close" })); + expect(screen.queryByRole("dialog", { name: "Filters" })).toBeNull(); + }); + + it("does not add filter controls when there is no rail", () => { + render( + +
table-content
+
, + ); + expect(screen.queryByRole("button", { name: "Open filters" })).toBeNull(); + expect(screen.queryByRole("button", { name: "Hide filters" })).toBeNull(); + }); + it("honors stored collapsed state on first render", () => { localStorage.setItem("scope:list-layout:runs:rail-collapsed", "1"); @@ -53,3 +123,11 @@ describe("ListLayout", () => { }); }); +describe("ListLayout stories", () => { + for (const [name, Story] of Object.entries(composeStories(stories))) { + it(`${name} renders and play passes`, async () => { + const { container } = render(); + await Story.play?.({ canvas: within(container), canvasElement: container }); + }); + } +}); diff --git a/apps/portal/src/components/list-layout/ListLayout.tsx b/apps/portal/src/components/list-layout/ListLayout.tsx index b3224d99d..6f5325aca 100644 --- a/apps/portal/src/components/list-layout/ListLayout.tsx +++ b/apps/portal/src/components/list-layout/ListLayout.tsx @@ -11,6 +11,7 @@ import { } from "@/components/ui/sheet"; import { useIsCompactViewport } from "@/hooks/useMediaQuery"; import { cn } from "@/lib/utils"; +import { getListSectionIcon } from "@/components/navigation-icons"; export interface ListLayoutProps { /** Page title (rendered above the table). */ @@ -97,6 +98,7 @@ export function ListLayout({ const detailVisible = detail !== null && detail !== undefined && detail !== false; const secondaryVisible = secondaryPanel !== null && secondaryPanel !== undefined && secondaryPanel !== false; const hasRail = filterRail !== null; + const SectionIcon = getListSectionIcon(railStorageKey) ?? SlidersHorizontal; return (
@@ -150,9 +152,9 @@ export function ListLayout({ className="h-8 w-8 shrink-0 lg:hidden" onClick={() => setMobileRailOpen(true)} aria-label="Open filters" - title="Filters" + title="Open filters" > - + )} diff --git a/apps/portal/src/components/navigation-icons.test.ts b/apps/portal/src/components/navigation-icons.test.ts new file mode 100644 index 000000000..7abaed975 --- /dev/null +++ b/apps/portal/src/components/navigation-icons.test.ts @@ -0,0 +1,38 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { describe, expect, it } from "vitest"; +import { getListSectionIcon, navigationIcons, type NavigationIconKey } from "./navigation-icons"; + +const sections: [string, NavigationIconKey][] = [ + ["runs", "runs"], + ["reports", "reports"], + ["report-templates", "reports"], + ["insights", "insights"], + ["task-prompts", "prompts"], + ["criteria", "criteria"], + ["prompt-features", "features"], + ["profiles", "profiles"], + ["skills", "skills"], + ["codebases", "codebases"], + ["resources", "resources"], + ["mcp-servers", "mcp"], + ["extensions", "extensions"], + ["agents", "agents"], + ["models", "models"], + ["tokens", "secrets"], + ["accounts", "secrets"], +]; + +describe("getListSectionIcon", () => { + it.each(sections)("maps %s to the %s navigation icon", (storageKey, section) => { + expect(getListSectionIcon(storageKey)).toBe(navigationIcons[section]); + }); + + it.each([undefined, "", "custom-list", "__proto__", "constructor", "toString"])( + "leaves unregistered storage key %s to the caller's fallback", + (storageKey) => { + expect(getListSectionIcon(storageKey)).toBeNull(); + }, + ); +}); diff --git a/apps/portal/src/components/navigation-icons.ts b/apps/portal/src/components/navigation-icons.ts new file mode 100644 index 000000000..a57927c2a --- /dev/null +++ b/apps/portal/src/components/navigation-icons.ts @@ -0,0 +1,74 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { + BarChart3, + BookOpen, + Bot, + Boxes, + Cpu, + FileText, + FlaskConical, + FolderGit2, + FolderKanban, + KeyRound, + Lightbulb, + List, + MessageSquareText, + Puzzle, + Server, + SlidersHorizontal, + Tags, + type LucideIcon, +} from "lucide-react"; + +export const navigationIcons = { + runs: List, + statistics: BarChart3, + reports: FileText, + insights: Lightbulb, + prompts: MessageSquareText, + criteria: FlaskConical, + features: Tags, + profiles: SlidersHorizontal, + skills: BookOpen, + codebases: FolderGit2, + resources: Boxes, + mcp: Server, + extensions: Puzzle, + agents: Bot, + models: Cpu, + secrets: KeyRound, + projects: FolderKanban, +} as const satisfies Record; + +export type NavigationIconKey = keyof typeof navigationIcons; + +const listSectionByStorageKey = { + runs: "runs", + reports: "reports", + "report-templates": "reports", + insights: "insights", + "task-prompts": "prompts", + criteria: "criteria", + "prompt-features": "features", + profiles: "profiles", + skills: "skills", + codebases: "codebases", + resources: "resources", + "mcp-servers": "mcp", + extensions: "extensions", + agents: "agents", + models: "models", + tokens: "secrets", + accounts: "secrets", +} as const satisfies Record; + +export function getListSectionIcon(storageKey: string | undefined): LucideIcon | null { + if (!storageKey || !Object.prototype.hasOwnProperty.call(listSectionByStorageKey, storageKey)) { + return null; + } + + const section = listSectionByStorageKey[storageKey as keyof typeof listSectionByStorageKey]; + return navigationIcons[section]; +} diff --git a/docs/architecture/app-design.md b/docs/architecture/app-design.md index f1a243262..dd1bc6e25 100644 --- a/docs/architecture/app-design.md +++ b/docs/architecture/app-design.md @@ -609,6 +609,15 @@ Workers publish log events to Redis Pub/Sub channels keyed by run ID. The API su The Portal desktop shell uses a persistent left navigation sidebar. It defaults to the compact icon rail, and users can expand it to show navigation labels; the choice is stored in `localStorage` under `scope:layout:sidebar-expanded`. Mobile navigation remains a sheet-based menu with labels always visible. +`components/navigation-icons.ts` owns the shared section icons used by `Layout` +and the compact `ListLayout` header's **Open filters** trigger. `ListLayout` +resolves its section from the existing `railStorageKey`; report templates share +the Reports icon, while keys and accounts share Secrets. Missing or unregistered +storage keys retain the generic sliders icon. Desktop rail toggles, persisted +collapse state, navigation routes, feature flags, and project gating are unchanged. +When adding a list section, register its storage key here to keep its header and +sidebar icons aligned. + The Keys list and its nested `/secrets/keys/:id/preview` route use the same full-bleed shell. Opening or closing the preview keeps the tabs and filter rail aligned with the navigation. Key registration and full-detail routes retain the