From 914645b047107ab3a7e77a276c728297c94ac387 Mon Sep 17 00:00:00 2001 From: Giuseppe Scuglia Date: Fri, 16 Oct 2026 11:25:58 +0200 Subject: [PATCH] refactor(studio): give Toggle the phone glyph rule and use its ghost variant in the composer Toggle now renders data-toggle after its props spread, as Button renders data-button, and the phone glyph rule in styles.css keys on it, so a Toggle's bare icon draws at 20px on phones without a hand-written size-4. The composer and side-thread dictation Toggles drop theirs and share one class on the ghost variant. The phone sheet's Mode and Model rows and Effort pills use the ghost variant instead of strings that undid the default variant's pressed fill; the rows inset the ghost focus ring so the scrolling sheet doesn't clip it. The model rows drop a dark pressed-hover token that never applied: the dark fill already sorts after hover. button.tsx notes that borrowing buttonVariants doesn't bring data-button. Closes #2380 Co-Authored-By: Claude Opus 5.5 (1M context) --- apps/web/e2e/composer-toggles.spec.ts | 230 ++++++++++++++++++ apps/web/src/components/ui/button.tsx | 2 + apps/web/src/components/ui/toggle.test.tsx | 20 ++ apps/web/src/components/ui/toggle.tsx | 3 + apps/web/src/features/chat/chat-composer.tsx | 15 +- .../features/chat/composer-options-sheet.tsx | 13 +- .../chat/composer-toggles.pinning.test.tsx | 16 ++ .../web/src/features/chat/dictation-toggle.ts | 9 + .../src/features/chat/model-effort-menu.tsx | 16 +- .../src/features/chat/side-thread-panel.tsx | 20 +- apps/web/src/lib/fonts.test.ts | 1 + apps/web/src/styles.css | 4 +- 12 files changed, 313 insertions(+), 36 deletions(-) create mode 100644 apps/web/e2e/composer-toggles.spec.ts create mode 100644 apps/web/src/features/chat/dictation-toggle.ts diff --git a/apps/web/e2e/composer-toggles.spec.ts b/apps/web/e2e/composer-toggles.spec.ts new file mode 100644 index 0000000000..ff1fc15049 --- /dev/null +++ b/apps/web/e2e/composer-toggles.spec.ts @@ -0,0 +1,230 @@ +// SPDX-License-Identifier: Apache-2.0 + +import type { Locator, Page, TestInfo } from "@playwright/test"; +import { expect, type OfflineBff, test } from "./fixtures"; + +// The composer area's pressed controls are Toggles: the dictation buttons, the +// phone sheet's Mode and Model rows, and its Effort pills. These journeys pin +// what a person sees of them in a real browser: the glyph size the phone rule +// in styles.css draws for a Toggle (20px on phones, as for a Button, and 16px +// on desktop), and how each control's fill and ink relate across rest, hover, +// pressed and pressed-hover, in light and in dark. + +const PHONE = 20; +const DESKTOP = 16; + +function serve(bff: OfflineBff) { + bff.json("GET", "/api/v1/auth/session", { + account: "composer-toggles", + mode: "oidc", + status: "authenticated", + }); + bff.json("GET", "/api/v1/status", { connection: "reachable", signInRequired: false }); + bff.json("GET", "/api/v1/runtime", { + capabilities: { image: false, modelSelection: true, posture: "managed" }, + connection: "online", + features: [], + }); + bff.json("GET", "/api/v1/settings/runtime", { + buildId: "test", + management: { + providerConfiguration: false, + providerConfigurationReason: "", + routingConfiguration: false, + routingConfigurationReason: "", + }, + models: ["Deep model", "Plain model"].map((displayName) => ({ + contextLimit: "200000", + displayName, + id: displayName.split(" ")[0]?.toLowerCase(), + image: false, + providerDisplayName: "Provider", + providerId: "provider", + reasoning: true, + })), + modelsReason: "", + modelsSupported: true, + providerEndpoint: "", + providers: [], + serverImplementation: "test", + }); + bff.json("GET", "/api/v1/sessions", { complete: true, items: [] }); +} + +/** A browser that can dictate, so the dictation Toggle presses. */ +async function fakeDictation(page: Page, theme: "light" | "dark" = "light") { + await page.emulateMedia({ colorScheme: theme }); + await page.addInitScript((value) => { + localStorage.setItem("mecatl-studio-theme", value); + class FakeRecognition { + onresult = null; + onerror = null; + onend: (() => void) | null = null; + continuous = false; + interimResults = false; + lang = ""; + start() {} + stop() { + this.onend?.(); + } + } + Object.defineProperty(window, "SpeechRecognition", { + configurable: true, + value: FakeRecognition, + }); + }, theme); +} + +async function glyphSize(control: Locator) { + await expect(control).toBeVisible(); + return control.locator("svg.lucide").evaluate((svg) => { + const box = svg.getBoundingClientRect(); + return { height: Math.round(box.height), width: Math.round(box.width) }; + }); +} + +function expected(testInfo: TestInfo) { + const size = testInfo.project.name === "chromium-mobile" ? PHONE : DESKTOP; + return { height: size, width: size }; +} + +interface Look { + background: string; + border: string; + color: string; + radius: number; +} + +async function look(control: Locator): Promise { + return control.evaluate((element) => { + const style = getComputedStyle(element); + return { + background: style.backgroundColor, + border: style.borderTopColor, + color: style.color, + radius: Number.parseFloat(style.borderTopLeftRadius), + }; + }); +} + +/** The control's look at rest and under the pointer. */ +async function lookAndHover(page: Page, control: Locator) { + await page.mouse.move(0, 0); + const rest = await look(control); + await control.hover(); + const hover = await look(control); + await page.mouse.move(0, 0); + return { hover, rest }; +} + +const CLEAR = "rgba(0, 0, 0, 0)"; + +test("a Toggle's glyph size at phone and desktop widths", async ({ + offlineBff, + page, +}, testInfo) => { + serve(offlineBff); + await fakeDictation(page); + await page.goto("/workspace/chat"); + const dictation = page.getByRole("button", { name: "Start dictation" }); + expect(await glyphSize(dictation)).toEqual(expected(testInfo)); + await dictation.click(); + expect(await glyphSize(page.getByRole("button", { name: "Stop dictation" }))).toEqual( + expected(testInfo), + ); + + // Any Toggle's bare glyph follows the same rule: the canvas's view switch. + await page.getByRole("button", { name: "Open local canvas" }).click(); + for (const name of ["Preview", "Edit"]) { + expect(await glyphSize(page.getByRole("button", { name }))).toEqual(expected(testInfo)); + } +}); + +for (const theme of ["light", "dark"] as const) { + test(`the composer's Toggles keep their looks in ${theme}`, async ({ + offlineBff, + page, + }, testInfo) => { + // Hover styles need a pointer that hovers, so the phone sheet is measured + // at phone width in the desktop project. + test.skip(testInfo.project.name === "chromium-mobile", "hover needs a hovering pointer"); + serve(offlineBff); + await fakeDictation(page, theme); + await page.goto("/workspace/chat"); + // Measure end states, not a colour transition's first frame. + await page.addStyleTag({ content: "*, *::before, *::after { transition: none !important; }" }); + + // Dictation: ghost icon button ink, brand tint while listening. + const start = page.getByRole("button", { name: "Start dictation" }); + const off = await lookAndHover(page, start); + await start.click(); + const on = await lookAndHover(page, page.getByRole("button", { name: "Stop dictation" })); + expect(off.rest.radius).toBeGreaterThan(1000); + expect(off.rest.background).toBe(CLEAR); + expect(off.hover.background).not.toBe(CLEAR); + expect(off.hover.color).not.toBe(off.rest.color); + expect(on.rest.background).not.toBe(CLEAR); + expect(on.rest.color).not.toBe(off.rest.color); + expect(on.hover.background).not.toBe(on.rest.background); + expect(on.hover.color).toBe(off.hover.color); + await page.getByRole("button", { name: "Stop dictation" }).click(); + + await page.setViewportSize({ height: 844, width: 390 }); + const options = page.getByRole("button", { name: "Composer options" }); + const sheet = page.getByRole("dialog", { name: "Composer options" }); + + // Mode rows: no fill while chosen; the hover fill is the same either way. + await options.click(); + await sheet.getByRole("button", { name: /^Mode\s/ }).click(); + const manual = await lookAndHover(page, sheet.getByRole("button", { name: /^Manual/ })); + const plan = await lookAndHover(page, sheet.getByRole("button", { name: /^Plan/ })); + await expect(sheet.getByRole("button", { name: /^Manual/ })).toHaveAttribute( + "aria-pressed", + "true", + ); + expect(manual.rest).toEqual(plan.rest); + expect(manual.hover).toEqual(plan.hover); + expect(manual.rest.radius).toBe(0); + expect(manual.rest.background).toBe(CLEAR); + expect(manual.hover.background).not.toBe(CLEAR); + expect(manual.hover.color).toBe(manual.rest.color); + await page.keyboard.press("Escape"); + + // Model rows: a zinc fill while chosen. Hovering the chosen row shows the + // hover fill in light and keeps the zinc fill in dark. + await options.click(); + await sheet.getByRole("button", { name: /^Model/ }).click(); + const chosen = sheet.getByRole("button", { exact: true, name: "Default" }); + await expect(chosen).toHaveAttribute("aria-pressed", "true"); + const model = await lookAndHover(page, chosen); + const other = await lookAndHover(page, sheet.getByRole("button", { name: "Deep model" })); + expect(model.rest.radius).toBe(0); + expect(other.rest.radius).toBe(0); + expect(other.rest.background).toBe(CLEAR); + expect(other.hover.background).not.toBe(CLEAR); + expect(other.hover.color).toBe(other.rest.color); + expect(model.rest.background).not.toBe(CLEAR); + expect(model.rest.background).not.toBe(other.hover.background); + expect(model.rest.color).toBe(other.rest.color); + expect(model.hover.color).toBe(model.rest.color); + expect(model.hover.background).toBe( + theme === "dark" ? model.rest.background : other.hover.background, + ); + + // Effort pills: no hover fill; a brand border, fill and ink while chosen. + const effort = sheet.getByRole("group", { name: "Effort" }); + const auto = effort.getByRole("button", { name: "Auto" }); + await expect(auto).toHaveAttribute("aria-pressed", "true"); + const pressed = await lookAndHover(page, auto); + const pill = await lookAndHover(page, effort.getByRole("button", { name: "Low" })); + expect(pressed.rest.radius).toBeGreaterThan(1000); + expect(pill.rest.radius).toBeGreaterThan(1000); + expect(pill.rest.background).toBe(CLEAR); + expect(pill.hover).toEqual(pill.rest); + expect(pressed.hover).toEqual(pressed.rest); + expect(pressed.rest.background).not.toBe(CLEAR); + expect(pressed.rest.border).not.toBe(pill.rest.border); + expect(pressed.rest.color).not.toBe(pill.rest.color); + expect(pressed.rest.color).toBe(on.rest.color); + }); +} diff --git a/apps/web/src/components/ui/button.tsx b/apps/web/src/components/ui/button.tsx index 7608a2ef92..99effb26bf 100644 --- a/apps/web/src/components/ui/button.tsx +++ b/apps/web/src/components/ui/button.tsx @@ -45,6 +45,8 @@ const Button = React.forwardRef( // A Radix trigger with asChild (TooltipTrigger, DropdownMenuTrigger) passes its own // data-slot through props, so data-slot names the outermost part. data-button comes // after the spread and always marks a Button, for styles that apply to every Button. + // An element that only borrows buttonVariants classes has no data-button, so the + // phone glyph rule doesn't size its icons. return ( { + it("tags itself with data-slot and data-toggle, which an asChild trigger keeps", () => { + render( + <> + Bold + + + Italic + + + , + ); + const bold = screen.getByRole("button", { name: "Bold" }); + expect(bold.getAttribute("data-slot")).toBe("toggle"); + expect(bold.hasAttribute("data-toggle")).toBe(true); + const italic = screen.getByRole("button", { name: "Italic" }); + expect(italic.getAttribute("data-slot")).toBe("tooltip-trigger"); + expect(italic.hasAttribute("data-toggle")).toBe(true); + }); + it("flips aria-pressed on click", async () => { const user = userEvent.setup(); const onPressedChange = vi.fn(); diff --git a/apps/web/src/components/ui/toggle.tsx b/apps/web/src/components/ui/toggle.tsx index 220b677ecf..b45fb1c1b8 100644 --- a/apps/web/src/components/ui/toggle.tsx +++ b/apps/web/src/components/ui/toggle.tsx @@ -36,11 +36,14 @@ const Toggle = React.forwardRef< React.ElementRef, React.ComponentPropsWithoutRef & VariantProps >(({ className, size, variant, ...props }, ref) => ( + // As on Button, data-toggle comes after the spread so it survives an asChild + // trigger's data-slot: the phone glyph rule in styles.css keys on it. )); Toggle.displayName = TogglePrimitive.Root.displayName; diff --git a/apps/web/src/features/chat/chat-composer.tsx b/apps/web/src/features/chat/chat-composer.tsx index 35bcc55653..cc391ebe86 100644 --- a/apps/web/src/features/chat/chat-composer.tsx +++ b/apps/web/src/features/chat/chat-composer.tsx @@ -14,7 +14,7 @@ import { useState, } from "react"; import { Tooltip, TooltipContent, TooltipTrigger } from "@/components/ui/tooltip"; -import { Button, buttonVariants } from "../../components/ui/button"; +import { Button } from "../../components/ui/button"; import { Dialog, DialogContent, @@ -44,6 +44,7 @@ import { type ComposerModelControl, ComposerOptionsSheet, } from "./composer-options-sheet"; +import { dictationToggleClass } from "./dictation-toggle"; import { acceptImageAttachments, type ImageAttachment, @@ -791,21 +792,15 @@ export function ChatComposer({ - {/* Sized here: the phone's 20px glyph rule covers size-4 icons and - Button's bare ones, not a Toggle's. */} -
{option.label} @@ -309,6 +313,7 @@ function OptionView({ key={option.value} onPressedChange={() => onSelect(option.value)} pressed={option.value === value} + variant="ghost" > {option.dotClassName && ( { client.clear(); }); + it("shares the composer's look and leaves its glyph to the Toggle phone rule", async () => { + render(); + const composer = screen.getByRole("button", { name: "Start dictation" }); + const composerLook = composer.className; + cleanup(); + const client = await renderSideThread(); + const thread = await screen.findByRole("button", { name: "Start dictation" }); + expect(thread.className).toBe(composerLook); + for (const toggle of [composer, thread]) { + // The phone rule in styles.css sizes a Toggle's glyph that has no size of its own. + expect(toggle.hasAttribute("data-toggle")).toBe(true); + expect(toggle.querySelector("svg")?.getAttribute("class")).not.toMatch(/(^|\s)(size|h|w)-/); + } + client.clear(); + }); + it("is absent where the browser cannot dictate", async () => { Reflect.deleteProperty(window, "SpeechRecognition"); const client = await renderSideThread(); diff --git a/apps/web/src/features/chat/dictation-toggle.ts b/apps/web/src/features/chat/dictation-toggle.ts new file mode 100644 index 0000000000..723ccc7b3c --- /dev/null +++ b/apps/web/src/features/chat/dictation-toggle.ts @@ -0,0 +1,9 @@ +// SPDX-License-Identifier: Apache-2.0 + +/** + * The composer's and the side thread's dictation Toggle, on the ghost Toggle + * variant (`size="sm"`, `variant="ghost"`): an icon button with muted ink, a + * brand tint while listening, and the ghost's hover ink in both states. + */ +export const dictationToggleClass = + "size-8 min-w-0 shrink-0 px-0 text-muted-foreground hover:bg-muted/60 data-[state=on]:bg-brand/10 data-[state=on]:text-brand data-[state=on]:hover:bg-brand/20 data-[state=on]:hover:text-accent-foreground"; diff --git a/apps/web/src/features/chat/model-effort-menu.tsx b/apps/web/src/features/chat/model-effort-menu.tsx index 298146bf90..f329266742 100644 --- a/apps/web/src/features/chat/model-effort-menu.tsx +++ b/apps/web/src/features/chat/model-effort-menu.tsx @@ -270,13 +270,15 @@ export function ModelSheetSection({ const filteredGroups = filterGroups(groupedModels, filter); const normalized = filter.trim().toLocaleLowerCase(); const showDefault = allowUnset && (!normalized || "default".includes(normalized)); - // One Toggle per model, not a single-choice ToggleGroup: that would turn the - // rows into radios with roving focus. Pressing the chosen row picks it again. - // The classes undo the Toggle's own size, shape, ink and pressed fill. As - // before, hovering the chosen row shows the hover fill in light and keeps - // the zinc fill in dark. + // One ghost Toggle per model, not a single-choice ToggleGroup: that would + // turn the rows into radios with roving focus. Pressing the chosen row picks + // it again. The classes undo the Toggle's own size, shape and hover ink, + // inset its focus ring, which the scrolling list would clip, and fill the + // chosen row with zinc. Hovering the chosen row shows the hover fill in + // light; in dark the zinc fill stays, because the dark variant sorts after + // hover, as it did before the rows were Toggles. const rowClass = - "flex h-auto w-full min-w-0 items-center justify-start gap-3 rounded-none px-4 py-3 text-left text-sm font-normal whitespace-normal transition-colors hover:bg-muted/50 hover:text-current disabled:opacity-50 data-[state=on]:bg-zinc-100 data-[state=on]:text-current data-[state=on]:hover:bg-muted/50 dark:data-[state=on]:bg-zinc-800 dark:data-[state=on]:hover:bg-zinc-800"; + "flex h-auto w-full min-w-0 items-center justify-start gap-3 rounded-none px-4 py-3 text-left text-sm font-normal whitespace-normal transition-colors hover:bg-muted/50 hover:text-current focus-visible:ring-inset disabled:opacity-50 data-[state=on]:bg-zinc-100 data-[state=on]:hover:bg-muted/50 dark:data-[state=on]:bg-zinc-800"; return (
@@ -298,6 +300,7 @@ export function ModelSheetSection({ disabled={disabled} onPressedChange={() => onModelChange(undefined)} pressed={model === undefined} + variant="ghost" > Default {model === undefined &&