Open a comment card under its marker with Resolve in view

e2e/comment-sheet.e2e.ts

@@ -668,3 +668,29 @@ test("the desktop header has no comments list button", async ({ join, seed }) =>
 	await expect(page.getByRole("button", { name: /^Comment on “review the pilot”/ })).toBeVisible();
 	await expect(page.getByRole("button", { name: /open comments?$/ })).toHaveCount(0);
 });
+
+test("a desktop card that unfolds a long thread stays in view and scrolls to its reply box", async ({ join, seed }) => {
+	let notes = Array.from(
+		{ length: 9 },
+		(_, index) =>
+			`Reply ${index + 1}: ${"A longer note that wraps across several lines. ".repeat(3)}`,
+	);
+	await seed(SOURCE, { threads: [await open("review the pilot", notes)] });
+	let page = await join("ana", { viewport: { width: 1100, height: 640 } });
+	await page.getByRole("button", { name: /^Comment on “review the pilot”/ }).click();
+	let card = page.getByRole("dialog", { name: "Comment thread" });
+	await expect(card).toBeVisible();
+
+	await card.getByRole("button", { name: /^Show \d+ replies$/ }).click();
+	await expect(card.getByText("Reply 2:")).toBeVisible();
+	// The card is re-placed for its new height: inside the viewport, scrolling within itself.
+	await expect.poll(async () => {
+		let box = await card.boundingBox();
+		return !!box && box.y >= 0 && box.y + box.height <= 640;
+	}).toBe(true);
+	expect(await card.evaluate(element => element.scrollHeight > element.clientHeight)).toBe(true);
+	let reply = card.getByPlaceholder("Reply", { exact: true });
+	await reply.scrollIntoViewIfNeeded();
+	await expectInsideViewport(reply);
+	await reply.fill("Reachable.");
+});

e2e/sidecar.e2e.ts

