From 50e72b47fd98031cd939f24c2f5bdec070fd1ab6 Mon Sep 17 00:00:00 2001 From: Maggie Appleton <5599295+MaggieAppleton@users.noreply.github.com> Date: Wed, 30 Sep 2026 21:21:53 +0100 Subject: [PATCH 01/10] Restyle the decision card to the approved design Drop the Decision label row for a mark beside the question, lettered option rows with a trailing check, an always-last Add an option row, Discard/Save footer with in-place confirm, a stepper in place of question tabs, and a shared callout for a failed save. Adds DecisionIcon. Co-Authored-By: Claude Sonnet 5.5 --- apps/web/src/icon-assets.test.ts | 5 +- apps/web/src/theme.css | 307 ++++++++++- apps/web/src/tokens.test.ts | 36 +- e2e/responsive-decisions.e2e.ts | 10 +- e2e/sidecar.e2e.ts | 81 +-- .../editor/src/widgets/questionnaire.test.tsx | 32 +- packages/icons/src/index.ts | 1 + packages/icons/src/line.tsx | 11 + .../question/src/react/question-view.test.ts | 78 +++ packages/question/src/react/question-view.tsx | 487 +++++++++--------- .../exceptions/dynamic-packages.json | 20 +- 11 files changed, 729 insertions(+), 339 deletions(-) diff --git a/apps/web/src/icon-assets.test.ts b/apps/web/src/icon-assets.test.ts index ec91b53d..11226a79 100644 --- a/apps/web/src/icon-assets.test.ts +++ b/apps/web/src/icon-assets.test.ts @@ -77,7 +77,10 @@ test("interface icons default to fourteen pixels", () => { for (let match of readFileSync(file, "utf8").matchAll(explicit)) { let size = Number(match[1]); let emptyStateException = file.endsWith("design-audit/surfaces.tsx") && size === 24; - if (size !== 14 && !emptyStateException) offenders.push(`${file}: ${match[0]}`); + let stepperCaret = file.endsWith("question/src/react/question-view.tsx") && size === 16; + if (size !== 14 && !emptyStateException && !stepperCaret) { + offenders.push(`${file}: ${match[0]}`); + } } } expect(offenders).toEqual([]); diff --git a/apps/web/src/theme.css b/apps/web/src/theme.css index 3c4ec5ae..7d7ad924 100644 --- a/apps/web/src/theme.css +++ b/apps/web/src/theme.css @@ -616,7 +616,6 @@ body { :root[data-plan-coarse-pointer] .repository-picker-search, :root[data-plan-coarse-pointer] .channel-create-input, :root[data-plan-coarse-pointer] [data-hosted] .btn, -:root[data-plan-coarse-pointer] .question-tab, :root[data-plan-coarse-pointer] .question-choice-row, :root[data-plan-coarse-pointer] .question-actions button, :root[data-plan-coarse-pointer] :where(button, [role="button"])[data-press="wide"] { @@ -632,6 +631,312 @@ body { scroll-margin-block-end: calc(var(--keyboard-inset, 0px) + 1rem); } +/* The decision card: a question, lettered options, and a quiet footer. */ +.question-card { + padding: 1rem 0.5rem 0.75rem; +} + +/* Coloured icon holders: the global icon colour would otherwise win. */ +:is(.question-mark, .question-check) [data-nucleo-icon], +.question-callout .plan-research-badge [data-nucleo-icon] { + color: inherit; +} + +.question-head { + display: flex; + align-items: flex-start; + gap: 0.625rem; + padding-inline: 0.5rem; +} + +.question-mark { + display: grid; + flex: none; + place-items: center; + inline-size: 1.5rem; + block-size: 1.5rem; + border-radius: var(--radius-full); + background: var(--color-success-wash); + color: var(--color-success); +} + +.question-head-text { + flex: 1; + min-width: 0; +} + +.question-title { + /* Centre the first line on the 1.5rem mark, then lift 2px optically. */ + margin: calc((1.5rem - 1lh) / 2 - 0.125rem) 0 0; + font-size: var(--text-base); + line-height: var(--text-lg--line-height); + font-weight: 600; + color: var(--color-text-primary); + overflow-wrap: break-word; + text-wrap: pretty; +} + +.question-title-link { + font: inherit; +} + +.question-hint { + margin: 0.125rem 0 0; + font-size: var(--text-xs); + line-height: var(--text-xs--line-height); + color: var(--color-text-tertiary); +} + +.question-options { + display: grid; + gap: 0.125rem; + min-width: 0; + margin: 0.75rem 0 0; + padding: 0; + border: 0; +} + +.question-option { + position: relative; + display: flex; + align-items: flex-start; + gap: 0.625rem; + padding: 0.375rem 0.75rem 0.375rem 0.5rem; + border-radius: var(--radius-md); + font-size: var(--text-sm); + line-height: var(--text-sm--line-height); + transition: background-color var(--duration-fast) var(--ease-out); +} + +/* The native control stays focusable but unseen; its label row is the target. */ +.question-input { + appearance: none; + position: absolute; + inset-block-start: 0; + inset-inline-start: 0; + inline-size: 1px; + block-size: 1px; + margin: 0; + opacity: 0; +} + +label.question-option:has(.question-input:not(:disabled)) { + cursor: pointer; +} + +.question-option:has(.question-input:focus-visible) { + outline: var(--focus-ring-width) solid var(--focus-ring-color); + outline-offset: var(--focus-ring-offset); +} + +.question-option:has(.question-input:not(:disabled)):hover { + background: var(--color-hover); +} + +.question-option:has(.question-input:checked), +.question-option:has(.question-input:checked:not(:disabled)):hover { + background: var(--color-brand-wash); +} + +/* A one-line slot (inherits the label's line height) holding a small tile. */ +.question-key { + display: flex; + flex: none; + align-items: center; + block-size: 1lh; +} + +/* Same 1.5rem as the header mark, so option text shares the title's left edge. */ +.question-key > span { + display: grid; + place-items: center; + inline-size: 1.5rem; + block-size: 1.5rem; + border-radius: var(--radius-sm); + background: var(--color-gray-150); + font-size: var(--text-xs); + line-height: 1; + font-weight: 500; + color: var(--color-text-secondary); + transition: + background-color var(--duration-fast) var(--ease-out), + color var(--duration-fast) var(--ease-out); +} + +.question-option:has(.question-input:checked) .question-key > span { + background: var(--color-brand); + color: var(--color-white); +} + +/* With a description below, the tile sits 4px under the label's centre (optical). */ +.question-option:has(.question-desc) .question-key > span { + translate: 0 0.25rem; +} + +.question-text { + display: grid; + flex: 1; + min-width: 0; +} + +.question-label { + font-weight: 500; + color: var(--color-text-primary); +} + +.question-desc { + color: var(--color-text-secondary); +} + +.question-check { + flex: none; + display: grid; + block-size: 1lh; + place-items: center; + color: var(--color-brand); + opacity: 0; + transition: opacity var(--duration-fast) var(--ease-out); +} + +.question-option:has(.question-input:checked) .question-check { + opacity: 1; +} + +.question-add, +.question-add .question-key { + color: var(--color-text-tertiary); +} + +.question-add .question-key > span { + background: var(--color-inset); +} + +:is(.question-add, .question-adding) { + align-items: center; + padding-block: 0.5rem; +} + +.question-add:hover .question-text, +.question-add:hover .question-key { + color: var(--color-text-secondary); +} + +.question-adding { + outline: var(--edge-width) solid var(--color-control-edge); + outline-offset: calc(-1 * var(--edge-width)); +} + +.question-adding:focus-within { + outline-color: var(--color-brand); +} + +.question-field { + flex: 1; + min-width: 0; + padding: 0; + border: 0; + background: transparent; + font: inherit; + color: var(--color-text-primary); + outline: none; + resize: none; + field-sizing: content; +} + +.question-field:focus-visible { + outline: none; +} + +.question-field::placeholder { + color: var(--color-text-tertiary); +} + +.question-card[data-saving] .question-options { + opacity: 0.6; +} + +/* Matches the research card: danger badge beside pale-red callout text. */ +.question-callout { + display: flex; + align-items: flex-start; + gap: 0.5rem; + margin: 0.5rem 0.5rem 0; + padding: 0.5rem 0.625rem 0.5rem 0.5rem; +} + +.question-callout p { + display: grid; +} + +.question-callout strong { + font-weight: 500; +} + +.question-actions { + display: flex; + align-items: center; + justify-content: flex-end; + gap: 0.375rem; + margin-block-start: 0.75rem; + padding-inline: 0.5rem; +} + +@media (prefers-reduced-motion: no-preference) { + .question-actions > * { + animation: question-fade var(--duration-fast) var(--ease-out); + } +} + +@keyframes question-fade { + from { + opacity: 0; + } +} + +/* Discard confirm: the footer swaps in place and fades in. */ +.question-confirm { + margin-inline-end: auto; + font-size: var(--text-sm); + font-weight: 500; + color: var(--color-text-primary); +} + +/* Footer stepper, left: ‹ question name n/m ›, 28px carets and --text-sm. */ +.question-stepper { + display: inline-flex; + min-width: 0; + align-items: center; + gap: 0.125rem; + margin-inline: -0.375rem auto; +} + +/* 28px button, 16px chevron: drop btn-icon's padding so the icon isn't squeezed. */ +.question-caret { + padding: 0; +} + +.question-caret[data-flip] [data-nucleo-icon] { + rotate: 180deg; +} + +.question-count { + display: inline-flex; + min-width: 0; + gap: 0.375rem; + padding-inline: 0.125rem; + font-size: var(--text-sm); + color: var(--color-text-tertiary); + font-variant-numeric: tabular-nums; +} + +.question-count strong { + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + font-weight: 500; + color: var(--color-text-primary); +} + /* * What focus looks like, once, for everything that can take it. An outline * rather than a ring: it follows `border-radius` and takes no space in the diff --git a/apps/web/src/tokens.test.ts b/apps/web/src/tokens.test.ts index 79d52c5b..3a03bdb1 100644 --- a/apps/web/src/tokens.test.ts +++ b/apps/web/src/tokens.test.ts @@ -476,7 +476,8 @@ type StandardAction = { action: string; marker: string; size: "btn-sm" | "btn-md" | "btn-icon"; - tiers: readonly ("btn-primary" | "btn-secondary" | "btn-ghost" | "btn-destructive")[]; + tiers: + readonly ("btn-primary" | "btn-secondary" | "btn-outline" | "btn-ghost" | "btn-destructive")[]; }; function classLists(button: string): string[][] { @@ -515,7 +516,7 @@ function standardButtonOffenders(source: string, file: string, action: StandardA for (let list of classes) { let sizes = list.filter(name => /^(btn-sm|btn-md|btn-icon)$/.test(name)); let currentTiers = list.filter(name => - /^(btn-primary|btn-secondary|btn-ghost|btn-destructive)$/.test(name) + /^(btn-primary|btn-secondary|btn-outline|btn-ghost|btn-destructive)$/.test(name) ); let legacy = list.filter(name => /^(bg|px|py)-/.test(name)); tiers.push(...currentTiers); @@ -595,27 +596,6 @@ describe("migration", () => { tag: "textarea", utility: "field", }, - { - file: "packages/question/src/react/question-view.tsx", - marker: "Type another answer", - name: "custom questionnaire answer", - tag: "textarea", - utility: "field", - }, - { - file: "packages/question/src/react/question-view.tsx", - marker: "checked={!custom && selected}", - name: "questionnaire option choice", - tag: "input", - utility: "choice-control", - }, - { - file: "packages/question/src/react/question-view.tsx", - marker: "checked={active}", - name: "custom questionnaire choice", - tag: "input", - utility: "choice-control", - }, ]; let offenders = controls.flatMap(control => controlOffenders( @@ -666,7 +646,7 @@ describe("migration", () => { let previousGuardWouldAccept = classes.includes("btn") && classes.filter(name => /^(btn-sm|btn-md|btn-icon)$/.test(name)).length === 1 && classes.filter(name => - /^(btn-primary|btn-secondary|btn-ghost|btn-destructive)$/.test(name) + /^(btn-primary|btn-secondary|btn-outline|btn-ghost|btn-destructive)$/.test(name) ).length === 1; expect(previousGuardWouldAccept).toBe(true); @@ -784,7 +764,7 @@ describe("migration", () => { action: "Keep it", marker: "setConfirming(false)", size: "btn-sm", - tiers: ["btn-secondary"], + tiers: ["btn-outline"], }], ["packages/question/src/react/question-view.tsx", { action: "cancel confirmation", @@ -793,13 +773,13 @@ describe("migration", () => { tiers: ["btn-destructive"], }], ["packages/question/src/react/question-view.tsx", { - action: "Cancel", + action: "Discard", marker: "setConfirming(true)", size: "btn-sm", - tiers: ["btn-secondary"], + tiers: ["btn-outline"], }], ["packages/question/src/react/question-view.tsx", { - action: "Submit", + action: "Save", marker: "onClick={onSubmit}", size: "btn-sm", tiers: ["btn-primary"], diff --git a/e2e/responsive-decisions.e2e.ts b/e2e/responsive-decisions.e2e.ts index 01620d02..fe305aa2 100644 --- a/e2e/responsive-decisions.e2e.ts +++ b/e2e/responsive-decisions.e2e.ts @@ -89,7 +89,7 @@ for (let viewport of [{ width: 320, height: 568 }, { width: 390, height: 844 }]) await page.goto(`/channels/${room}`); let card = questionnaire(page).filter({ hasText: LONG_QUESTIONS[0]!.header }); await expect(card).toBeVisible(); - await expect(card.getByRole("textbox", { name: /Custom answer for/ })).toHaveCount(0); + await expect(card.getByRole("textbox", { name: "Add an option" })).toHaveCount(0); await expectNoHorizontalOverflow(page); let firstChoice = card.getByRole("radio", { name: "Use the compact layout" }); @@ -108,8 +108,8 @@ for (let viewport of [{ width: 320, height: 568 }, { width: 390, height: 844 }]) await expect(firstChoice).toBeChecked(); let actions = [ - card.getByRole("button", { name: "Cancel" }), - card.getByRole("button", { name: "Save answer" }), + card.getByRole("button", { name: "Discard", exact: true }), + card.getByRole("button", { name: "Save", exact: true }), ]; for (let action of actions) { await action.scrollIntoViewIfNeeded(); @@ -132,10 +132,10 @@ for (let viewport of [{ width: 320, height: 568 }, { width: 390, height: 844 }]) .getByRole("button", { name: /Decisions, 8 unanswered/ }), ).toBeVisible(); - let customChoice = card.getByRole("radio", { name: "Write a custom answer" }); + let customChoice = card.getByRole("radio", { name: "Add an option" }); await customChoice.focus(); await page.keyboard.press("Space"); - let custom = card.getByRole("textbox", { name: /Custom answer for/ }); + let custom = card.getByRole("textbox", { name: "Add an option" }); await expect(custom).toBeFocused(); await setVisualViewport(page, { event: "resize", height: 360 }); await expect.poll(() => diff --git a/e2e/sidecar.e2e.ts b/e2e/sidecar.e2e.ts index b82c5b3d..4cb55b98 100644 --- a/e2e/sidecar.e2e.ts +++ b/e2e/sidecar.e2e.ts @@ -110,9 +110,11 @@ test("question step swaps overlap only for pointer input", async ({ join, seed } let outgoing = stack.locator( ':scope > [data-content-swap-state="outgoing"]:not([hidden])', ); - let scope = card.getByRole("tab", { name: "Scope" }); + let next = card.getByRole("button", { name: "Next question" }); + let previous = card.getByRole("button", { name: "Previous question" }); + let count = card.getByText("2/2"); - await scope.click(); + await next.click(); await expect(visible).toHaveCount(2); await expect(stack.locator(":scope > [data-content-swap-state]:not([hidden]):not([inert])")) .toHaveCount(1); @@ -140,14 +142,12 @@ test("question step swaps overlap only for pointer input", async ({ join, seed } expect(accessibility).toEqual({ duplicateIds: [], invalidReferences: [] }); await expect(visible).toHaveCount(1); - await scope.focus(); - await page.keyboard.press("ArrowLeft"); + await expect(previous).toBeFocused(); + await previous.click(); await expect(visible).toHaveCount(1); await expect(outgoing).toHaveCount(0); - await expect(card.getByRole("tab", { name: "Rollout" })).toHaveAttribute( - "aria-selected", - "true", - ); + await expect(card.getByText("1/2")).toBeVisible(); + await expect(card.getByRole("heading", { name: "How should we deploy?" })).toBeVisible(); await page.emulateMedia({ reducedMotion: "reduce" }); await stack.evaluate(root => { @@ -164,7 +164,7 @@ test("question step swaps overlap only for pointer input", async ({ join, seed } Reflect.set(window, "__questionStepObserver", observer); recordActiveCount(); }); - await scope.click(); + await next.click(); await expect(visible).toHaveCount(1); let activeCounts = await page.evaluate(() => { let observer = Reflect.get(window, "__questionStepObserver") as MutationObserver; @@ -172,7 +172,8 @@ test("question step swaps overlap only for pointer input", async ({ join, seed } return Reflect.get(window, "__questionStepActiveCounts") as number[]; }); expect(activeCounts).not.toContain(0); - await expect(scope).toHaveAttribute("aria-selected", "true"); + await expect(count).toBeVisible(); + await expect(card.getByRole("heading", { name: "What belongs in the first cut?" })).toBeVisible(); }); async function rewriteFirstBlock(page: import("@playwright/test").Page, value: string) { @@ -262,7 +263,9 @@ test( await decisions.click(); await expect(questionnaire(page)).toHaveCount(2); - await expect(questionnaire(page).getByRole("heading", { name: "Storage" })).toBeVisible(); + await expect( + questionnaire(page).getByRole("heading", { name: "Where should room state live?" }), + ).toBeVisible(); await expect(page.locator('[data-document-view="decisions"] [data-plan-sidecar-thread]')) .toHaveCount(0); }, @@ -278,8 +281,10 @@ test( .toHaveAttribute("aria-pressed", "true"); let card = questionnaire(page); await expect(card).toHaveCount(2); - await expect(card.getByRole("heading", { name: "Storage" })).toBeVisible(); - await expect(card.getByRole("heading", { name: "Scope" })).toBeVisible(); + await expect(card.getByRole("heading", { name: "Where should room state live?" })) + .toBeVisible(); + await expect(card.getByRole("heading", { name: "Which of these belong in the first cut?" })) + .toBeVisible(); await expect(card.getByRole("tablist")).toHaveCount(0); }, ); @@ -547,21 +552,22 @@ test("decision cards save independently with progressive custom answers", async await seed(PROSE); let page = await join("ana"); await page.getByRole("button", { name: /^Decisions/ }).click(); - let storage = questionnaire(page).filter({ has: page.getByRole("heading", { name: "Storage" }) }); - let scope = questionnaire(page).filter({ has: page.getByRole("heading", { name: "Scope" }) }); - let saveStorage = storage.getByRole("button", { name: "Save answer" }); + let storage = questionnaire(page).filter({ + has: page.getByRole("heading", { name: "Where should room state live?" }), + }); + let scope = questionnaire(page).filter({ + has: page.getByRole("heading", { name: "Which of these belong in the first cut?" }), + }); + let saveStorage = storage.getByRole("button", { name: "Save", exact: true }); - await expect(storage.getByRole("textbox", { name: /Custom answer for/ })).toHaveCount(0); - await expect(scope.getByRole("textbox", { name: /Custom answer for/ })).toHaveCount(0); - let check = saveStorage.locator('svg[data-plan-icon="check"]'); - await expect(check).toHaveCount(1); - await expect(check).toHaveAttribute("aria-hidden", "true"); + await expect(storage.getByRole("textbox", { name: "Add an option" })).toHaveCount(0); + await expect(scope.getByRole("textbox", { name: "Add an option" })).toHaveCount(0); await storage.getByRole("radio", { name: /On disk as MDX/ }).check(); await saveStorage.click(); await expect(scope).toBeVisible(); await expect(scope).toBeFocused(); - await expect(scope.getByRole("button", { name: "Save answer" })).toBeVisible(); + await expect(scope.getByRole("button", { name: "Save", exact: true })).toBeVisible(); await expect(scope).not.toContainText("Answered by"); await expect(scope.getByRole("checkbox", { name: "Anchors" })).not.toBeChecked(); @@ -602,7 +608,7 @@ test("decision cards save independently with progressive custom answers", async let resolved = questionnaire(page).filter({ hasText: "Where should room state live?" }); await expect(resolved).toContainText("On disk as MDX"); await expect(resolved).toContainText("Answered by @ana"); - await expect(resolved.getByRole("button", { name: "Save answer" })).toHaveCount(0); + await expect(resolved.getByRole("button", { name: "Save", exact: true })).toHaveCount(0); let iconStarts = await page.evaluate(() => { let record = Reflect.get(window, "__feedbackIconTransitions") as { starts: number }; return record.starts; @@ -637,12 +643,12 @@ test("decision cards save independently with progressive custom answers", async await expect(history).not.toHaveAttribute("aria-controls"); await expect(historyContent).toHaveCount(0); - let customChoice = scope.getByRole("checkbox", { name: "Write a custom answer" }); + let customChoice = scope.getByRole("checkbox", { name: "Add an option" }); await customChoice.focus(); await page.keyboard.press("Space"); - let custom = scope.getByRole("textbox", { name: "Custom answer for Scope" }); + let custom = scope.getByRole("textbox", { name: "Add an option" }); await expect(custom).toBeFocused(); - let saveScope = scope.getByRole("button", { name: "Save answer" }); + let saveScope = scope.getByRole("button", { name: "Save", exact: true }); await saveScope.hover(); await page.evaluate(() => { let record = { starts: 0 }; @@ -673,11 +679,11 @@ test("decision cards save independently with progressive custom answers", async await custom.fill("Only collaborative anchors"); await scope.getByRole("checkbox", { name: "Anchors" }).check(); await expect(custom).toHaveCount(0); - await customChoice.check(); - custom = scope.getByRole("textbox", { name: "Custom answer for Scope" }); + await customChoice.click(); + custom = scope.getByRole("textbox", { name: "Add an option" }); await expect(custom).toHaveValue("Only collaborative anchors"); await expect(custom).toBeFocused(); - await scope.getByRole("button", { name: "Save answer" }).click(); + await scope.getByRole("button", { name: "Save", exact: true }).click(); await expect(questionnaire(page).filter({ hasText: "Which of these belong in the first cut?" })) .toContainText("Only collaborative anchors"); }); @@ -686,26 +692,31 @@ test("an unanswered decision reports its own validation error", async ({ join, s await seed(PROSE); let page = await join("ana"); await page.getByRole("button", { name: /^Decisions/ }).click(); - let card = questionnaire(page).filter({ has: page.getByRole("heading", { name: "Scope" }) }); + let card = questionnaire(page).filter({ + has: page.getByRole("heading", { name: "Which of these belong in the first cut?" }), + }); - await card.getByRole("button", { name: "Save answer" }).click(); + await card.getByRole("button", { name: "Save", exact: true }).click(); await expect(card.getByRole("alert")).toBeVisible(); await expect(card).not.toContainText("Answered by"); }); -test("cancelling asks first", async ({ join, seed }) => { +test("discarding asks first", async ({ join, seed }) => { await seed(PROSE); let page = await join("ana"); await page.getByRole("button", { name: /^Decisions/ }).click(); - let card = questionnaire(page).filter({ has: page.getByRole("heading", { name: "Scope" }) }); + let card = questionnaire(page).filter({ + has: page.getByRole("heading", { name: "Which of these belong in the first cut?" }), + }); - await card.getByRole("button", { name: "Cancel" }).click(); + await card.getByRole("button", { name: "Discard", exact: true }).click(); + await expect(card.getByText("Discard this decision?")).toBeVisible(); let keep = card.getByRole("button", { name: "Keep it" }); await expect(keep).toBeVisible(); await keep.click(); - await expect(card.getByRole("button", { name: "Save answer" })).toBeVisible(); + await expect(card.getByRole("button", { name: "Save", exact: true })).toBeVisible(); }); test("a marked passage has document chrome with a hover preview", async ({ join, seed }) => { diff --git a/packages/editor/src/widgets/questionnaire.test.tsx b/packages/editor/src/widgets/questionnaire.test.tsx index 4a71e024..d35347a6 100644 --- a/packages/editor/src/widgets/questionnaire.test.tsx +++ b/packages/editor/src/widgets/questionnaire.test.tsx @@ -15,7 +15,7 @@ const SINGLE = { }], }; -test("a single decision renders as a saveable card without tabs", () => { +test("a single decision renders as a saveable card without a stepper", () => { let markup = renderToStaticMarkup( createElement(QuestionView, { definition: SINGLE, @@ -24,14 +24,15 @@ test("a single decision renders as a saveable card without tabs", () => { }), ); - expect(markup).toContain("Decision"); - expect(markup).toContain("Save answer"); - expect(markup).toContain('data-plan-icon="check"'); - expect(markup).toContain('aria-hidden="true"'); - expect(markup).not.toContain('role="tablist"'); + expect(markup).toContain("Where should room state live?"); + expect(markup).toContain("Add an option"); + expect(markup).toContain(">Save<"); + expect(markup).not.toContain("Save answer"); + expect(markup).not.toContain("Questions"); + expect(markup).not.toContain(">Next<"); }); -test("a stored multi-question questionnaire keeps its tabbed compatibility view", () => { +test("a stored multi-question questionnaire keeps its stepper compatibility view", () => { let markup = renderToStaticMarkup( createElement(QuestionView, { definition: { @@ -51,8 +52,11 @@ test("a stored multi-question questionnaire keeps its tabbed compatibility view" }), ); - expect(markup).toContain('role="tablist"'); - expect(markup).toContain("Scope"); + expect(markup).not.toContain('role="tablist"'); + expect(markup).toContain("Previous question"); + expect(markup).toContain("Storage"); + expect(markup).toContain("1/2"); + expect(markup).toContain(">Next<"); }); test("a stored unanswered questionnaire keeps its compatibility view without a live record", () => { @@ -82,11 +86,11 @@ test("a stored unanswered questionnaire keeps its compatibility view without a l }), ); - expect(markup).toContain('role="tablist"'); + expect(markup).not.toContain('role="tablist"'); expect(markup).toContain("disabled"); expect(markup).toContain("Next"); - expect(markup).not.toContain("Save answer"); - expect(markup).not.toContain("Cancel"); + expect(markup).not.toContain(">Save<"); + expect(markup).not.toContain("Discard"); }); test("a host motion contract owns the active question step", () => { @@ -148,6 +152,6 @@ test("a read-only decision remains linked but has no answer actions", () => { expect(markup).toContain("show in plan"); expect(markup).toContain("disabled"); - expect(markup).not.toContain("Save answer"); - expect(markup).not.toContain("Cancel"); + expect(markup).not.toContain(">Save<"); + expect(markup).not.toContain("Discard"); }); diff --git a/packages/icons/src/index.ts b/packages/icons/src/index.ts index 76f0d3ba..00599a65 100644 --- a/packages/icons/src/index.ts +++ b/packages/icons/src/index.ts @@ -5,6 +5,7 @@ export { ChevronIcon, CloseIcon, CodeIcon, + DecisionIcon, InfoIcon, LightbulbIcon, LinkPlusIcon, diff --git a/packages/icons/src/line.tsx b/packages/icons/src/line.tsx index 2790d566..8cb78bd6 100644 --- a/packages/icons/src/line.tsx +++ b/packages/icons/src/line.tsx @@ -44,6 +44,17 @@ export function CheckIcon(props: IconProps) { ); } +export function DecisionIcon(props: IconProps) { + return ( + + + + + + + ); +} + export function InfoIcon(props: IconProps) { return ( diff --git a/packages/question/src/react/question-view.test.ts b/packages/question/src/react/question-view.test.ts index d6be1fea..fa5f404a 100644 --- a/packages/question/src/react/question-view.test.ts +++ b/packages/question/src/react/question-view.test.ts @@ -77,3 +77,81 @@ test("a host can present an error as motion feedback", () => { expect(markup).toContain('role="alert"'); expect(markup).toContain('data-motion-feedback="alert"'); }); + +const ROLLOUT = { + id: "rollout", + header: "Rollout", + question: "How should we roll this out?", + multiple: false, + options: [ + { id: "all", label: "All at once", description: "Everyone moves on the same day" }, + { id: "team", label: "Team by team", description: "" }, + ], +}; + +test("options carry letter tiles and the last row offers to add one", () => { + let markup = renderToStaticMarkup(createElement(QuestionView, { + definition: { questions: [ROLLOUT] }, + drafts: {}, + onCancel() {}, + onSubmit() {}, + })); + + expect(markup).toContain(">A<"); + expect(markup).toContain(">B<"); + expect(markup.lastIndexOf("Add an option")).toBeGreaterThan(markup.indexOf("Team by team")); + expect(markup).toContain(">Discard<"); + expect(markup).toContain(">Save<"); + expect(markup).not.toContain("Choose any"); + expect(markup).not.toContain("Write a custom answer"); +}); + +test("an existing custom answer opens the add row as a field with the next letter", () => { + let markup = renderToStaticMarkup(createElement(QuestionView, { + definition: { questions: [ROLLOUT] }, + drafts: { rollout: { mode: "custom", choice: "", options: {}, custom: "Opt-in beta" } }, + })); + + expect(markup).toContain("C<"); +}); + +test("a multiple-choice question says so once, under its title", () => { + let markup = renderToStaticMarkup(createElement(QuestionView, { + definition: { questions: [{ ...ROLLOUT, multiple: true }] }, + drafts: {}, + })); + + expect(markup.match(/Choose any/g)).toHaveLength(1); + expect(markup).toContain('type="checkbox"'); +}); + +test("several questions use a stepper, and only the last one saves", () => { + let second = { ...ROLLOUT, id: "pilot", header: "Pilot team", question: "Who pilots it?" }; + let markup = renderToStaticMarkup(createElement(QuestionView, { + definition: { questions: [ROLLOUT, second] }, + drafts: {}, + onSubmit() {}, + })); + + expect(markup).not.toContain('role="tablist"'); + expect(markup).toContain("Rollout"); + expect(markup).toContain("1/2"); + expect(markup).toContain(">Next<"); + expect(markup).not.toContain(">Save<"); +}); + +test("a failed save explains itself in a callout and offers another try", () => { + let markup = renderToStaticMarkup(createElement(QuestionView, { + definition: { questions: [ROLLOUT] }, + drafts: {}, + error: "Check your connection and try again.", + onSubmit() {}, + })); + + expect(markup).toContain("Couldn’t save"); + expect(markup).toContain("Check your connection and try again."); + expect(markup).toContain('role="alert"'); + expect(markup).toContain("Try again"); +}); diff --git a/packages/question/src/react/question-view.tsx b/packages/question/src/react/question-view.tsx index 5a2cc8ab..704ef660 100644 --- a/packages/question/src/react/question-view.tsx +++ b/packages/question/src/react/question-view.tsx @@ -9,12 +9,10 @@ * as they arrive rather than tracking local state. */ -import { useCallback, useEffect, useId, useRef, useState } from "react"; -import { CheckIcon, CloseIcon } from "@chopin/icons"; +import { useEffect, useId, useRef, useState } from "react"; +import { CheckIcon, ChevronIcon, DecisionIcon, PlusIcon, WarningIcon } from "@chopin/icons"; -import { answered } from "../draft"; - -import type { KeyboardEvent, ReactNode } from "react"; +import type { ReactNode } from "react"; import type { Draft, Drafts } from "../draft"; import type { Answer, Definition, Item } from "../schema"; @@ -99,6 +97,19 @@ function DecisionHeading() { ); } +function letter(index: number): string { + return String.fromCharCode(65 + index); +} + +/** A line-tall slot, so the tile centres on the label's first line, not the whole row. */ +function Key({ children }: { children: ReactNode }) { + return ( + + ); +} + function Choices( { question, draft, disabled, name, onChange }: { question: Item; @@ -111,19 +122,14 @@ function Choices( let custom = draft?.mode === "custom"; return ( -
- {question.header} - - {question.options.map(option => { + <> + {question.options.map((option, index) => { let selected = question.multiple ? !!draft?.options[option.id] : draft?.choice === option.id; return ( -
+ ); } +/** The last row: a prompt to add an option, which becomes the field for it. */ function Custom( { question, draft, disabled, name, onChange }: { question: Item; @@ -195,36 +204,50 @@ function Custom( return () => viewport.removeEventListener("resize", reveal); }, []); - return ( -
-