From 8bccad64ce4e81173dce80266eed6fec8c09b6b3 Mon Sep 17 00:00:00 2001 From: Maggie Appleton <5599295+MaggieAppleton@users.noreply.github.com> Date: Tue, 29 Sep 2026 10:31:07 +0100 Subject: [PATCH 01/10] Redesign code block controls as a header bar Language menu becomes a ghost button with a listbox panel, the source toggle an icon button, and the block one bordered box with a shared 0.75rem left edge. Co-Authored-By: Claude Opus 5.5 --- apps/web/src/focus.test.ts | 12 +- apps/web/src/tokens.test.ts | 7 - e2e/code.e2e.ts | 16 +- packages/editor/src/styles.css | 107 ++++++++-- packages/editor/src/widgets/code-view.tsx | 10 +- packages/editor/src/widgets/code.test.ts | 16 +- packages/editor/src/widgets/code.ts | 10 + packages/editor/src/widgets/language-menu.tsx | 195 ++++++++++++++++++ packages/editor/src/widgets/render-blocks.tsx | 39 ++-- packages/icons/src/index.ts | 1 + packages/icons/src/line.tsx | 9 + 11 files changed, 371 insertions(+), 51 deletions(-) create mode 100644 packages/editor/src/widgets/language-menu.tsx diff --git a/apps/web/src/focus.test.ts b/apps/web/src/focus.test.ts index 9c84d7dc..2d9a1aa3 100644 --- a/apps/web/src/focus.test.ts +++ b/apps/web/src/focus.test.ts @@ -40,6 +40,13 @@ const OUTLINE_SUPPRESSION_UTILITY = /\b(?:outline-none|outline-0|outline-hidden|outline-transparent|outline-(?!offset-)[^\s"'`}\]]+\/0\b)/; const FOCUS_TOKEN_OVERRIDE = /--focus-ring-(?:color|width|offset)\s*:/; +/** + * The code block's language listbox takes focus only to hold the keyboard's + * position (aria-activedescendant); its highlighted row is the indicator. The + * block's header draws control focus inside itself because the box clips it. + */ +const EXEMPT_STYLES = new Set(["packages/editor/src/styles.css"]); + /** Files whose markup matches, reported by path so a failure names the offender. */ function offenders(files: string[], pattern: RegExp): string[] { return files @@ -57,7 +64,8 @@ describe("focus", () => { it("leaves no component suppressing the outline it is meant to show", () => { expect(offenders(COMPONENTS, OUTLINE_SUPPRESSION_UTILITY)).toEqual([]); - expect(offenders(STYLES, OUTLINE_SUPPRESSION)).toEqual([]); + expect(offenders(STYLES, OUTLINE_SUPPRESSION).filter(file => !EXEMPT_STYLES.has(file))) + .toEqual([]); }); it("keeps one focus dialect rather than three", () => { @@ -65,7 +73,7 @@ describe("focus", () => { }); it("keeps focus geometry in the theme", () => { - expect(offenders(STYLES, FOCUS_OUTLINE)).toEqual([]); + expect(offenders(STYLES, FOCUS_OUTLINE).filter(file => !EXEMPT_STYLES.has(file))).toEqual([]); expect(offenders([...STYLES, ...COMPONENTS], FOCUS_TOKEN_OVERRIDE)).toEqual([]); }); }); diff --git a/apps/web/src/tokens.test.ts b/apps/web/src/tokens.test.ts index 3b4178e2..36ef881e 100644 --- a/apps/web/src/tokens.test.ts +++ b/apps/web/src/tokens.test.ts @@ -616,13 +616,6 @@ describe("migration", () => { tag: "input", utility: "choice-control", }, - { - file: "packages/editor/src/widgets/render-blocks.tsx", - marker: 'aria-label="Code language"', - name: "code language", - tag: "select", - utility: "field-ghost", - }, ]; let offenders = controls.flatMap(control => controlOffenders( diff --git a/e2e/code.e2e.ts b/e2e/code.e2e.ts index 95a4d8dd..f69e58fa 100644 --- a/e2e/code.e2e.ts +++ b/e2e/code.e2e.ts @@ -16,10 +16,16 @@ import { content, expect, test, written } from "./room"; import { expectNoHorizontalOverflow } from "./responsive"; -import type { Page } from "@playwright/test"; +import type { Locator, Page } from "@playwright/test"; let MENU = { name: "Insert block" }; +/** The language control is a button that opens a listbox. */ +async function chooseLanguage(scope: Locator, from: string, to: string) { + await scope.getByRole("button", { name: `Code language: ${from}` }).click(); + await scope.page().getByRole("option", { name: to, exact: true }).click(); +} + /** A fence with the counts a person or a model actually writes. */ const PATCH = `\`\`\`diff --- a/apps/server/src/plan/room.ts @@ -183,7 +189,7 @@ test("naming a fence colours it, and the name reaches the file", async ({ join, // an uncoloured original is two of the same thing. await expect(content(page).locator("[data-file]")).toHaveCount(0); - await content(page).getByRole("combobox", { name: "Code language" }).selectOption("typescript"); + await chooseLanguage(content(page), "Plain text", "TypeScript"); await expect(content(page).locator("[data-file]")).toBeVisible(); await expect.poll(() => colours(page)).toBeGreaterThan(1); @@ -321,13 +327,13 @@ test("a language chosen by one is a change for everyone", async ({ join, room, s await expect(content(bo).locator("[data-file]")).toHaveCount(0); - await content(ana).getByRole("combobox", { name: "Code language" }).selectOption("typescript"); + await chooseLanguage(content(ana), "Plain text", "TypeScript"); // The language is a property of the fence rather than a way of looking at // it, so it travels: the other reader's copy is coloured too, and their // control says what it now is. - await expect(content(bo).getByRole("combobox", { name: "Code language" })) - .toHaveValue("typescript"); + await expect(content(bo).getByRole("button", { name: "Code language: TypeScript" })) + .toBeVisible(); await expect(content(bo).locator("[data-file]")).toBeVisible(); await expect.poll(() => colours(bo)).toBeGreaterThan(1); diff --git a/packages/editor/src/styles.css b/packages/editor/src/styles.css index 7d80698d..d3178fd6 100644 --- a/packages/editor/src/styles.css +++ b/packages/editor/src/styles.css @@ -1474,20 +1474,8 @@ --diffs-tab-size: 2; --diffs-header-font-family: var(--font-sans); - /* - * A hairline rather than a fill. The renderer paints its own background from - * the syntax theme, and a second one behind it would show at the edges as - * a rim of the wrong grey — but with neither, a snippet on a white page - * has nothing to say where it starts. - */ max-inline-size: 100%; overflow-x: auto; - border-radius: var(--radius-md); -} - -:where(.plan-content .plan-code-view) { - outline: var(--edge-width) solid var(--color-edge); - outline-offset: calc(-1 * var(--edge-width)); } /* The renderer's shadow root owns syntax painting; this host supplies the @@ -1591,6 +1579,96 @@ margin-inline: auto; } +/* + * A code block is one box: header, rendered code, source. A border rather than + * an outline, because the renderer paints over an inset outline. + */ +.plan-content .planCode { + display: flex; + flex-direction: column; + overflow: hidden; + border: var(--edge-width) solid var(--color-edge); + border-radius: var(--radius-lg); + background: var(--color-page); +} + +.plan-content .planCode [data-plan-chrome="block"] { + order: -1; + padding: 0.25rem; + border-block-end: var(--edge-width) solid var(--color-edge); + background: var(--color-inset); +} + +/* The box clips its corners, so focus is drawn inside each control. */ +.plan-content .plan-code-chrome :focus-visible { + outline-offset: calc(-1 * var(--focus-ring-width)); +} + +.plan-content .plan-code-toggle[aria-expanded="true"] { + background: var(--color-brand-wash); + color: var(--color-brand); +} + +/* A global icon colour rule would otherwise keep the icon grey. */ +.plan-content .plan-code-toggle[aria-expanded="true"] [data-nucleo-icon] { + color: inherit; +} + +.plan-content .plan-code-language-label { + padding-inline: 0.5rem; + font-size: var(--text-sm); + color: var(--color-text-tertiary); +} + +.plan-content .planCode .plan-code-view { + border-radius: 0; +} + +/* + * One left edge for every line of text in the block: the language label, + * rendered code, and source all start 0.75rem in. The renderer pads each line + * by 1ch inside its shadow root, so its wrapper supplies the remainder in the + * same monospace face. Diffs keep their gutter, which is its own column. + */ +.plan-content .planCode .plan-code-view[data-view="file"] { + /* The renderer adds 8px above its code and 8px below; make both 0.75rem. */ + padding-block: calc(0.75rem - 8px); + padding-inline-start: calc(0.75rem - 1ch); + font-family: ui-monospace, SFMono-Regular, monospace; + font-size: var(--text-sm); +} + +.plan-content .planCode [data-plan-source] { + margin: 0; + padding-inline: 0.75rem; + border-radius: 0; + background: var(--color-page); +} + +.plan-content .planCode [data-plan-preview]:not(:empty) ~ [data-plan-source] { + border-block-start: var(--edge-width) solid var(--color-edge); + background: var(--color-inset); +} + +.plan-content .planCode [data-plan-preview] :is(.plan-diagram, [data-plan-error]) { + padding-inline: 0.75rem; +} + +.plan-content .plan-code-title { + min-width: 0; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + font-size: var(--text-sm); + color: var(--color-text-secondary); +} + +.plan-content .plan-code-title::before { + content: "·"; + margin-inline: 0.125rem 0.375rem; + color: var(--color-text-quaternary); +} + /* Tabs ------------------------------------------------------------------- */ .plan-content [data-plan-chrome="tabs"] { @@ -1874,3 +1952,8 @@ white-space: nowrap; text-overflow: ellipsis; } + +/* The highlighted option is the keyboard's position; the panel needs no ring. */ +.plan-language-menu[role="listbox"]:focus-visible { + outline: none; +} diff --git a/packages/editor/src/widgets/code-view.tsx b/packages/editor/src/widgets/code-view.tsx index 32e013a2..55aa0bd8 100644 --- a/packages/editor/src/widgets/code-view.tsx +++ b/packages/editor/src/widgets/code-view.tsx @@ -22,7 +22,7 @@ import { Component, useEffect, useMemo, useState } from "react"; -import { fileNameOf, repaired, titled } from "./code"; +import { fileNameOf, repaired } from "./code"; import type { ErrorInfo, ReactNode } from "react"; import type { Kind } from "./code"; @@ -191,14 +191,13 @@ function View({ kind, source, language, meta }: CodeViewProps) { () => ({ theme: THEME, themeType: "light" as const, - // A snippet's identity is its language, and the control beside it - // already says that. A snippet quoting a file has a second one. - disableFileHeader: !titled(meta), + // The block's own header carries the language and any file name. + disableFileHeader: true, disableLineNumbers: true, overflow: "scroll" as const, disableWorkerPool: true, }), - [meta], + [], ); let diffOptions = useMemo( @@ -235,6 +234,7 @@ function View({ kind, source, language, meta }: CodeViewProps) { contentEditable={false} role="group" aria-label={kind === "diff" ? "Diff preview" : "Code preview"} + data-view={patch ? "diff" : "file"} tabIndex={0} > {patch diff --git a/packages/editor/src/widgets/code.test.ts b/packages/editor/src/widgets/code.test.ts index 5c9f2354..0b2a0487 100644 --- a/packages/editor/src/widgets/code.test.ts +++ b/packages/editor/src/widgets/code.test.ts @@ -9,7 +9,7 @@ import { describe, expect, it } from "bun:test"; import { DIFF_LANGUAGE, MERMAID_LANGUAGE } from "@chopin/dialect"; -import { fileNameOf, kindOf, LANGUAGES, repaired, titled, titleOf } from "./code"; +import { fileNameOf, kindOf, languageOptions, LANGUAGES, repaired, titled, titleOf } from "./code"; describe("what a fence is", () => { it("tells the two rendered languages apart from ordinary code", () => { @@ -202,3 +202,17 @@ describe("repairing a patch on the way to the renderer", () => { expect(repaired(patch)).toBe("--- x.ts\n+++ x.ts\n@@ -1,1 +1,1 @@\n-a\n+b\n"); }); }); + +describe("languageOptions", () => { + it("starts with plain text and lists every language once", () => { + let options = languageOptions("typescript"); + expect(options[0]).toEqual(["", "Plain text"]); + expect(options).toHaveLength(LANGUAGES.length + 1); + }); + + it("keeps an unlisted language selectable, right after plain text", () => { + let options = languageOptions("brainfuck"); + expect(options[1]).toEqual(["brainfuck", "brainfuck"]); + expect(options).toHaveLength(LANGUAGES.length + 2); + }); +}); diff --git a/packages/editor/src/widgets/code.ts b/packages/editor/src/widgets/code.ts index 61825320..e9c41815 100644 --- a/packages/editor/src/widgets/code.ts +++ b/packages/editor/src/widgets/code.ts @@ -241,3 +241,13 @@ function named(line: string): string { if (line.startsWith("+++ b/")) return `+++ ${line.slice("+++ b/".length)}`; return line; } + +/** The language menu's rows: plain text, then a fence's own unlisted language, then the list. */ +export function languageOptions(language: string): (readonly [string, string])[] { + let listed = LANGUAGES.some(([id]) => id === language); + return [ + ["", "Plain text"], + ...(!listed && language ? [[language, language] as const] : []), + ...LANGUAGES, + ]; +} diff --git a/packages/editor/src/widgets/language-menu.tsx b/packages/editor/src/widgets/language-menu.tsx new file mode 100644 index 00000000..3269c8f1 --- /dev/null +++ b/packages/editor/src/widgets/language-menu.tsx @@ -0,0 +1,195 @@ +/** + * A fence's language, chosen from the app's own menu. + * + * The trigger is a ghost button and the list is a portalled listbox styled + * like every other picker, rather than the operating system's select. The + * panel lives on `body`, outside the contenteditable, so opening it never + * moves the caret and the editor's clipping never crops it. + */ + +import { useEffect, useId, useLayoutEffect, useRef, useState } from "react"; +import { createPortal } from "react-dom"; +import { CheckIcon, ChevronIcon } from "@chopin/icons"; + +import { useTransitionPresence } from "../transition-presence"; + +import type { CSSProperties, KeyboardEvent } from "react"; + +export type LanguageOption = readonly [id: string, label: string]; + +const GAP = 4; +const MARGIN = 8; +const MAX_HEIGHT = 288; + +export function LanguageMenu( + { disabled, onChange, options, value }: { + disabled?: boolean; + onChange: (value: string) => void; + options: readonly LanguageOption[]; + value: string; + }, +) { + let [open, setOpen] = useState(false); + let [active, setActive] = useState(0); + let [position, setPosition] = useState({ visibility: "hidden" }); + let trigger = useRef(null); + let panel = useRef(null); + let listId = useId(); + let presence = useTransitionPresence(open ? true : undefined, 150, false); + let selected = Math.max(0, options.findIndex(([id]) => id === value)); + let label = options[selected]?.[1] ?? value; + + useLayoutEffect(() => { + if (!open) return; + let place = () => { + let rect = trigger.current?.getBoundingClientRect(); + if (!rect) return; + let below = window.innerHeight - rect.bottom - GAP - MARGIN; + let above = rect.top - GAP - MARGIN; + let height = Math.min(MAX_HEIGHT, Math.max(below, above)); + let flip = below < Math.min(MAX_HEIGHT, panel.current?.scrollHeight ?? MAX_HEIGHT) + && above > below; + setPosition({ + left: Math.max(MARGIN, rect.left), + maxHeight: height, + top: flip ? undefined : rect.bottom + GAP, + bottom: flip ? window.innerHeight - rect.top + GAP : undefined, + transformOrigin: flip ? "bottom left" : "top left", + visibility: "visible", + }); + }; + place(); + window.addEventListener("resize", place); + window.addEventListener("scroll", place, true); + return () => { + window.removeEventListener("resize", place); + window.removeEventListener("scroll", place, true); + }; + }, [open]); + + // Keep the highlighted option in view as the keyboard moves through a long list. + useEffect(() => { + if (!open) return; + panel.current?.querySelector(`[data-index="${active}"]`) + ?.scrollIntoView({ block: "nearest" }); + }, [open, active]); + + useEffect(() => { + if (!open) return; + let dismiss = (event: PointerEvent) => { + let target = event.target as Node; + if (panel.current?.contains(target) || trigger.current?.contains(target)) return; + setOpen(false); + }; + document.addEventListener("pointerdown", dismiss, true); + return () => document.removeEventListener("pointerdown", dismiss, true); + }, [open]); + + let show = () => { + setActive(selected); + setOpen(true); + requestAnimationFrame(() => panel.current?.focus()); + }; + + let close = () => { + setOpen(false); + trigger.current?.focus(); + }; + + let choose = (index: number) => { + let option = options[index]; + if (option && option[0] !== value) onChange(option[0]); + close(); + }; + + let onKey = (event: KeyboardEvent) => { + let last = options.length - 1; + let step: Record void> = { + ArrowDown: () => setActive(index => Math.min(last, index + 1)), + ArrowUp: () => setActive(index => Math.max(0, index - 1)), + Home: () => setActive(0), + End: () => setActive(last), + Enter: () => choose(active), + " ": () => choose(active), + Escape: () => close(), + Tab: () => setOpen(false), + }; + let action = step[event.key]; + if (action) { + if (event.key !== "Tab") event.preventDefault(); + event.stopPropagation(); + action(); + return; + } + // Type to jump, as a native select does. + if (event.key.length === 1) { + let letter = event.key.toLowerCase(); + let found = options.findIndex(([, name], index) => + index > active && name.toLowerCase().startsWith(letter) + ); + if (found < 0) found = options.findIndex(([, name]) => name.toLowerCase().startsWith(letter)); + if (found >= 0) setActive(found); + } + }; + + return ( + <> + + {presence.phase !== "closed" && createPortal( +
+ {options.map(([id, name], index) => ( +
choose(index)} + onPointerMove={() => setActive(index)} + role="option" + > + {name} + {index === selected && ( +
+ ))} +
, + document.body, + )} + + ); +} diff --git a/packages/editor/src/widgets/render-blocks.tsx b/packages/editor/src/widgets/render-blocks.tsx index 46011f1a..62e9abc4 100644 --- a/packages/editor/src/widgets/render-blocks.tsx +++ b/packages/editor/src/widgets/render-blocks.tsx @@ -36,8 +36,10 @@ import { import { $isCodeBlockNode, $isMathNode } from "@chopin/dialect"; import { enclosing, remember } from "../collapse"; -import { kindOf, LANGUAGES } from "./code"; +import { kindOf, languageOptions, titleOf } from "./code"; import { CodeView } from "./code-view"; +import { LanguageMenu } from "./language-menu"; +import { CodeIcon } from "@chopin/icons"; import type { ElementNode, LexicalEditor } from "lexical"; import type { Kind } from "./code"; @@ -184,21 +186,15 @@ function Language( }); }, [editor, block.key]); - let listed = LANGUAGES.some(([id]) => id === block.language); + let options = languageOptions(block.language); - return ( - - ); + // A reader cannot change it, so it is a label rather than a disabled control. + if (disabled) { + let label = options.find(([id]) => id === block.language)?.[1] ?? block.language; + return {label}; + } + + return ; } function Toggle({ collapsed, onToggle }: { collapsed: boolean; onToggle: () => void }) { @@ -213,9 +209,11 @@ function Toggle({ collapsed, onToggle }: { collapsed: boolean; onToggle: () => v // change arrives asynchronously, after the collapse, and reads // as the reader arrowing in — reopening what they just closed. onMouseDown={event => event.preventDefault()} - className="cursor-pointer rounded-sm px-1.5 py-0.5 text-sm text-text-tertiary transition hover:bg-hover hover:text-text-primary" + aria-label={collapsed ? "Show source" : "Hide source"} + className="plan-code-toggle btn btn-icon btn-ghost" + data-tooltip={collapsed ? "Show source" : "Hide source"} > - {collapsed ? "Show source" : "Hide source"} +