@@ -232,21 +232,36 @@ test("a narrow split document keeps the desktop comment popover", async ({ join,
 	await seed(PROSE);
 	let page = await join("ana");
 	await page.setViewportSize({ width: 1_024, height: 800 });
+	await page.mouse.move(0, 0);
 	let card = await thread(page);
 	let paragraph = content(page).locator("p").first();
 
 	await expect(card).not.toHaveAttribute("aria-modal", "true");
 	await expect(page.locator("[data-plan-comment-sheet]")).toHaveCount(0);
-	// No gutter is wide enough here, so the card sits below the passage inside its column.
+	// No gutter is wide enough here, so the card sits below the passage, under its marker:
+	// right-aligned with the paragraph.
+	await expect(card).toHaveAttribute("data-side", "below");
 	await expect.poll(async () => {
 		let cardBox = await card.boundingBox();
 		let paragraphBox = await paragraph.boundingBox();
 		if (!cardBox || !paragraphBox) return false;
 		return cardBox.width <= 320
 			&& cardBox.y >= paragraphBox.y + paragraphBox.height
 			&& cardBox.x >= paragraphBox.x - 1
-			&& cardBox.x + cardBox.width <= paragraphBox.x + paragraphBox.width + 1;
+			&& Math.abs(cardBox.x + cardBox.width - (paragraphBox.x + paragraphBox.width)) <= 2;
 	}).toBe(true);
+	// Resolve is one click from opening the card, without pointing at the card first.
+	let resolve = card.getByRole("button", { name: "Resolve", exact: true });
+	await expect(resolve).toBeVisible();
+	await expect.poll(() =>
+		resolve.evaluate(element => {
+			let opacity = 1;
+			for (let at: Element | null = element; at; at = at.parentElement) {
+				opacity *= Number(getComputedStyle(at).opacity);
+			}
+			return opacity;
+		})
+	).toBe(1);
 });
 
 test(

packages/editor/src/comment-geometry.test.ts

@@ -204,33 +204,45 @@ function overlaps(card: CardPoint, height: number, target: Rect, page: Rect): bo
 		&& right > target.left;
 }
 
-test("places a comment card directly below its passage, at the passage's left edge", () => {
+test("places a comment card directly below its passage, at the column's right edge", () => {
 	expect(commentCardPoint(passage(200), column, host, 320, 200))
-		.toEqual({ top: 132, left: 200, width: 320, side: "below" });
+		.toEqual({ top: 132, left: 280, width: 320, side: "below" });
 });
 
-test("keeps a comment card inside the prose column", () => {
-	expect(commentCardPoint(passage(200, 600, 690), column, host, 320, 200).left).toBe(280);
-	expect(commentCardPoint(passage(200), { ...column, width: 280 }, host, 320, 200).width)
-		.toBe(280);
+test("right-aligns a comment card to the column wherever its passage starts", () => {
+	for (let left of [200, 300, 600]) {
+		let card = commentCardPoint(passage(200, left, 690), column, host, 320, 200);
+		expect(host.left + card.left + card.width).toBe(column.right);
+	}
+	let narrow = commentCardPoint(passage(200), { ...column, left: 420, width: 280 }, host, 320, 200);
+	expect(narrow).toMatchObject({ left: 320, width: 280 });
+});
+
+test("keeps a draft at its selection's left edge, inside the column", () => {
+	let options = { align: "passage" as const };
+	expect(commentCardPoint(passage(200), column, host, 320, 200, options))
+		.toEqual({ top: 132, left: 200, width: 320, side: "below" });
+	expect(commentCardPoint(passage(200, 600, 690), column, host, 320, 200, options).left).toBe(280);
 });
 
 test("flips a comment card above its passage when there is no room below", () => {
 	expect(commentCardPoint(passage(600), column, host, 320, 200))
-		.toEqual({ top: 292, left: 200, width: 320, side: "above" });
+		.toEqual({ top: 292, left: 280, width: 320, side: "above" });
 });
 
 test("caps a card that fits neither side to the roomier side of its passage", () => {
 	expect(commentCardPoint(passage(300), column, host, 320, 500))
-		.toEqual({ top: 232, left: 200, width: 320, side: "below", maxHeight: 356 });
+		.toEqual({ top: 232, left: 280, width: 320, side: "below", maxHeight: 356 });
 	expect(commentCardPoint(passage(500), column, host, 320, 500))
-		.toEqual({ top: 12, left: 200, width: 320, side: "above", maxHeight: 380 });
+		.toEqual({ top: 12, left: 280, width: 320, side: "above", maxHeight: 380 });
 });
 
 test("puts a comment card in a wide gutter beside the passage's first line", () => {
 	let wide = { ...host, right: 1_300, width: 1_200 };
 	expect(commentCardPoint(passage(200), column, wide, 320, 200))
 		.toEqual({ top: 100, left: 656, width: 320, side: "right" });
+	expect(commentCardPoint(passage(200), column, wide, 320, 900))
+		.toEqual({ top: 12, left: 656, width: 320, side: "right", maxHeight: 576 });
 	let snug = { ...host, right: 1_036, width: 936 };
 	expect(commentCardPoint(passage(200), column, snug, 320, 200).width).toBe(268);
 });

packages/editor/src/comment-geometry.ts

@@ -209,7 +209,9 @@ export type CardPoint = Point & { width: number; side: CardSide; maxHeight?: num
  * When the gutter right of the prose column holds the card beyond the marker
  * lane, it sits there beside the passage's first line. Otherwise it sits
  * directly below the passage, or above it when only that side has room, aligned
- * to the passage's left edge and kept inside the column. When neither side
+ * to the column's right edge so it opens beneath the marker that raised it. A
+ * draft has no marker yet: `align: "passage"` keeps it at the selection's left
+ * edge instead, inside the column. When neither side
  * holds the whole card, the roomier side caps its height so it scrolls instead.
  * A passage scrolled out of view lets the card clamp back inside the document.
  */
@@ -219,7 +221,8 @@ export function commentCardPoint(
 	host: Rect,
 	width: number,
 	height: number,
-	{ gap = 8, inset = 12, lane = 56, minWidth = 264 }: {
+	{ align = "column", gap = 8, inset = 12, lane = 56, minWidth = 264 }: {
+		align?: "column" | "passage";
 		gap?: number;
 		inset?: number;
 		lane?: number;
@@ -229,20 +232,20 @@ export function commentCardPoint(
 	let gutter = host.right - inset - (column.right + lane);
 	if (gutter >= minWidth) {
 		let fitted = Math.min(width, gutter);
+		let room = host.height - inset * 2;
 		return {
 			top: clamp(passage.top - host.top, inset, host.height - height - inset),
 			left: column.right + lane - host.left,
 			width: fitted,
 			side: "right",
+			...(height > room ? { maxHeight: room } : {}),
 		};
 	}
 
 	let fitted = Math.min(width, column.width);
-	let left = clamp(
-		passage.left - host.left,
-		column.left - host.left,
-		column.right - host.left - fitted,
-	);
+	let left = align === "passage"
+		? clamp(passage.left - host.left, column.left - host.left, column.right - host.left - fitted)
+		: clamp(column.right - host.left - fitted, 0, host.width - fitted);
 	let below = host.bottom - inset - (passage.bottom + gap);
 	let above = passage.top - gap - (host.top + inset);
 	let point: CardPoint = {

packages/editor/src/comment-layer.tsx

@@ -342,8 +342,21 @@ function CommentSurface(
 	},
 ) {
 	let lifecycle = useCommentPresence(value, compact ? 180 : 150, immediately);
-	if (lifecycle.presence.phase === "closed") return null;
-	let surface = lifecycle.presence.value;
+	let surface = lifecycle.presence.phase === "closed" ? undefined : lifecycle.presence.value;
+	let element = useRef<HTMLDivElement | null>(null);
+	let measure = useRef(surface?.onMeasure);
+	measure.current = surface?.onMeasure;
+	let open = !!surface;
+	// Content can grow after the card is placed, as when a long thread unfolds its replies,
+	// so the card is measured again and re-placed rather than running off the document.
+	useLayoutEffect(() => {
+		let card = element.current;
+		if (!open || !card) return;
+		let observer = new ResizeObserver(() => measure.current?.(card));
+		for (let child of card.children) observer.observe(child);
+		return () => observer.disconnect();
+	}, [open, surface?.id]);
+	if (!surface) return null;
 	return (
 		<div
 			aria-hidden={lifecycle.ariaHidden}
@@ -356,7 +369,10 @@ function CommentSurface(
 			inert={lifecycle.inert}
 			onMouseEnter={surface.onMouseEnter}
 			onMouseLeave={surface.onMouseLeave}
-			ref={surface.onMeasure}
+			ref={node => {
+				element.current = node;
+				surface.onMeasure?.(node);
+			}}
 			role="dialog"
 			style={surface.style}
 		>
@@ -1083,14 +1099,14 @@ export function CommentLayer({ store }: { store: ThreadStore }) {
 	let editorRoot = editor.getRootElement();
 	let column = editorRoot ? proseColumn(editorRoot) : page;
 	let cardWidth = Math.min(320, host.clientWidth - 24);
-	let cardPlacement = (passage: Rect, id: string) => {
+	let cardPlacement = (passage: Rect, id: string, align?: "passage") => {
 		let { side, ...style } = commentCardPoint(
 			passage,
 			column,
 			page,
 			cardWidth,
 			cardHeights[id] ?? 0,
-			{ lane: MARKER_LANE },
+			{ align, lane: MARKER_LANE },
 		);
 		return { side, style };
 	};
@@ -1394,7 +1410,7 @@ export function CommentLayer({ store }: { store: ThreadStore }) {
 						),
 						className: "plan-comment-card",
 						onMeasure: element => rememberHeight("draft", element),
-						...cardPlacement(draft.placement, "draft"),
+						...cardPlacement(draft.placement, "draft", "passage"),
 					}
 					: undefined}
 			/>

packages/editor/src/comments.tsx

@@ -163,7 +163,7 @@ function PlannerStatus({ state }: { state: NonNullable<ThreadView["planner"]> })
 	);
 }
 
-/** The `…` menu and the resolve check, revealed on hover or focus and always shown on touch. */
+/** The resolve check, always shown, and the `…` menu, revealed on hover or focus. */
 function Actions({ link, onResolve }: { link?: string; onResolve?: () => void }) {
 	let [menu, setMenu] = useState(false);
 	let trigger = useRef<HTMLButtonElement>(null);

packages/editor/src/styles.css

@@ -841,12 +841,12 @@
 
 .plan-comment-card[data-side="below"] {
 	--enter-y: -0.25rem;
-	transform-origin: top left;
+	transform-origin: top right;
 }
 
 .plan-comment-card[data-side="above"] {
 	--enter-y: 0.25rem;
-	transform-origin: bottom left;
+	transform-origin: bottom right;
 }
 
 /*
@@ -899,25 +899,29 @@
 	text-wrap: pretty;
 }
 
-/* Revealed on hover or focus, always on touch; inset equally from the card's top and right. */
+/* Inset equally from the card's top and right. */
 .plan-comment-actions {
 	display: flex;
 	flex: none;
 	align-items: center;
 	align-self: flex-start;
 	gap: 0.125rem;
 	margin: -0.375rem -0.375rem 0 auto;
+}
+
+/* Resolve stays in view; the `…` menu is revealed on hover or focus, always on touch. */
+.plan-comment-menu-anchor {
 	opacity: 0;
 	transition: opacity var(--duration-fast) var(--ease-out);
 }
 
-.plan-comment-thread:is(:hover, :focus-within) .plan-comment-actions,
-.plan-comment-actions[data-open] {
+.plan-comment-thread:is(:hover, :focus-within) .plan-comment-menu-anchor,
+.plan-comment-actions[data-open] .plan-comment-menu-anchor {
 	opacity: 1;
 }
 
 @media (hover: none) {
-	.plan-comment-actions {
+	.plan-comment-menu-anchor {
 		opacity: 1;
 	}
 }
@@ -1876,6 +1880,9 @@ body:has(.plan-comment-sheet-popup[data-swiping])
 /* Actions are always shown in a sheet, each a full 44px target that keeps the note head's line. */
 .plan-comment-sheet-content .plan-comment-actions {
 	margin: -0.625rem -0.75rem -0.625rem auto;
+}
+
+.plan-comment-sheet-content .plan-comment-menu-anchor {
 	opacity: 1;
 }
 
@@ -1913,7 +1920,7 @@ body:has(.plan-comment-sheet-popup[data-swiping])
 		animation: none;
 	}
 
-	.plan-comment-actions,
+	.plan-comment-menu-anchor,
 	.plan-comment-composer-fold,
 	.plan-comment-composer-footer,
 	.plan-comment-menu,

scripts/design-contract/exceptions/dynamic-editor.json

@@ -214,7 +214,7 @@
 				1
 			]
 		],
-		"sourceHash": "b3ba35a387b51602f42db5f1158514b31146e1f33679e17d88f8400ca115db30"
+		"sourceHash": "471ede86d5440a1ca80c90259133f954048f12a185ee2e5c89ecea1b8704b6da"
 	},
 	{
 		"file": "packages/editor/src/comment-layer.tsx",
@@ -242,7 +242,7 @@
 				1
 			]
 		],
-		"sourceHash": "b3ba35a387b51602f42db5f1158514b31146e1f33679e17d88f8400ca115db30"
+		"sourceHash": "471ede86d5440a1ca80c90259133f954048f12a185ee2e5c89ecea1b8704b6da"
 	},
 	{
 		"file": "packages/editor/src/table/rails.tsx",