fix(canvas): address all code review findings on PR #482

- Reconcile TIER_CONFIG/TIER_COLORS into single TIER_CONFIG with both `color` (pill style) and `border` (bordered badge style) fields - Remove TemplatePalette alias indirection (TIER_LABELS_SHARED → direct import) - Extract inline spinner SVGs to shared Spinner component (3 copies → 1) - Migrate status dot colors from 6 remaining files to shared tokens: SearchDialog, StatusDot, Legend, ContextMenu, Toolbar + add statusDotClass() - Add COMM_TYPE_LABELS to design-tokens, used by CommunicationOverlay sr-only - Update reduced-motion tests: components that delegate to design-tokens pass the guard check via import detection; add design-tokens.ts own test - 507/507 tests pass, build clean Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-16 07:48:47 -07:00 · 2026-04-16 07:48:47 -07:00 · a4b1886c35
commit a4b1886c35
parent ac6d6ce5bd
11 changed files with 75 additions and 64 deletions
--- a/canvas/src/tests/reduced-motion.test.ts
+++ b/canvas/src/tests/reduced-motion.test.ts
@ -13,6 +13,12 @@ function readSrc(rel: string) {
  return readFileSync(join(root, "src", rel), "utf8");
 }

+function usesGuardedPulse(src: string): boolean {
+  if (src.includes("motion-safe:animate-pulse")) return true;
+  if (src.includes("from \"@/lib/design-tokens\"") || src.includes("from '@/lib/design-tokens'")) return true;
+  return false;
+}
+
 describe("prefers-reduced-motion compliance", () => {
  it("globals.css contains @media (prefers-reduced-motion: reduce) block", () => {
    const css = readSrc("app/globals.css");
@ -34,10 +40,10 @@ describe("prefers-reduced-motion compliance", () => {
    expect(src).toContain("motion-safe:animate-pulse");
  });

-  it("StatusDot.tsx uses motion-safe:animate-pulse", () => {
+  it("StatusDot.tsx uses motion-safe:animate-pulse (inline or via shared tokens)", () => {
    const src = readSrc("components/StatusDot.tsx");
    expect(src.includes("animate-pulse") && !src.includes("motion-safe:animate-pulse")).toBe(false);
-    expect(src).toContain("motion-safe:animate-pulse");
+    expect(usesGuardedPulse(src)).toBe(true);
  });

  it("Toolbar.tsx uses motion-safe:animate-pulse", () => {
@ -52,16 +58,16 @@ describe("prefers-reduced-motion compliance", () => {
    expect(src).toContain("motion-safe:animate-pulse");
  });

-  it("Legend.tsx uses motion-safe:animate-pulse", () => {
+  it("Legend.tsx uses motion-safe:animate-pulse (inline or via shared tokens)", () => {
    const src = readSrc("components/Legend.tsx");
    expect(src.includes("animate-pulse") && !src.includes("motion-safe:animate-pulse")).toBe(false);
-    expect(src).toContain("motion-safe:animate-pulse");
+    expect(usesGuardedPulse(src)).toBe(true);
  });

-  it("SearchDialog.tsx uses motion-safe:animate-pulse", () => {
+  it("SearchDialog.tsx uses motion-safe:animate-pulse (inline or via shared tokens)", () => {
    const src = readSrc("components/SearchDialog.tsx");
    expect(src.includes("animate-pulse") && !src.includes("motion-safe:animate-pulse")).toBe(false);
-    expect(src).toContain("motion-safe:animate-pulse");
+    expect(usesGuardedPulse(src)).toBe(true);
  });

  it("TerminalTab.tsx uses motion-safe:animate-pulse", () => {
@ -76,6 +82,12 @@ describe("prefers-reduced-motion compliance", () => {
    expect(src).toContain("motion-safe:animate-pulse");
  });

+  it("design-tokens.ts uses motion-safe:animate-pulse, not bare animate-pulse", () => {
+    const src = readSrc("lib/design-tokens.ts");
+    expect(src.includes("animate-pulse") && !src.includes("motion-safe:animate-pulse")).toBe(false);
+    expect(src).toContain("motion-safe:animate-pulse");
+  });
+
  it("globals.css disables animate-in and slide-in classes under reduced-motion", () => {
    const css = readSrc("app/globals.css");
    expect(css).toContain(".animate-in");
--- a/canvas/src/components/CommunicationOverlay.tsx
+++ b/canvas/src/components/CommunicationOverlay.tsx
@ -3,6 +3,7 @@
 import { useState, useEffect, useCallback, useRef } from "react";
 import { useCanvasStore } from "@/store/canvas";
 import { api } from "@/lib/api";
+import { COMM_TYPE_LABELS } from "@/lib/design-tokens";

 interface Communication {
  id: string;
@ -143,7 +144,7 @@ export function CommunicationOverlay() {
              <div className="flex items-center justify-between gap-2">
                <div className="flex items-center gap-1.5 min-w-0">
                  <span className={typeColor} aria-hidden="true">{typeIcon}</span>
-                  <span className="sr-only">{c.type === "a2a_send" ? "sent" : c.type === "a2a_receive" ? "received" : "task update"}</span>
+                  <span className="sr-only">{COMM_TYPE_LABELS[c.type] ?? c.type}</span>
                  <span className="text-zinc-300 font-medium truncate">
                    {c.sourceName}
                  </span>
--- a/canvas/src/components/ContextMenu.tsx
+++ b/canvas/src/components/ContextMenu.tsx
@ -5,6 +5,7 @@ import { useCanvasStore, type WorkspaceNodeData } from "@/store/canvas";
 import { api } from "@/lib/api";
 import { showToast } from "./Toaster";
 import { ConfirmDialog } from "./ConfirmDialog";
+import { statusDotClass } from "@/lib/design-tokens";

 interface MenuItem {
  label: string;
@ -277,9 +278,7 @@ export function ContextMenu() {
        <div className="flex items-center gap-1.5 mt-0.5">
          <div
            aria-hidden="true"
-            className={`w-1.5 h-1.5 rounded-full ${
-              isOnline ? "bg-emerald-400" : isOfflineOrFailed ? "bg-red-400" : "bg-zinc-500"
-            }`}
+            className={`w-1.5 h-1.5 rounded-full ${statusDotClass(contextMenu.nodeData.status)}`}
          />
          <span className="text-[10px] text-zinc-500">{contextMenu.nodeData.status}</span>
        </div>
--- a/canvas/src/components/EmptyState.tsx
+++ b/canvas/src/components/EmptyState.tsx
@ -4,7 +4,8 @@ import { useState, useEffect } from "react";
 import { api } from "@/lib/api";
 import { useCanvasStore } from "@/store/canvas";
 import { OrgTemplatesSection } from "./TemplatePalette";
-import { TIER_COLORS } from "@/lib/design-tokens";
+import { Spinner } from "./Spinner";
+import { TIER_CONFIG } from "@/lib/design-tokens";

 interface Template {
  id: string;
@ -100,16 +101,13 @@ export function EmptyState() {
        {/* Template grid */}
        {loading ? (
          <div className="flex items-center justify-center gap-2 text-xs text-zinc-400 py-4">
-            <svg className="w-4 h-4 motion-safe:animate-spin" viewBox="0 0 24 24" fill="none" aria-hidden="true">
-              <circle cx="12" cy="12" r="10" stroke="currentColor" strokeWidth="2" opacity="0.25" />
-              <path d="M12 2a10 10 0 0 1 10 10" stroke="currentColor" strokeWidth="2" strokeLinecap="round" />
-            </svg>
+            <Spinner />
            Loading templates...
          </div>
        ) : templates.length > 0 ? (
          <div className="grid grid-cols-2 sm:grid-cols-3 gap-2.5 mb-4 text-left">
            {templates.map((t) => {
-              const tierColor = TIER_COLORS[t.tier] || TIER_COLORS[1];
+              const tierColor = TIER_CONFIG[t.tier]?.border || TIER_CONFIG[1].border;
              return (
                <button
                  key={t.id}
--- a/canvas/src/components/Legend.tsx
+++ b/canvas/src/components/Legend.tsx
@ -1,5 +1,9 @@
 "use client";

+import { STATUS_CONFIG } from "@/lib/design-tokens";
+
+const LEGEND_STATUSES = ["online", "provisioning", "degraded", "failed", "paused", "offline"] as const;
+
 export function Legend() {
  return (
    <div className="fixed bottom-6 left-4 z-30 bg-zinc-900/95 border border-zinc-700/50 rounded-xl px-4 py-3 shadow-xl shadow-black/30 backdrop-blur-sm max-w-[280px]">
@ -9,12 +13,9 @@ export function Legend() {
      <div className="mb-2">
        <div className="text-[11px] text-zinc-500 font-medium mb-1">Status</div>
        <div className="flex flex-wrap gap-x-3 gap-y-1">
-          <StatusItem color="bg-emerald-400" label="Online" />
-          <StatusItem color="bg-sky-400 motion-safe:animate-pulse" label="Starting" />
-          <StatusItem color="bg-amber-400" label="Degraded" />
-          <StatusItem color="bg-red-400" label="Failed" />
-          <StatusItem color="bg-indigo-400" label="Paused" />
-          <StatusItem color="bg-zinc-500" label="Offline" />
+          {LEGEND_STATUSES.map((s) => (
+            <StatusItem key={s} color={STATUS_CONFIG[s].dot} label={STATUS_CONFIG[s].label} />
+          ))}
        </div>
      </div>

--- a/canvas/src/components/SearchDialog.tsx
+++ b/canvas/src/components/SearchDialog.tsx
@ -2,6 +2,7 @@

 import { useState, useEffect, useRef, useCallback } from "react";
 import { useCanvasStore } from "@/store/canvas";
+import { statusDotClass } from "@/lib/design-tokens";

 export function SearchDialog() {
  const open = useCanvasStore((s) => s.searchOpen);
@ -142,12 +143,7 @@ export function SearchDialog() {
              >
                <div
                  aria-hidden="true"
-                  className={`w-2 h-2 rounded-full shrink-0 ${
-                    node.data.status === "online" ? "bg-emerald-400" :
-                    node.data.status === "failed" ? "bg-red-400" :
-                    node.data.status === "provisioning" ? "bg-sky-400 motion-safe:animate-pulse" :
-                    "bg-zinc-500"
-                  }`}
+                  className={`w-2 h-2 rounded-full shrink-0 ${statusDotClass(node.data.status)}`}
                />
                <div className="min-w-0 flex-1">
                  <div className="text-sm text-zinc-200 truncate">{node.data.name}</div>
--- a/canvas/src/components/Spinner.tsx
+++ b/canvas/src/components/Spinner.tsx
@ -0,0 +1,12 @@
+"use client";
+
+const SIZES = { sm: "w-3 h-3", md: "w-4 h-4", lg: "w-5 h-5" } as const;
+
+export function Spinner({ size = "md" }: { size?: keyof typeof SIZES }) {
+  return (
+    <svg className={`${SIZES[size]} motion-safe:animate-spin`} viewBox="0 0 24 24" fill="none" aria-hidden="true">
+      <circle cx="12" cy="12" r="10" stroke="currentColor" strokeWidth="2" opacity="0.25" />
+      <path d="M12 2a10 10 0 0 1 10 10" stroke="currentColor" strokeWidth="2" strokeLinecap="round" />
+    </svg>
+  );
+}
--- a/canvas/src/components/StatusDot.tsx
+++ b/canvas/src/components/StatusDot.tsx
@ -1,13 +1,6 @@
 "use client";

-export const STATUS_COLORS: Record<string, string> = {
-  online: "bg-emerald-400",
-  offline: "bg-zinc-500",
-  paused: "bg-indigo-400",
-  degraded: "bg-amber-400",
-  failed: "bg-red-400",
-  provisioning: "bg-sky-400 motion-safe:animate-pulse",
-};
+import { STATUS_CONFIG, statusDotClass } from "@/lib/design-tokens";

 export function StatusDot({
  status,
@ -17,10 +10,10 @@ export function StatusDot({
  size?: "sm" | "md";
 }) {
  const sizeClass = size === "md" ? "w-2.5 h-2.5" : "w-2 h-2";
-  const glowClass = status === "online" ? "shadow-sm shadow-emerald-400/50" : "";
+  const glowClass = STATUS_CONFIG[status]?.glow ?? "";
  return (
    <div
-      className={`${sizeClass} rounded-full shrink-0 ${STATUS_COLORS[status] || "bg-zinc-600"} ${glowClass}`}
+      className={`${sizeClass} rounded-full shrink-0 ${statusDotClass(status)} ${glowClass}`}
    />
  );
 }
--- a/canvas/src/components/TemplatePalette.tsx
+++ b/canvas/src/components/TemplatePalette.tsx
@ -5,7 +5,8 @@ import { api } from "@/lib/api";
 import { checkDeploySecrets, type PreflightResult } from "@/lib/deploy-preflight";
 import { MissingKeysModal } from "./MissingKeysModal";
 import { ConfirmDialog } from "./ConfirmDialog";
-import { TIER_CONFIG as TIER_LABELS_SHARED } from "@/lib/design-tokens";
+import { Spinner } from "./Spinner";
+import { TIER_CONFIG } from "@/lib/design-tokens";

 interface Template {
  id: string;
@ -92,10 +93,7 @@ export function OrgTemplatesSection() {

      {loading && (
        <div role="status" aria-live="polite" className="flex items-center gap-1.5 text-[10px] text-zinc-500">
-          <svg className="w-3 h-3 motion-safe:animate-spin" viewBox="0 0 24 24" fill="none" aria-hidden="true">
-            <circle cx="12" cy="12" r="10" stroke="currentColor" strokeWidth="2" opacity="0.25" />
-            <path d="M12 2a10 10 0 0 1 10 10" stroke="currentColor" strokeWidth="2" strokeLinecap="round" />
-          </svg>
+          <Spinner size="sm" />
          Loading…
        </div>
      )}
@ -146,7 +144,7 @@ export function OrgTemplatesSection() {
  );
 }

-const TIER_LABELS = TIER_LABELS_SHARED;
+const TIER_LABELS = TIER_CONFIG;

 function ImportAgentButton({ onImported }: { onImported: () => void }) {
  const [importing, setImporting] = useState(false);
@ -355,10 +353,7 @@ export function TemplatePalette() {
          <div className="flex-1 overflow-y-auto p-3 space-y-2">
            {loading && (
              <div role="status" aria-live="polite" className="flex items-center justify-center gap-2 text-xs text-zinc-500 text-center py-8">
-                <svg className="w-4 h-4 motion-safe:animate-spin" viewBox="0 0 24 24" fill="none" aria-hidden="true">
-                  <circle cx="12" cy="12" r="10" stroke="currentColor" strokeWidth="2" opacity="0.25" />
-                  <path d="M12 2a10 10 0 0 1 10 10" stroke="currentColor" strokeWidth="2" strokeLinecap="round" />
-                </svg>
+                <Spinner />
                Loading…
              </div>
            )}
--- a/canvas/src/components/Toolbar.tsx
+++ b/canvas/src/components/Toolbar.tsx
@ -7,6 +7,7 @@ import { SettingsButton } from "@/components/settings/SettingsButton";
 import { settingsGearRef } from "@/components/settings/SettingsPanel";
 import { ConfirmDialog } from "@/components/ConfirmDialog";
 import { showToast } from "@/components/Toaster";
+import { statusDotClass } from "@/lib/design-tokens";

 export function Toolbar() {
  const nodes = useCanvasStore((s) => s.nodes);
@ -120,15 +121,15 @@ export function Toolbar() {

      {/* Status counts */}
      <div className="flex items-center gap-2.5">
-        <StatusPill color="bg-emerald-400" count={counts.online} label="online" />
+        <StatusPill color={statusDotClass("online")} count={counts.online} label="online" />
        {counts.offline > 0 && (
-          <StatusPill color="bg-zinc-500" count={counts.offline} label="offline" />
+          <StatusPill color={statusDotClass("offline")} count={counts.offline} label="offline" />
        )}
        {counts.provisioning > 0 && (
-          <StatusPill color="bg-sky-400 motion-safe:animate-pulse" count={counts.provisioning} label="starting" />
+          <StatusPill color={statusDotClass("provisioning")} count={counts.provisioning} label="starting" />
        )}
        {counts.failed > 0 && (
-          <StatusPill color="bg-red-400" count={counts.failed} label="failed" />
+          <StatusPill color={statusDotClass("failed")} count={counts.failed} label="failed" />
        )}
      </div>

@ -259,7 +260,7 @@ function WsStatusPill({ status }: { status: "connected" | "connecting" | "discon
  if (status === "connected") {
    return (
      <div className="flex items-center gap-1.5" title="Real-time updates: connected">
-        <div className="w-1.5 h-1.5 rounded-full bg-emerald-400" />
+        <div className={`w-1.5 h-1.5 rounded-full ${statusDotClass("online")}`} />
        <span className="text-[10px] text-zinc-500">Live</span>
      </div>
    );
@ -274,7 +275,7 @@ function WsStatusPill({ status }: { status: "connected" | "connecting" | "discon
  }
  return (
    <div className="flex items-center gap-1.5" title="Real-time updates: disconnected">
-      <div className="w-1.5 h-1.5 rounded-full bg-red-400" />
+      <div className={`w-1.5 h-1.5 rounded-full ${statusDotClass("failed")}`} />
      <span className="text-[10px] text-zinc-500">Offline</span>
    </div>
  );
--- a/canvas/src/lib/design-tokens.ts
+++ b/canvas/src/lib/design-tokens.ts
@ -7,16 +7,19 @@ export const STATUS_CONFIG: Record<string, { dot: string; glow: string; label: s
  provisioning: { dot: "bg-sky-400 motion-safe:animate-pulse", glow: "shadow-sky-400/50", label: "Starting", bar: "from-sky-500/20 to-transparent" },
 };

-export const TIER_CONFIG: Record<number, { label: string; color: string }> = {
-  1: { label: "T1", color: "text-zinc-500 bg-zinc-800/80" },
-  2: { label: "T2", color: "text-sky-400 bg-sky-950/50" },
-  3: { label: "T3", color: "text-violet-400 bg-violet-950/50" },
-  4: { label: "T4", color: "text-amber-400 bg-amber-950/50" },
+export function statusDotClass(status: string): string {
+  return STATUS_CONFIG[status]?.dot ?? "bg-zinc-500";
+}
+
+export const TIER_CONFIG: Record<number, { label: string; color: string; border: string }> = {
+  1: { label: "T1", color: "text-zinc-500 bg-zinc-800/80", border: "text-zinc-400 border-zinc-700/60" },
+  2: { label: "T2", color: "text-sky-400 bg-sky-950/50", border: "text-sky-400 border-sky-500/30" },
+  3: { label: "T3", color: "text-violet-400 bg-violet-950/50", border: "text-violet-400 border-violet-500/30" },
+  4: { label: "T4", color: "text-amber-400 bg-amber-950/50", border: "text-amber-400 border-amber-500/30" },
 };

-export const TIER_COLORS: Record<number, string> = {
-  1: "text-zinc-400 border-zinc-700/60",
-  2: "text-sky-400 border-sky-500/30",
-  3: "text-violet-400 border-violet-500/30",
-  4: "text-amber-400 border-amber-500/30",
+export const COMM_TYPE_LABELS: Record<string, string> = {
+  a2a_send: "sent",
+  a2a_receive: "received",
+  task_update: "task update",
 };