Phone comments list, Build empty state and a Chopin chip

apps/web/src/chat/chat.tsx

@@ -10,14 +10,16 @@
 import { useEffect, useId, useLayoutEffect, useReducer, useRef, useState } from "react";
 
 import {
+	ChopinMark,
 	CONNECTION_GRACE,
 	MAX_RESEARCH_BRIEF,
 	SendAction,
 	useConnectionNotice,
 	usePopoverDismissal,
 } from "@chopin/editor";
+import { usePhone } from "@chopin/editor/pointer";
 import { MENTION } from "@chopin/protocol/address";
-import { ArchiveIcon, InfoIcon, LockIcon, PlusIcon, WarningIcon } from "@chopin/icons";
+import { ArchiveIcon, CloseIcon, InfoIcon, LockIcon, PlusIcon, WarningIcon } from "@chopin/icons";
 import { DraftInput } from "./draft-input";
 import type { DraftInputHandle } from "./draft-input";
 import { ModeSwitch } from "./mode-switch";
@@ -240,6 +242,7 @@ export function Chat(
 	let textarea = useRef<DraftInputHandle>(null);
 	let composerRoot = useRef<HTMLDivElement>(null);
 	let [mode, setMode] = useState(false);
+	let phone = usePhone();
 	let [historyKey, setHistoryKey] = useState(0);
 	let pendingCaret = useRef<number | { start: number; end: number } | undefined>(undefined);
 	let submission = useRef<object | undefined>(undefined);
@@ -562,10 +565,15 @@ export function Chat(
 		let end = selection.end;
 		if (effectiveMode) {
 			let removals = [...current.text.matchAll(/(^|[^\w@])@chopin\b/gi)]
-				.map(match => ({
-					start: match.index + match[1]!.length,
-					end: match.index + match[0].length,
-				}))
+				.map(match => {
+					let start = match.index + match[1]!.length;
+					let end = match.index + match[0].length;
+					// Take one following space too where the mention began the text or followed one,
+					// so removing it leaves no doubled or leading space.
+					let before = current.text[start - 1];
+					if (current.text[end] === " " && (start === 0 || before === " ")) end += 1;
+					return { start, end };
+				})
 				.filter(edit =>
 					!current.references.some(reference =>
 						edit.start < reference.end && edit.end > reference.start
@@ -798,6 +806,23 @@ export function Chat(
 						)
 						: (
 							<>
+								{/* A phone's switch shows only an icon; the chip says where the message goes. */}
+								{phone && effectiveMode && (
+									<span className="composer-chopin-row">
+										<button
+											aria-label="Remove Chopin"
+											className="composer-chopin-chip"
+											data-press="none"
+											disabled={!draftable || sending}
+											onClick={toggleMode}
+											type="button"
+										>
+											<ChopinMark />
+											<span aria-hidden="true">Chopin</span>
+											<CloseIcon aria-hidden="true" size={12} />
+										</button>
+									</span>
+								)}
 								<DraftInput
 									aria-label="Message"
 									aria-activedescendant={commandOpen

apps/web/src/chat/composer.css

@@ -410,6 +410,46 @@
 		outline-color: var(--color-inset);
 		opacity: 1;
 	}
+	/* Addressing Chopin is a removable chip on a row of its own above the draft. */
+	.chat-composer .composer-chopin-row {
+		/* Above the draft, so the chip's touch target may reach into the draft's top padding. */
+		position: relative;
+		z-index: 1;
+		display: flex;
+		order: 0;
+		flex-basis: 100%;
+		padding: 0.25rem;
+	}
+	/* Coarse pointers would otherwise grow every control in the pill to a full target. */
+	&[data-plan-coarse-pointer] .chat-composer .composer-surface button.composer-chopin-chip,
+	.chat-composer .composer-surface .composer-chopin-chip {
+		position: relative;
+		display: inline-flex;
+		align-items: center;
+		gap: 0.25rem;
+		min-width: 0;
+		min-height: 0;
+		height: var(--control-height-compact);
+		padding-inline: 0.5rem 0.375rem;
+		border-radius: var(--radius-full);
+		background: var(--color-brand-wash);
+		color: var(--color-brand-ink);
+		font-size: var(--text-xs);
+		font-weight: var(--font-weight-medium);
+	}
+	/* The chip is the way back to Chat while it shows, so the switch steps aside. */
+	.chat-composer .composer-surface:has(.composer-chopin-row) .composer-mode-trigger {
+		display: none;
+	}
+	/* A full touch target around a chip drawn smaller than one. */
+	.chat-composer .composer-chopin-chip::after {
+		position: absolute;
+		inset: calc((100% - var(--control-target-coarse)) / 2) -0.25rem;
+		content: "";
+	}
+	.chat-composer .composer-chopin-chip:active:not(:disabled) {
+		color: var(--color-brand-active);
+	}
 	/* Connection state becomes a slim first row inside the pill. */
 	.chat-composer .composer-connection-group:has(.composer-connection-label) {
 		display: flex;

apps/web/src/phone-header.tsx

@@ -1,7 +1,10 @@
-/** The phone top bar: Documents on the left, a small centred title, and actions on the right. */
+/**
+ * The phone top bar: Documents on the left, a small centred title, and the document's
+ * comments and actions on the right.
+ */
 
 import { useContext, useEffect, useLayoutEffect, useRef } from "react";
-import { DocumentsIcon, EllipsisIcon } from "@chopin/icons";
+import { CommentIcon, DocumentsIcon, EllipsisIcon } from "@chopin/icons";
 
 import { DocumentActionsMenu, loadActionSheet } from "./document-actions-menu";
 import { DocumentRename } from "./document-rename";
@@ -23,22 +26,27 @@ export function PhoneHeader(
 	{
 		archivedAt,
 		canManage,
+		comments = 0,
 		editing,
 		editingDocument,
 		label,
 		onAction,
+		onComments,
 		onEditingChange,
 		onRenamed,
 		presentation,
 		room,
 	}: {
 		archivedAt?: string;
 		canManage: boolean;
+		/** Open comment threads in the document; the bar offers a list of them when there are any. */
+		comments?: number;
 		editing?: TitleEdit;
 		/** The document itself is in edit mode, which the bar says under the title. */
 		editingDocument?: boolean;
 		label: string;
 		onAction: (action: DocumentAction) => void;
+		onComments?: () => void;
 		onEditingChange: (editing?: TitleEdit) => void;
 		onRenamed: (channel: DocumentMetadata) => void;
 		presentation: WorkspacePresentation;
@@ -123,6 +131,19 @@ export function PhoneHeader(
 				)}
 			</div>
 			<div className="phone-top-bar-slot">
+				{comments > 0 && onComments && (
+					<button
+						aria-haspopup="dialog"
+						aria-label={comments === 1 ? "1 open comment" : `${comments} open comments`}
+						className="phone-top-bar-comments btn btn-md btn-ghost"
+						data-phone-comments=""
+						onClick={onComments}
+						type="button"
+					>
+						<CommentIcon aria-hidden="true" size={18} />
+						<span aria-hidden="true">{comments}</span>
+					</button>
+				)}
 				{canManage && (
 					<DocumentActionsMenu
 						channel={{ archivedAt, title: label }}

apps/web/src/phone-workspace.css

@@ -120,11 +120,8 @@
 	inset-inline: 0;
 	top: 0;
 	display: grid;
-	grid-template-columns:
-		minmax(var(--control-target-coarse), auto) minmax(0, 1fr) minmax(
-			var(--control-target-coarse),
-			auto
-		);
+	/* Equal sides keep the title on the bar's centre line whatever each side holds. */
+	grid-template-columns: minmax(max-content, 1fr) minmax(0, max-content) minmax(max-content, 1fr);
 	align-items: center;
 	gap: var(--spacing);
 	height: var(--phone-top-chrome);
@@ -194,6 +191,7 @@
 
 .phone-top-bar-slot {
 	display: flex;
+	min-width: var(--control-target-coarse);
 	align-items: center;
 	justify-content: center;
 }
@@ -569,3 +567,24 @@
 		animation: none;
 	}
 }
+
+/* Comments join the actions on the right; each side keeps to its outer edge. */
+.phone-top-bar-slot:first-child {
+	justify-content: flex-start;
+}
+
+.phone-top-bar-slot:last-child {
+	justify-content: flex-end;
+}
+
+.phone-top-bar-slot .phone-top-bar-comments {
+	min-width: var(--control-target-coarse);
+	height: var(--control-target-coarse);
+	gap: var(--spacing);
+	padding-inline: calc(var(--spacing) * 2.5);
+	border-radius: var(--radius-full);
+	color: var(--color-text-secondary);
+	font-size: var(--text-sm);
+	font-weight: var(--font-weight-medium);
+	font-variant-numeric: tabular-nums;
+}

apps/web/src/room-workspace.tsx

@@ -32,6 +32,7 @@ import {
 	Face,
 	firstOpenDecision,
 	GitHubReferencesProvider,
+	openCommentList,
 	opensEditing,
 	PlanEditor,
 	PlanStatus,
@@ -1196,6 +1197,11 @@ export function RoomWorkspace(
 	}
 	let acceptedComments =
 		threadState.threads.filter(item => item.thread.status === "accepted" && !item.applied).length;
+	// The threads a phone's list of comments can show: open, with a passage left to stand on.
+	let openComments =
+		threadState.threads.filter(item =>
+			item.thread.status === "open" && !item.orphaned && !item.drifted
+		).length;
 
 	let statusNotices = (
 		<>
@@ -1479,10 +1485,16 @@ export function RoomWorkspace(
 							<PhoneHeader
 								archivedAt={workspaceArchivedAt}
 								canManage={effectiveCanManage}
+								comments={openComments}
 								editing={titleEdit}
 								editingDocument={!!barEdit?.editing}
 								label={metadata.title}
 								onAction={action => onDocumentAction(room, action)}
+								onComments={() => {
+									// The list stands over the document, so it opens from any tab onto it.
+									selectDestination("plan");
+									openCommentList(room);
+								}}
 								onEditingChange={setTitleEdit}
 								onRenamed={updateMetadata}
 								presentation={presentation}

e2e/chat-phone.e2e.ts

@@ -218,3 +218,60 @@ test("the desktop composer keeps its labelled mode switch, footer and dropdown p
 	]);
 	expect(list!.width).toBeLessThan(pill!.width / 2);
 });
+
+test("a phone composer shows a removable Chopin chip while a message addresses Chopin", async ({ join }) => {
+	let page = await join("ana", PHONE);
+	// The Planner is off in this suite; the session says it is on, as the composer reads it.
+	await page.context().route("**/api/session", async route => {
+		let response = await route.fetch();
+		await route.fulfill({ response, json: { ...await response.json(), agent: true } });
+	});
+	await page.reload();
+	await openPhoneChat(page);
+	let chat = chatPane(page);
+	let draft = chatInput(chat);
+	let chip = chat.getByRole("button", { name: "Remove Chopin", exact: true });
+	// Hidden while the chip shows, but its pressed state still says where the message goes.
+	let mode = chat.getByRole("button", { includeHidden: true, name: "Talk to Chopin", exact: true });
+	await expect(chip).toHaveCount(0);
+
+	// A mention addresses Chopin; the chip says so above the draft, inside the pill.
+	await draft.click();
+	await page.keyboard.type("@chopin what is left? ");
+	await expect(chip).toBeVisible();
+	await expect(mode).toHaveAttribute("aria-pressed", "true");
+	// The chip stands in for the switch while it shows.
+	await expect(mode).toBeHidden();
+	let [chipBox, draftBox, pill] = await Promise.all([
+		chip.boundingBox(),
+		draft.boundingBox(),
+		chat.locator(".composer-surface").boundingBox(),
+	]);
+	expect(chipBox!.y + chipBox!.height).toBeLessThanOrEqual(draftBox!.y + 1);
+	expect(chipBox!.y).toBeGreaterThanOrEqual(pill!.y);
+	// Drawn small, but a full touch target.
+	let reach = await chip.evaluate(element => {
+		let box = element.getBoundingClientRect();
+		let x = box.left + box.width / 2;
+		let y = box.top + box.height / 2;
+		return [y - 21, y + 21].every(at =>
+			document.elementFromPoint(x, at)?.closest("button") === element
+		);
+	});
+	expect(reach).toBe(true);
+
+	// Removing the chip removes the address, as switching to Chat does.
+	await chip.tap();
+	await expect(chip).toHaveCount(0);
+	await expect(mode).toHaveAttribute("aria-pressed", "false");
+	// No leading space is left where the mention was.
+	expect(await draft.textContent()).toBe("what is left? ");
+
+	// The switch turns it back on without a mention.
+	await expect(mode).toBeVisible();
+	await mode.tap();
+	await expect(chip).toBeVisible();
+	await chip.tap();
+	await expect(mode).toHaveAttribute("aria-pressed", "false");
+	await expectNoHorizontalOverflow(page);
+});