diff --git a/ui/src/components/AdapterLoginChrome.test.tsx b/ui/src/components/AdapterLoginChrome.test.tsx new file mode 100644 index 0000000000..6d2499135c --- /dev/null +++ b/ui/src/components/AdapterLoginChrome.test.tsx @@ -0,0 +1,104 @@ +// @vitest-environment jsdom + +import { createRoot, type Root } from "react-dom/client"; +import { flushSync } from "react-dom"; +import { afterEach, describe, expect, it } from "vitest"; + +import { + OnboardingLoginCard, + OnboardingLoginCodeInput, + onboardingCardInputClass, +} from "./AdapterLoginChrome"; +import { ApiKeyField } from "./onboarding/ConnectInputCanvas"; + +/** + * The connect step's canvas holds one of two cards, and the credential switch + * above trades between them. They are two answers to one question, so they have + * to be built the same way — and the last time they were only *matched*, by + * restating each other's measurements, they drifted the moment one was redrawn. + * + * These tests pin the sharing rather than the appearance. A colour or a radius + * is the design's to change; what must not change is that both cards get it + * from the same declaration. + */ + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +(globalThis as any).IS_REACT_ACT_ENVIRONMENT = true; + +async function act(callback: () => void | Promise) { + let result: void | Promise = undefined; + flushSync(() => { + result = callback(); + }); + await result; +} + +let roots: Root[] = []; + +afterEach(async () => { + for (const root of roots) { + await act(async () => root.unmount()); + } + roots = []; + document.body.innerHTML = ""; +}); + +async function render(node: React.ReactNode): Promise { + const container = document.createElement("div"); + document.body.appendChild(container); + const root = createRoot(container); + roots.push(root); + await act(async () => root.render(node)); + return container; +} + +describe("the connect step's two cards", () => { + it("gives the key field and the sign-in field the same input, from one declaration", async () => { + // The assertion that would have caught the drift this file exists for: not + // "both look like X", which passes right up until one of them is restyled, + // but that the two carry the byte-identical class string. + const signIn = await render( + + {}} onSubmit={() => {}} /> + , + ); + const keyCard = await render( + {}} />, + ); + + const codeInput = signIn.querySelector("input")!; + const keyInput = keyCard.querySelector("input")!; + expect(codeInput.className).toBe(onboardingCardInputClass); + expect(keyInput.className).toBe(codeInput.className); + }); + + it("wraps the key field in the same card shell as the sign-in", async () => { + // The shell, not just the input. The key field used to draw its own + // bordered box, so flipping the credential switch changed the shape of the + // step rather than its content. + const signIn = await render( + + {}} onSubmit={() => {}} /> + , + ); + const keyCard = await render( + {}} />, + ); + + expect(keyCard.firstElementChild!.className).toBe(signIn.firstElementChild!.className); + }); + + it("labels the key field with the variable it will be written to", async () => { + // The name answers what a paster cannot answer for themselves — where this + // step puts the key — so it is the label rather than a sentence about it, + // and it reaches assistive tech as the field's name too. + const keyCard = await render( + {}} />, + ); + + expect(keyCard.textContent).toContain("ANTHROPIC_API_KEY"); + expect(keyCard.querySelector("input")!.getAttribute("aria-label")).toBe("ANTHROPIC_API_KEY"); + // Still a password field: the key is a secret even while being pasted. + expect(keyCard.querySelector("input")!.getAttribute("type")).toBe("password"); + }); +}); diff --git a/ui/src/components/AdapterLoginChrome.tsx b/ui/src/components/AdapterLoginChrome.tsx index 23a700ab22..cbf38220bd 100644 --- a/ui/src/components/AdapterLoginChrome.tsx +++ b/ui/src/components/AdapterLoginChrome.tsx @@ -35,7 +35,12 @@ export function OnboardingLoginCard({ onCancel, children, }: { - instruction: string; + /** + * A node rather than a string: the sign-in cards pass a sentence, and the + * key card passes the environment variable it will write to, which has to + * be mono to read as a name rather than as prose. + */ + instruction: ReactNode; onCancel?: () => void; children: ReactNode; }) { @@ -201,6 +206,19 @@ export function OnboardingLoginCodeRow({ code }: { code: string }) { ); } +/** + * One row's worth of input, shared by every card that takes one. + * + * Exported rather than duplicated because the two inputs that use it — the + * browser code here and the API key on the credential card — sit in the same + * canvas one toggle apart, so a divergence between them is visible by flipping + * a switch. They differ in what they hold, not in what they look like. + */ +export const onboardingCardInputClass = + "h-(--sz-44px) w-full rounded-lg bg-muted px-5 font-mono text-xs text-foreground " + + "placeholder:font-sans placeholder:text-sm placeholder:text-muted-foreground " + + "outline-none focus-visible:ring-ring/50 focus-visible:ring-(length:--rad-3)"; + /** * The field the browser code is pasted back into. * @@ -245,7 +263,7 @@ export function OnboardingLoginCodeInput({ onSubmit(); } }} - className="h-(--sz-44px) w-full rounded-lg bg-muted px-5 font-mono text-xs text-foreground placeholder:font-sans placeholder:text-sm placeholder:text-muted-foreground outline-none focus-visible:ring-ring/50 focus-visible:ring-(length:--rad-3)" + className={onboardingCardInputClass} /> ); } diff --git a/ui/src/components/onboarding/ConnectInputCanvas.tsx b/ui/src/components/onboarding/ConnectInputCanvas.tsx index 89d4a0492c..ea0db7f6af 100644 --- a/ui/src/components/onboarding/ConnectInputCanvas.tsx +++ b/ui/src/components/onboarding/ConnectInputCanvas.tsx @@ -1,7 +1,10 @@ import { useLayoutEffect, useRef, type ReactNode } from "react"; import { AnimatePresence, motion } from "motion/react"; -import { cn } from "../../lib/utils"; +import { + OnboardingLoginCard, + onboardingCardInputClass, +} from "../AdapterLoginChrome"; import { CANVAS_CONTENT_ENTER, CANVAS_ENTER_TRAVEL, @@ -105,20 +108,24 @@ export function ConnectInputCanvas({ * The API key field, for when the credential mode is keys rather than a * subscription. * - * Built to the login panel's shape on purpose: same card, same padding, same - * label-left / control-right row, same 28px control height. These two are - * alternatives to each other — one canvas shows one or the other, and the - * credential switch above trades between them — so they should read as two - * answers to one question rather than as two different kinds of thing. Before - * this the key field was a stacked label over a full-width input with no card - * at all, and flipping the mode changed the shape of the step rather than its - * content. + * Built to the sign-in card's shape on purpose, and that reasoning is + * unchanged from when it was written — only its target moved. These two are + * alternatives to each other: one canvas shows one or the other and the + * credential switch above trades between them, so they have to read as two + * answers to one question rather than as two different kinds of thing. It was + * matched to the old bordered panel; the connect step's sign-in became a + * borderless card with 44px rows, and this stayed behind, so flipping the + * toggle changed the shape of the step rather than its content — the exact + * failure the original note was written to prevent. + * + * It now composes the same primitives rather than restating their measurements, + * which is what keeps that from happening again. * * The variable name is the label rather than a sentence about it. Someone - * pasting a key knows which one they are holding; what they cannot know is where - * this step will put it, and the name answers that in the place it is asked — - * while staying short enough to sit opposite the field the way "Sign in to the - * environment" sits opposite its button. + * pasting a key knows which one they are holding; what they cannot know is + * where this step will put it, and the name answers that in the place it is + * asked. It takes the instruction slot the sign-in cards use for their + * sentence, in mono, because it is a name and not prose. */ export function ApiKeyField({ envKey, @@ -139,29 +146,20 @@ export function ApiKeyField({ }, []); return ( -
- -
+ {envKey}} + > + onChange(event.target.value)} + placeholder="Paste your key" + className={onboardingCardInputClass} + /> + ); }