From 9ef3b087c14686327e4915705ae1fe4537fccc33 Mon Sep 17 00:00:00 2001 From: Tonio Date: Thu, 3 Sep 2026 20:52:31 -0700 Subject: [PATCH] feat(onboarding): round-4 corrections to the connect and agent steps (#12796) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reads the connect and agent steps from the design's own values through the Figma MCP rather than measuring an export, which corrected the arc column inset (433px content, 64px inset — `--sz-68px` goes with the mismeasurement it was minted for) and restored the selected tile's border alongside its fill. Round-4 items: sources named for the provider you sign in with, OpenAI's mark inlined so it can take `currentColor` on a light tile, monochrome autofill via `box-shadow` (Chrome ignores `background-color`), and the agent step's placeholder. Also wires `MODEL_SOURCE_NAMES`, which was added for the rename and never read — the tiles kept passing the display registry's label, so the step still showed "Claude Code" and "Codex" under a heading asking which provider you are signing in to. Covered by a test that fails on the unwired version. --- ui/src/components/OnboardingWizard.test.tsx | 30 ++++++++- ui/src/components/OnboardingWizard.tsx | 63 +++++++++++++++++-- .../onboarding/ModelSourceTiles.tsx | 13 ++-- ui/src/index.css | 27 +++++++- 4 files changed, 121 insertions(+), 12 deletions(-) diff --git a/ui/src/components/OnboardingWizard.test.tsx b/ui/src/components/OnboardingWizard.test.tsx index 6497046447..ed1a69ba95 100644 --- a/ui/src/components/OnboardingWizard.test.tsx +++ b/ui/src/components/OnboardingWizard.test.tsx @@ -1849,6 +1849,31 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", ( return { root, queryClient }; } + it("names the tiles for the provider, not the adapter type", async () => { + // `MODEL_SOURCE_NAMES` exists so this row says "Claude" and "OpenAI" — + // which provider you are signing in to, the question the step's heading + // asks — rather than the display registry's tool names, which the agent + // config screens want. It was added with a long comment justifying it and + // then never read, so the row went on rendering whatever the registry + // supplied: "Claude Code" and "Codex" in the app, and the bare type here, + // since this suite's registry mock returns `label: type`. + mockAdapterRegistry.list = [{ type: "claude_local" }, { type: "codex_local" }]; + const { root } = await openStep4({ adapterType: "claude_local" }); + + const labels = [...document.body.querySelectorAll("button[aria-checked]")].map( + (tile) => tile.textContent ?? "", + ); + expect(labels.length, "both recommended sources should render").toBe(2); + expect(labels.some((l) => l.includes("Claude"))).toBe(true); + expect(labels.some((l) => l.includes("OpenAI"))).toBe(true); + // The negative half is the one that fails on the unwired version: the + // registry label is the adapter type, and it must not reach the tile. + expect(labels.join(" ")).not.toContain("claude_local"); + expect(labels.join(" ")).not.toContain("codex_local"); + + await act(async () => root.unmount()); + }); + it("will not advance on a saved adapter the step no longer offers", async () => { // A draft can name an adapter this registry does not carry — a cloud // sandbox without claude_local, an adapter since disabled. The row hides @@ -2036,7 +2061,10 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", ( // the credential switch instead. What is asserted below is unchanged — // changing the source re-reads the signal — only the route there is. // The tile's text is the label plus its credential tag, hence the prefix. - await clickByText((t) => t.startsWith("codex_local")); + // That label is the provider name now, not the adapter type: this row + // asks which provider you are signing in to, so it reads through + // `MODEL_SOURCE_NAMES` rather than the display registry. + await clickByText((t) => t.startsWith("OpenAI")); expect(mockAgentsApi.getAdapterAuthSignal).toHaveBeenCalledWith( "company-new", diff --git a/ui/src/components/OnboardingWizard.tsx b/ui/src/components/OnboardingWizard.tsx index 09dc2cf6e8..c6b704fe12 100644 --- a/ui/src/components/OnboardingWizard.tsx +++ b/ui/src/components/OnboardingWizard.tsx @@ -201,7 +201,49 @@ function adapterConfigHasAnthropicApiKey(config: Record): boole */ const MODEL_SOURCE_BRAND_MARKS: Record = { claude_local: "/brands/claude-color.svg", - codex_local: "/brands/codex-color.svg", +}; + +/** + * What the connect step calls each source. + * + * Deliberately not the display registry's label, which ten other surfaces read. + * This step asks which *provider* you are signing in with — the panel under the + * row says "Sign in to Anthropic" and "Sign in to OpenAI" — while the agent + * config screens name the tool that runs ("Codex CLI was not found on this + * host"). One rename in the registry would make that message say OpenAI, which + * is vaguer, not clearer. + * + * It is a tension worth naming rather than hiding: DESIGN.md asks for one name + * per concept, and this is two names for one adapter. The concepts are + * different — vendor here, tool there — but if the product decides otherwise, + * this map is the thing to delete. + */ +const MODEL_SOURCE_NAMES: Record = { + claude_local: "Claude", + codex_local: "OpenAI", +}; + +/** + * OpenAI's blossom, inline rather than served from `/brands`. + * + * The supplied asset is a white fill, which was fine while this row only ever + * sat on a dark tile. It follows the reader's system setting now, and white on + * the light tile is invisible. Inlining lets the path take + * `currentColor` and be legible in both, which an `` cannot do. + */ +function OpenAiBlossom({ className }: { className?: string }) { + return ( + + + + ); +} + +const MODEL_SOURCE_INLINE_MARKS: Record> = { + codex_local: OpenAiBlossom, }; /** @@ -228,6 +270,8 @@ function ModelSourceMark({ type: string; Fallback: ComponentType<{ className?: string }>; }) { + const Inline = MODEL_SOURCE_INLINE_MARKS[type]; + if (Inline) return ; const brand = MODEL_SOURCE_BRAND_MARKS[type]; if (!brand) return ; return ; @@ -2175,7 +2219,7 @@ function OnboardingWizardInner({ // tiles stretch and the name field sits under a question far // narrower than itself. isAgentArcStep || step === 1 - ? "w-(--sz-560px) max-w-full px-8 py-10 sm:px-(--sz-68px) sm:py-11" + ? "w-(--sz-560px) max-w-full px-8 py-10 sm:px-(--sz-64px) sm:py-11" : "w-full max-w-md px-8 py-12", )} > @@ -2644,7 +2688,7 @@ function OnboardingWizardInner({ setAgentName(e.target.value)} autoFocus @@ -2675,7 +2719,18 @@ function OnboardingWizardInner({ label="Model source" sources={recommendedAdapters.map((opt) => ({ id: opt.type, - label: opt.label, + // The vendor name where this step has one, the registry's + // tool name where it does not. `MODEL_SOURCE_NAMES` was + // added with the reasoning above it and then never read, + // so the row went on showing "Claude Code" and "Codex" + // — the tool names — under a heading asking which + // provider you are signing in to. + // + // The fallback is what keeps the row rendering if the + // registry ever marks a third adapter `recommended`: + // an unnamed source gets its tool name rather than + // nothing. + label: MODEL_SOURCE_NAMES[opt.type] ?? opt.label, icon: , }))} mode={credentialMode} diff --git a/ui/src/components/onboarding/ModelSourceTiles.tsx b/ui/src/components/onboarding/ModelSourceTiles.tsx index b2cf82c238..92137b53ee 100644 --- a/ui/src/components/onboarding/ModelSourceTiles.tsx +++ b/ui/src/components/onboarding/ModelSourceTiles.tsx @@ -82,16 +82,17 @@ function ModelSourceTile({ // and lending it to focus as well would mean tabbing across the row // looked like picking every tile in turn. "outline-none focus-visible:ring-ring/50 focus-visible:ring-(length:--rad-3)", - // Selection is a lighter surface, not a brighter edge. Both states keep - // the same border — it draws the tile, not the choice — and the fill - // carries the state. A bright stroke on one tile made the row read as - // one outlined object beside one plain one, rather than two of a kind - // with one of them picked. + // Selection is a lighter surface *and* a brighter edge. An earlier pass + // here used the fill alone, reasoning that a bright stroke on one tile + // made the row read as one outlined object beside a plain one. The + // design does both, and it is right: at these sizes one step of fill is + // too quiet to answer "which did I pick?" from across the screen, and + // the stroke is what carries it. // // Hover stops short of the selected fill, so pointing at a tile says // "this one is live" rather than "this one is chosen". selected - ? "border-border bg-accent" + ? "border-foreground/40 bg-accent" : "border-border bg-card hover:bg-accent/40", )} > diff --git a/ui/src/index.css b/ui/src/index.css index 2721a5cccf..ef2d25941c 100644 --- a/ui/src/index.css +++ b/ui/src/index.css @@ -446,6 +446,32 @@ } @layer base { + /* + Autofill, in the app's own colours. + + Chrome paints an autofilled field with a fixed blue-tinted fill of its own — + it ignores `background-color` entirely, which is why nothing in the token + layer reached it and a filled email field came out blue on an otherwise + monochrome screen. `box-shadow` is the one property that does reach it: an + inset shadow thick enough to cover the field repaints the surface, and + `-webkit-text-fill-color` does the same job for the text. + + The transition delay is the standard trick for keeping it: Chrome re-applies + its own fill on interaction, and a delay long enough to outlast the frame is + what stops it flashing back. + */ + input:-webkit-autofill, + input:-webkit-autofill:hover, + input:-webkit-autofill:focus, + input:-webkit-autofill:active, + textarea:-webkit-autofill, + select:-webkit-autofill { + -webkit-text-fill-color: var(--foreground); + caret-color: var(--foreground); + box-shadow: inset 0 0 0 1000px var(--muted); + transition: background-color 100000s ease-in-out 0s; + } + * { @apply border-border outline-ring/50; } @@ -2373,7 +2399,6 @@ span.paperclip-mention-chip[data-mention-kind="external-object"] { --sz-calc-13: calc(0.75rem + 0.5rem); /* Extracted from ui/src/components/IssueRow.tsx (ml-[calc(theme(spacing.3)+theme(spacing.2))]). */ --sz-140px: 140px; /* Extracted from ui/src/components/JsonSchemaForm.tsx (min-h-[140px]). */ --sz-52px: 52px; /* Extracted from ui/src/components/KanbanBoard.tsx (w-[52px]). */ - --sz-68px: 68px; /* The onboarding arc's side inset — 560px frame, 424px column. */ --sz-48px: 48px; /* Extracted from ui/src/components/KanbanBoard.tsx (min-w-[48px]). */ --sz-260px: 260px; /* Extracted from ui/src/components/KanbanBoard.tsx (min-w-[260px]). */ --sz-120px: 120px; /* Extracted from ui/src/components/KanbanBoard.tsx (min-h-[120px]). */