From 8f5a45ef517f3b3a8543b10b3a5dcfe98536ed0a Mon Sep 17 00:00:00 2001
From: Keith Avery <slabgorb@gmail.com>
Date: Sun, 26 Apr 2026 07:46:03 -0400
Subject: [PATCH] ux(chargen): add pencil icon and hover state to row Edit
 affordances
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The chargen confirmation sheet's per-row "Edit" links rendered as bare
text in `text-muted-foreground` against a dark card background — pale,
small, and easy to miss. ux-observer flagged this in playtest 2026-04-26
(see sq-playtest-pingpong "[UX] Edit affordances on the chargen sheet
are too pale to discover"); especially impactful on the stats and
equipment rows where the row's own content is dense.

Two-pronged upgrade, each one independently improving discoverability:
- `lucide-react` `Pencil` icon next to the visible "Edit" text. Makes
  the affordance obvious without requiring hover (better for first-
  timers and touch users; serves Sebastien who scans for mechanical
  controls per CLAUDE.md playgroup rubric).
- Row-level named-group hover (`group/edit-row`) brightens the row
  background and the Edit button's border on hover. Elegant cue for
  mouse users; doesn't demand attention until the user moves toward it.

Accessibility preserved: icon is `aria-hidden`, the button still
carries `aria-label="Edit <Field>"`, and the visible "Edit" text is
retained alongside the icon (not icon-only). Dark aesthetic kept —
all colors are existing theme tokens (muted-foreground, border, card).

No behavior change — the click handler still emits
`{action: "edit", target_step: index}` exactly as before.

Tests:
- `CharacterCreation.edit-affordance.test.tsx` — 5 unit assertions
  (icon renders per row, accessible name preserved, "Edit" text kept,
  icon aria-hidden, row-hover Tailwind class wiring) + 1 wiring test
  asserting App.tsx still imports + mounts CharacterCreation in the
  `creation` session phase.

Refs: sq-playtest-pingpong 2026-04-26 — UX section.
---
 .../CharacterCreation/CharacterCreation.tsx   |   9 +-
 ...CharacterCreation.edit-affordance.test.tsx | 128 ++++++++++++++++++
 2 files changed, 134 insertions(+), 3 deletions(-)
 create mode 100644 src/components/CharacterCreation/__tests__/CharacterCreation.edit-affordance.test.tsx

diff --git a/src/components/CharacterCreation/CharacterCreation.tsx b/src/components/CharacterCreation/CharacterCreation.tsx
index 65c1245..5031f94 100644
--- a/src/components/CharacterCreation/CharacterCreation.tsx
+++ b/src/components/CharacterCreation/CharacterCreation.tsx
@@ -1,4 +1,5 @@
 import { useState } from "react";
+import { Pencil } from "lucide-react";
 import { toRoman } from "@/lib/utils";
 
 interface CreationChoice {
@@ -132,7 +133,7 @@ export function CharacterCreation({ scene, loading, onRespond }: CharacterCreati
               <div
                 key={key}
                 data-testid={`review-section-${key}`}
-                className="flex items-center justify-between py-2 border-b border-border/20 last:border-0"
+                className="group/edit-row flex items-center justify-between py-2 px-2 -mx-2 rounded-md border-b border-border/20 last:border-0 hover:bg-muted/30 transition-colors"
               >
                 <div>
                   <span className="text-xs uppercase tracking-wider text-muted-foreground/60">{key}</span>
@@ -140,10 +141,12 @@ export function CharacterCreation({ scene, loading, onRespond }: CharacterCreati
                 </div>
                 <button
                   onClick={() => onRespond({ action: "edit", target_step: index })}
-                  className="text-xs text-muted-foreground hover:text-foreground transition-colors px-2 py-1"
+                  data-testid={`review-edit-${key}`}
+                  className="inline-flex items-center gap-1.5 text-xs text-muted-foreground/70 group-hover/edit-row:text-foreground hover:text-foreground transition-colors px-2 py-1 rounded border border-border/30 group-hover/edit-row:border-border/70 hover:border-border bg-transparent group-hover/edit-row:bg-card/40"
                   aria-label={`Edit ${key}`}
                 >
-                  Edit
+                  <Pencil aria-hidden="true" className="w-3 h-3" data-testid={`review-edit-icon-${key}`} />
+                  <span>Edit</span>
                 </button>
               </div>
             ))
diff --git a/src/components/CharacterCreation/__tests__/CharacterCreation.edit-affordance.test.tsx b/src/components/CharacterCreation/__tests__/CharacterCreation.edit-affordance.test.tsx
new file mode 100644
index 0000000..68b166d
--- /dev/null
+++ b/src/components/CharacterCreation/__tests__/CharacterCreation.edit-affordance.test.tsx
@@ -0,0 +1,128 @@
+import { readFileSync } from "node:fs";
+import { resolve } from "node:path";
+import { describe, expect, it } from "vitest";
+import { render, screen } from "@testing-library/react";
+import { CharacterCreation } from "../CharacterCreation";
+
+/**
+ * Playtest 2026-04-26 sq-playtest-pingpong section
+ * `[UX] Edit affordances on the chargen sheet are too pale to discover`.
+ *
+ * The original "Edit" link was a bare text-only `<button>` rendered in
+ * `text-muted-foreground` — too pale to read against a dark background, no
+ * visual icon, no per-row hover state. ux-observer flagged it as a
+ * discoverability hit (low priority, but real for first-timers and touch
+ * users).
+ *
+ * Fix: add a `lucide-react` `Pencil` icon next to the "Edit" label, plus a
+ * row-level hover state that brightens the entire row's background and the
+ * Edit button's border. Accessibility-preserving: the icon is `aria-hidden`
+ * and the button still carries `aria-label="Edit <Field>"`.
+ *
+ * These tests pin the visual upgrade so a future refactor cannot quietly
+ * regress it back to the bare-text pale link.
+ */
+describe("CharacterCreation confirmation preview — edit affordance discoverability", () => {
+  const renderConfirmation = () =>
+    render(
+      <CharacterCreation
+        scene={{
+          phase: "confirmation",
+          scene_index: 3,
+          total_scenes: 4,
+          input_type: "confirm",
+          message: "Confirm your character?",
+          character_preview: {
+            Name: "Rux",
+            Race: "Beastkin",
+            Class: "Delver",
+          },
+        }}
+        loading={false}
+        onRespond={() => {}}
+      />,
+    );
+
+  it("renders a pencil icon inside each Edit button", () => {
+    renderConfirmation();
+    // One pencil per row; testid mirrors the row testid pattern.
+    expect(screen.getByTestId("review-edit-icon-Name")).toBeInTheDocument();
+    expect(screen.getByTestId("review-edit-icon-Race")).toBeInTheDocument();
+    expect(screen.getByTestId("review-edit-icon-Class")).toBeInTheDocument();
+  });
+
+  it("preserves the accessible name on each Edit button", () => {
+    renderConfirmation();
+    // `getByRole("button", { name })` does the accessibility-tree lookup;
+    // this would fail if the icon swallowed the accessible name (e.g. by
+    // not being aria-hidden) or if the visible "Edit" text were dropped.
+    expect(
+      screen.getByRole("button", { name: "Edit Name" }),
+    ).toBeInTheDocument();
+    expect(
+      screen.getByRole("button", { name: "Edit Race" }),
+    ).toBeInTheDocument();
+    expect(
+      screen.getByRole("button", { name: "Edit Class" }),
+    ).toBeInTheDocument();
+  });
+
+  it("keeps the visible 'Edit' text alongside the icon (not icon-only)", () => {
+    renderConfirmation();
+    // Belt-and-suspenders: the button text content should still contain
+    // "Edit" — the icon is reinforcement, not replacement. Helps Sebastien
+    // (mechanics-first) scan the column quickly without parsing icons.
+    const button = screen.getByTestId("review-edit-Name");
+    expect(button.textContent).toMatch(/Edit/);
+  });
+
+  it("marks the icon aria-hidden so the button's accessible name is unambiguous", () => {
+    renderConfirmation();
+    const icon = screen.getByTestId("review-edit-icon-Name");
+    expect(icon.getAttribute("aria-hidden")).toBe("true");
+  });
+
+  it("applies a row-level hover state class so the affordance brightens on row hover", () => {
+    renderConfirmation();
+    // Tailwind class assertion — JSDOM doesn't simulate :hover paint, so
+    // we verify the structural class that drives the effect. The
+    // `group/edit-row` named-group plus `hover:bg-muted/30` on the row
+    // wrapper, and `group-hover/edit-row:*` on the button, are what give
+    // first-timers a "this is interactive" cue without requiring a click.
+    const row = screen.getByTestId("review-section-Name");
+    expect(row.className).toMatch(/group\/edit-row/);
+    expect(row.className).toMatch(/hover:bg-muted/);
+    const button = screen.getByTestId("review-edit-Name");
+    expect(button.className).toMatch(/group-hover\/edit-row:/);
+  });
+});
+
+/**
+ * Wiring test (CLAUDE.md "Every Test Suite Needs a Wiring Test"):
+ * the upgraded affordance is meaningless if `CharacterCreation` isn't
+ * actually mounted by the production session router. We verify the
+ * component-level upgrade is reachable from `App.tsx`'s real render path
+ * by inspecting the production source for both the import and the JSX
+ * mount inside the `creation` phase branch.
+ *
+ * This is deliberately a static-source check rather than a full App
+ * mount: the App-mount path requires a WebSocket harness, slug routing
+ * setup, and identity confirmation — all of which already have dedicated
+ * wiring tests (`mp-03-event-sync-wiring`, `slug-routing`, etc.). What
+ * this test catches is a future PR that deletes the `<CharacterCreation>`
+ * mount or renames the component without updating App.tsx, leaving the
+ * upgraded affordance present in the file but unreachable in production.
+ */
+describe("CharacterCreation edit affordance — wiring", () => {
+  it("is mounted by App.tsx during the chargen session phase", () => {
+    // process.cwd() in vitest is the package root (sidequest-ui).
+    const appSource = readFileSync(resolve("src/App.tsx"), "utf8");
+    // Production import wiring.
+    expect(appSource).toMatch(
+      /from\s+["']@\/components\/CharacterCreation\/CharacterCreation["']/,
+    );
+    // Production JSX wiring inside the creation phase branch.
+    expect(appSource).toMatch(/sessionPhase\s*===\s*["']creation["']/);
+    expect(appSource).toMatch(/<CharacterCreation\b/);
+  });
+});