diff --git a/apps/web/src/hosted.tsx b/apps/web/src/hosted.tsx index 8133eca5..74cde8c6 100644 --- a/apps/web/src/hosted.tsx +++ b/apps/web/src/hosted.tsx @@ -465,6 +465,7 @@ export function HostedApp( { agent, user }: { agent: boolean; user: Api.User }, ) { let [route, setRoute] = useState(() => hostedRoute(location.pathname)); + let [navigationRevision, setNavigationRevision] = useState(0); let hostedRouteRef = useRef(route); hostedRouteRef.current = route; let childOpener = useRef(undefined); @@ -497,6 +498,7 @@ export function HostedApp( let next = `${target.pathname}${target.search}${target.hash}`; let current = `${location.pathname}${location.search}${location.hash}`; if (next === current) return; + setNavigationRevision(value => value + 1); childRouteChanged(target.pathname); let nextRoute = hostedRoute(target.pathname); if (options.replace) history.replaceState(history.state, "", next); @@ -596,6 +598,7 @@ export function HostedApp( useEffect(() => { let changed = () => { + setNavigationRevision(value => value + 1); childRouteChanged(location.pathname); setRoute(hostedRoute(location.pathname)); }; @@ -632,7 +635,12 @@ export function HostedApp( break; } return ( - + {workspace} ); diff --git a/apps/web/src/navigation-shell.tsx b/apps/web/src/navigation-shell.tsx index efe94731..510a47ee 100644 --- a/apps/web/src/navigation-shell.tsx +++ b/apps/web/src/navigation-shell.tsx @@ -245,11 +245,13 @@ export function NavigationShell( { children, navigate, + navigationRevision, route, user, }: { children?: ReactNode; navigate: Navigate; + navigationRevision: number; route: NavigationRoute; user: Api.User; }, @@ -521,6 +523,7 @@ export function NavigationShell( let creation = useDocumentCreation({ routeKey, + navigationRevision, onCreated: upsertDocument, onNavigate: navigateToDocument, onAccessChanged: () => void refresh(), diff --git a/apps/web/src/theme.css b/apps/web/src/theme.css index 9abc5e22..0a450838 100644 --- a/apps/web/src/theme.css +++ b/apps/web/src/theme.css @@ -656,6 +656,11 @@ body { outline: none; } +/* A listbox holds the keyboard's place on its highlighted option instead. */ +:where(.plan-language-menu[role="listbox"]):focus-visible { + outline: none; +} + /* The editable plan has a live caret, so a second focus marker is redundant. */ :where(.focus-caret):focus-visible { outline: none; diff --git a/apps/web/src/tokens.test.ts b/apps/web/src/tokens.test.ts index 3b4178e2..79d52c5b 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( @@ -703,6 +696,18 @@ describe("migration", () => { tiers: ["btn-ghost"], }, ], + ["packages/editor/src/widgets/render-blocks.tsx", { + action: "code source toggle", + marker: 'aria-label={collapsed ? "Show source" : "Hide source"}', + size: "btn-icon", + tiers: ["btn-ghost"], + }], + ["packages/editor/src/widgets/language-menu.tsx", { + action: "code language trigger", + marker: 'aria-haspopup="listbox"', + size: "btn-sm", + tiers: ["btn-ghost"], + }], ["apps/web/src/chat/transcript.tsx", { action: "Withdraw", marker: 'title="Withdraw"', diff --git a/apps/web/src/use-document-creation.ts b/apps/web/src/use-document-creation.ts index 2769f12b..315fedc0 100644 --- a/apps/web/src/use-document-creation.ts +++ b/apps/web/src/use-document-creation.ts @@ -13,8 +13,9 @@ type Attempt = { }; export function useDocumentCreation( - { routeKey, onCreated, onNavigate, onAccessChanged }: { + { routeKey, navigationRevision, onCreated, onNavigate, onAccessChanged }: { routeKey: string; + navigationRevision: number; onCreated: (channel: Api.Channel) => void; onNavigate: (documentId: string, path: string) => void; onAccessChanged: () => void; @@ -24,8 +25,11 @@ export function useDocumentCreation( let [pending, setPending] = useState>(() => new Map()); let [error, setError] = useState<{ project: Api.NavigationProject; message: string }>(); let latest = useRef(undefined); - let location = useRef({ routeKey }); - if (location.current.routeKey !== routeKey) location.current = { routeKey }; + // Canonicalizing the same document changes its route key without a navigation. + let location = useRef({ routeKey, navigationRevision }); + if (location.current.navigationRevision !== navigationRevision) { + location.current = { routeKey, navigationRevision }; + } else location.current.routeKey = routeKey; let publish = useCallback(() => { setPending(new Map([...attempts.current].map(([id, attempt]) => [id, attempt.phase]))); }, []); @@ -37,7 +41,7 @@ export function useDocumentCreation( useEffect(() => () => { latest.current = undefined; - location.current = { routeKey: "" }; + location.current = { routeKey: "", navigationRevision: -1 }; }, []); useEffect(() => { diff --git a/e2e/code.e2e.ts b/e2e/code.e2e.ts index 95a4d8dd..285054d7 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); @@ -226,6 +232,15 @@ test("a fence that is not a patch is drawn as the text it is", async ({ join, se await expect(content(page).locator("[data-diff]")).toHaveCount(0); }); +test("an invalid diff keeps its authored filename", async ({ join, seed }) => { + await seed('```diff title="broken.patch"\nnot a patch\n```\n'); + let page = await join("ana"); + + await expect(content(page).locator("[data-file]")).toBeVisible(); + await expect(content(page).getByText("broken.patch", { exact: true })).toBeVisible(); + await expect(content(page).locator("[data-diff]")).toHaveCount(0); +}); + test("enter is a newline in a fence, and twice over is the way out", async ({ join, room }) => { let page = await join("ana"); @@ -321,13 +336,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); @@ -349,3 +364,58 @@ test("showing the source leaves everybody else's hidden", async ({ join, seed }) await expect(content(bo).locator("[data-plan-source]")).toBeHidden(); await expect(content(bo).getByRole("button", { name: "Show source" })).toBeVisible(); }); + +test("the language menu is a keyboard-operable listbox", async ({ join, room, seed }) => { + await seed("```typescript\nlet total = 1;\n```\n"); + let page = await join("ana"); + let trigger = content(page).getByRole("button", { name: "Code language: TypeScript" }); + let list = page.getByRole("listbox", { name: "Code language" }); + + await trigger.focus(); + await page.keyboard.press("ArrowDown"); + await expect(list).toBeVisible(); + + await page.keyboard.press("Escape"); + await expect(list).toBeHidden(); + await expect(trigger).toBeFocused(); + + await trigger.click(); + await expect(list).toBeVisible(); + await page.mouse.click(5, 5); + await expect(list).toBeHidden(); + + // Tab from the open menu continues from the trigger, not from the end of the page. + await trigger.focus(); + await page.keyboard.press("ArrowDown"); + await expect(list).toBeVisible(); + await page.keyboard.press("Tab"); + await expect(list).toBeHidden(); + await expect(content(page).getByRole("button", { name: "Show source" })).toBeFocused(); + + await trigger.focus(); + await page.keyboard.press("ArrowDown"); + await page.keyboard.press("ArrowDown"); + await page.keyboard.press("Enter"); + await expect(list).toBeHidden(); + await expect(content(page).getByRole("button", { name: /^Code language: (?!TypeScript)/ })) + .toBeVisible(); + await written(page, room, /^```(?!typescript$)\S+$/m); +}); + +test("the language menu takes focus before the next animation frame", async ({ join, seed }) => { + await seed("```typescript\nlet total = 1;\n```\n"); + let page = await join("ana"); + let trigger = content(page).getByRole("button", { name: "Code language: TypeScript" }); + let list = page.getByRole("listbox", { name: "Code language" }); + + await page.clock.install(); + await page.clock.pauseAt(new Date()); + await trigger.focus(); + await page.keyboard.press("ArrowDown"); + await expect(list).toBeFocused(); + await page.keyboard.press("ArrowDown"); + await page.keyboard.press("Enter"); + await page.clock.resume(); + await expect(content(page).getByRole("button", { name: "Code language: XML", exact: true })) + .toBeVisible(); +}); diff --git a/e2e/design/approved-contrast/chat.json b/e2e/design/approved-contrast/chat.json index b498d28f..4019e29a 100644 --- a/e2e/design/approved-contrast/chat.json +++ b/e2e/design/approved-contrast/chat.json @@ -16,7 +16,7 @@ "finding": { "rule": "color-contrast", "target": [ - ".gap-3.flex[data-chat-entry=\"true\"]:nth-child(1) > .-mt-0\\.5.gap-1.flex-1 > .items-baseline.gap-1\\.5.text-sm > .tabular-nums.text-text-quaternary.text-sm" + ".gap-3.flex[data-chat-entry=\"true\"]:nth-child(1) > .-mt-0\\.5.gap-1.flex-1 > .items-baseline.gap-1\\.5.text-sm > .text-text-quaternary.tabular-nums.text-sm" ], "html": "10:40", "evidence": "Fix any of the following:\n Element has insufficient color contrast of 4.43 (foreground color: #78766e, background color: #fcfcfb, font size: 10.5pt (14.0351px), font weight: normal). Expected contrast ratio of 4.5:1" @@ -27,7 +27,7 @@ "finding": { "rule": "color-contrast", "target": [ - ".gap-3.flex[data-chat-entry=\"true\"]:nth-child(2) > .-mt-0\\.5.gap-1.flex-1 > .items-baseline.gap-1\\.5.text-sm > .tabular-nums.text-text-quaternary.text-sm" + ".gap-3.flex[data-chat-entry=\"true\"]:nth-child(2) > .-mt-0\\.5.gap-1.flex-1 > .items-baseline.gap-1\\.5.text-sm > .text-text-quaternary.tabular-nums.text-sm" ], "html": "10:41", "evidence": "Fix any of the following:\n Element has insufficient color contrast of 4.43 (foreground color: #78766e, background color: #fcfcfb, font size: 10.5pt (14.0351px), font weight: normal). Expected contrast ratio of 4.5:1" @@ -71,7 +71,7 @@ "finding": { "rule": "color-contrast", "target": [ - ".opacity-60 > .items-baseline.gap-1\\.5.text-sm > .tabular-nums.text-text-quaternary.text-sm" + ".opacity-60 > .items-baseline.gap-1\\.5.text-sm > .text-text-quaternary.tabular-nums.text-sm" ], "html": "queued", "evidence": "Fix any of the following:\n Element has insufficient color contrast of 2.21 (foreground color: #adaca6, background color: #fcfcfb, font size: 10.5pt (14.0351px), font weight: normal). Expected contrast ratio of 4.5:1" @@ -106,7 +106,7 @@ "finding": { "rule": "color-contrast", "target": [ - ".gap-3.flex[data-chat-entry=\"true\"]:nth-child(1) > .-mt-0\\.5.gap-1.flex-1 > .items-baseline.gap-1\\.5.text-sm > .tabular-nums.text-text-quaternary.text-sm" + ".gap-3.flex[data-chat-entry=\"true\"]:nth-child(1) > .-mt-0\\.5.gap-1.flex-1 > .items-baseline.gap-1\\.5.text-sm > .text-text-quaternary.tabular-nums.text-sm" ], "html": "10:40", "evidence": "Fix any of the following:\n Element has insufficient color contrast of 4.43 (foreground color: #78766e, background color: #fcfcfb, font size: 10.1pt (13.4107px), font weight: normal). Expected contrast ratio of 4.5:1" @@ -117,7 +117,7 @@ "finding": { "rule": "color-contrast", "target": [ - ".gap-3.flex[data-chat-entry=\"true\"]:nth-child(2) > .-mt-0\\.5.gap-1.flex-1 > .items-baseline.gap-1\\.5.text-sm > .tabular-nums.text-text-quaternary.text-sm" + ".gap-3.flex[data-chat-entry=\"true\"]:nth-child(2) > .-mt-0\\.5.gap-1.flex-1 > .items-baseline.gap-1\\.5.text-sm > .text-text-quaternary.tabular-nums.text-sm" ], "html": "10:41", "evidence": "Fix any of the following:\n Element has insufficient color contrast of 4.43 (foreground color: #78766e, background color: #fcfcfb, font size: 10.1pt (13.4107px), font weight: normal). Expected contrast ratio of 4.5:1" @@ -161,7 +161,7 @@ "finding": { "rule": "color-contrast", "target": [ - ".opacity-60 > .items-baseline.gap-1\\.5.text-sm > .tabular-nums.text-text-quaternary.text-sm" + ".opacity-60 > .items-baseline.gap-1\\.5.text-sm > .text-text-quaternary.tabular-nums.text-sm" ], "html": "queued", "evidence": "Fix any of the following:\n Element has insufficient color contrast of 2.21 (foreground color: #adaca6, background color: #fcfcfb, font size: 10.1pt (13.4107px), font weight: normal). Expected contrast ratio of 4.5:1" diff --git a/e2e/design/approved-contrast/code.json b/e2e/design/approved-contrast/code.json index 4e370820..de976993 100644 --- a/e2e/design/approved-contrast/code.json +++ b/e2e/design/approved-contrast/code.json @@ -17,7 +17,7 @@ "rule": "color-contrast", "target": [ [ - "div[data-plan-language=\"typescript\"] > div[data-plan-preview=\"\"] > .plan-code-view[aria-label=\"Code preview\"][role=\"group\"] > diffs-container", + "div[data-plan-language=\"typescript\"] > div[data-plan-preview=\"\"] > .plan-code-view[aria-label=\"Code preview\"][data-view=\"file\"] > diffs-container", "span:nth-child(3)" ] ], @@ -31,7 +31,7 @@ "rule": "color-contrast", "target": [ [ - "div[data-plan-language=\"typescript\"] > div[data-plan-preview=\"\"] > .plan-code-view[aria-label=\"Code preview\"][role=\"group\"] > diffs-container", + "div[data-plan-language=\"typescript\"] > div[data-plan-preview=\"\"] > .plan-code-view[aria-label=\"Code preview\"][data-view=\"file\"] > diffs-container", "span:nth-child(6)" ] ], @@ -45,7 +45,7 @@ "rule": "color-contrast", "target": [ [ - "div[data-plan-language=\"typescript\"] > div[data-plan-preview=\"\"] > .plan-code-view[aria-label=\"Code preview\"][role=\"group\"] > diffs-container", + "div[data-plan-language=\"typescript\"] > div[data-plan-preview=\"\"] > .plan-code-view[aria-label=\"Code preview\"][data-view=\"file\"] > diffs-container", "span:nth-child(10)" ] ], @@ -59,7 +59,7 @@ "rule": "color-contrast", "target": [ [ - "div[data-plan-language=\"typescript\"] > div[data-plan-preview=\"\"] > .plan-code-view[aria-label=\"Code preview\"][role=\"group\"] > diffs-container", + "div[data-plan-language=\"typescript\"] > div[data-plan-preview=\"\"] > .plan-code-view[aria-label=\"Code preview\"][data-view=\"file\"] > diffs-container", "span:nth-child(12)" ] ], @@ -73,7 +73,7 @@ "rule": "color-contrast", "target": [ [ - "div[data-plan-language=\"css\"] > div[data-plan-preview=\"\"] > .plan-code-view[aria-label=\"Code preview\"][role=\"group\"] > diffs-container", + "div[data-plan-language=\"css\"] > div[data-plan-preview=\"\"] > .plan-code-view[aria-label=\"Code preview\"][data-view=\"file\"] > diffs-container", "span:nth-child(2)" ] ], @@ -87,7 +87,7 @@ "rule": "color-contrast", "target": [ [ - "div[data-plan-language=\"css\"] > div[data-plan-preview=\"\"] > .plan-code-view[aria-label=\"Code preview\"][role=\"group\"] > diffs-container", + "div[data-plan-language=\"css\"] > div[data-plan-preview=\"\"] > .plan-code-view[aria-label=\"Code preview\"][data-view=\"file\"] > diffs-container", "span:nth-child(4)" ] ], @@ -101,7 +101,7 @@ "rule": "color-contrast", "target": [ [ - "div[data-plan-language=\"css\"] > div[data-plan-preview=\"\"] > .plan-code-view[aria-label=\"Code preview\"][role=\"group\"] > diffs-container", + "div[data-plan-language=\"css\"] > div[data-plan-preview=\"\"] > .plan-code-view[aria-label=\"Code preview\"][data-view=\"file\"] > diffs-container", "span:nth-child(8)" ] ], @@ -128,7 +128,7 @@ "rule": "color-contrast", "target": [ [ - "div[data-plan-language=\"typescript\"] > div[data-plan-preview=\"\"] > .plan-code-view[aria-label=\"Code preview\"][role=\"group\"] > diffs-container", + "div[data-plan-language=\"typescript\"] > div[data-plan-preview=\"\"] > .plan-code-view[aria-label=\"Code preview\"][data-view=\"file\"] > diffs-container", "span:nth-child(3)" ] ], @@ -142,7 +142,7 @@ "rule": "color-contrast", "target": [ [ - "div[data-plan-language=\"typescript\"] > div[data-plan-preview=\"\"] > .plan-code-view[aria-label=\"Code preview\"][role=\"group\"] > diffs-container", + "div[data-plan-language=\"typescript\"] > div[data-plan-preview=\"\"] > .plan-code-view[aria-label=\"Code preview\"][data-view=\"file\"] > diffs-container", "span:nth-child(6)" ] ], @@ -156,7 +156,7 @@ "rule": "color-contrast", "target": [ [ - "div[data-plan-language=\"typescript\"] > div[data-plan-preview=\"\"] > .plan-code-view[aria-label=\"Code preview\"][role=\"group\"] > diffs-container", + "div[data-plan-language=\"typescript\"] > div[data-plan-preview=\"\"] > .plan-code-view[aria-label=\"Code preview\"][data-view=\"file\"] > diffs-container", "span:nth-child(10)" ] ], @@ -170,21 +170,7 @@ "rule": "color-contrast", "target": [ [ - "div[data-plan-language=\"typescript\"] > div[data-plan-preview=\"\"] > .plan-code-view[aria-label=\"Code preview\"][role=\"group\"] > diffs-container", - "span:nth-child(12)" - ] - ], - "html": " 12", - "evidence": "Fix any of the following:\n Element has insufficient color contrast of 3.01 (foreground color: #1ca1c7, background color: #ffffff, font size: 10.1pt (13.4107px), font weight: normal). Expected contrast ratio of 4.5:1" - } - }, - { - "approvedRole": "original pierre-light syntax token", - "finding": { - "rule": "color-contrast", - "target": [ - [ - "div[data-plan-language=\"css\"] > div[data-plan-preview=\"\"] > .plan-code-view[aria-label=\"Code preview\"][role=\"group\"] > diffs-container", + "div[data-plan-language=\"css\"] > div[data-plan-preview=\"\"] > .plan-code-view[aria-label=\"Code preview\"][data-view=\"file\"] > diffs-container", "span:nth-child(2)" ] ], @@ -198,7 +184,7 @@ "rule": "color-contrast", "target": [ [ - "div[data-plan-language=\"css\"] > div[data-plan-preview=\"\"] > .plan-code-view[aria-label=\"Code preview\"][role=\"group\"] > diffs-container", + "div[data-plan-language=\"css\"] > div[data-plan-preview=\"\"] > .plan-code-view[aria-label=\"Code preview\"][data-view=\"file\"] > diffs-container", "span:nth-child(4)" ] ], @@ -212,7 +198,7 @@ "rule": "color-contrast", "target": [ [ - "div[data-plan-language=\"css\"] > div[data-plan-preview=\"\"] > .plan-code-view[aria-label=\"Code preview\"][role=\"group\"] > diffs-container", + "div[data-plan-language=\"css\"] > div[data-plan-preview=\"\"] > .plan-code-view[aria-label=\"Code preview\"][data-view=\"file\"] > diffs-container", "span:nth-child(8)" ] ], diff --git a/e2e/design/snapshots/narrow/code.png b/e2e/design/snapshots/narrow/code.png index 4849a813..a03d4953 100644 Binary files a/e2e/design/snapshots/narrow/code.png and b/e2e/design/snapshots/narrow/code.png differ diff --git a/e2e/design/snapshots/narrow/table.png b/e2e/design/snapshots/narrow/table.png index bd4d832c..ed6cc27e 100644 Binary files a/e2e/design/snapshots/narrow/table.png and b/e2e/design/snapshots/narrow/table.png differ diff --git a/e2e/design/snapshots/wide/code.png b/e2e/design/snapshots/wide/code.png index 84f94998..01f8b912 100644 Binary files a/e2e/design/snapshots/wide/code.png and b/e2e/design/snapshots/wide/code.png differ diff --git a/e2e/design/snapshots/wide/table.png b/e2e/design/snapshots/wide/table.png index 188e6078..83ea2f00 100644 Binary files a/e2e/design/snapshots/wide/table.png and b/e2e/design/snapshots/wide/table.png differ diff --git a/e2e/document-creation.e2e.ts b/e2e/document-creation.e2e.ts index 5ffc43fe..1e0e63f4 100644 --- a/e2e/document-creation.e2e.ts +++ b/e2e/document-creation.e2e.ts @@ -1,5 +1,5 @@ import { authenticate, expect, test } from "./room"; -import { createChannel } from "./database"; +import { createChannel, testChannelPath } from "./database"; import type { Page } from "@playwright/test"; import type { ChannelDetail } from "../apps/web/src/api"; @@ -406,3 +406,38 @@ for (let returnToOrigin of [false, true]) { .toHaveAttribute("aria-current", "page"); }); } + +test("canonicalizing the current document does not cancel a pending creation", async ({ baseURL, page }) => { + let id = crypto.randomUUID(); + await createChannel(Number(new URL(baseURL!).port), id); + await authenticate(page, `creator-${crypto.randomUUID()}`, baseURL!); + await page.request.post("/api/navigation/projects", { + data: { owner: "octo-org", repository: "score" }, + headers: { origin: baseURL! }, + }); + let reading = Promise.withResolvers(); + let releaseRead = Promise.withResolvers(); + let posted = Promise.withResolvers(); + let releasePost = Promise.withResolvers(); + await page.route(`**/api/channels/${id}`, async route => { + reading.resolve(); + await releaseRead.promise; + await route.continue(); + }); + await page.route("**/api/repositories/octo-org/score/channels", async route => { + if (route.request().method() !== "POST") return route.fallback(); + let response = await route.fetch(); + posted.resolve(await response.json()); + await releasePost.promise; + await route.fulfill({ response }); + }); + + await page.goto(`/channels/${id}`); + await reading.promise; + await page.getByRole("button", { name: "New document in score", exact: true }).click(); + let created = await posted.promise; + releaseRead.resolve(); + await expect(page).toHaveURL(testChannelPath(id)); + releasePost.resolve(); + await expect(page).toHaveURL(`/documents/octo-org/score/${created.channel.slug}`); +}); diff --git a/e2e/hosted.e2e.ts b/e2e/hosted.e2e.ts index a891b250..442fa92d 100644 --- a/e2e/hosted.e2e.ts +++ b/e2e/hosted.e2e.ts @@ -63,7 +63,7 @@ test("organization admission rejects outsiders and pending members", async ({ ba }); test("an authenticated user adds a Project and creates its first document", async ({ baseURL, page }) => { - await authenticate(page, "project-creator", baseURL!); + await authenticate(page, `project-creator-${crypto.randomUUID()}`, baseURL!); await page.goto("/"); let dialog = addProjectDialog(page); diff --git a/packages/editor/src/styles.css b/packages/editor/src/styles.css index 7d80698d..a833320e 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,92 @@ 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; + margin-block: 1em; + 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); +} + +.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"] { diff --git a/packages/editor/src/widgets/code-view.tsx b/packages/editor/src/widgets/code-view.tsx index 32e013a2..1de485b3 100644 --- a/packages/editor/src/widgets/code-view.tsx +++ b/packages/editor/src/widgets/code-view.tsx @@ -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), + // A diff's fallback has no title in the block's own header. + disableFileHeader: kind !== "diff" || !titled(meta), disableLineNumbers: true, overflow: "scroll" as const, disableWorkerPool: true, }), - [meta], + [kind, 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..3759f7dd --- /dev/null +++ b/packages/editor/src/widgets/language-menu.tsx @@ -0,0 +1,214 @@ +/** + * 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); + // By id, not index: a collaborator can add or remove the unlisted-language row. + let [activeId, setActiveId] = useState(value); + 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 found = options.findIndex(([id]) => id === activeId); + let active = found < 0 ? selected : found; + let setActive = (next: number | ((index: number) => number)) => { + let index = typeof next === "function" ? next(active) : next; + let option = options[index]; + if (option) setActiveId(option[0]); + }; + let label = options[selected]?.[1] ?? value; + + useLayoutEffect(() => { + if (!open) return; + let place = (event?: Event) => { + // The menu's own scrolling must not reposition it. + if (event?.target instanceof Node && panel.current?.contains(event.target)) return; + 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]); + + useLayoutEffect(() => { + if (open && position.visibility === "visible") panel.current?.focus({ preventScroll: true }); + }, [open, position.visibility]); + + // 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 = () => { + setActiveId(value); + setOpen(true); + }; + + 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(), + }; + // The panel is portalled to the end of body, so hand focus back to the + // trigger and let the browser's default Tab continue from there. + if (event.key === "Tab") { + setOpen(false); + trigger.current?.focus(); + return; + } + let action = step[event.key]; + if (action) { + 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..a0856ab0 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"} +