fix(nesting): pluralize the collapsed summary, trim comments

The collapsed container summary was built by concatenating counts with
translated words and a literal comma, which fixes English word order and
punctuation. It is now a single message with plural forms for both counts.

The comments added across these changes carried decision rationale and
narrative about what was tried. Cut back to the invariants a reader would
otherwise get wrong: the handle gutter and its offset have to agree, the
admin's grid template mirrors core's, and the node's contentDOM stays
mounted while collapsed.

Author: bimsonz

packages/admin/src/components/editor/DragHandleWrapper.tsx

@@ -38,26 +38,18 @@ export function _getDragHandlePlacement(direction: "ltr" | "rtl") {
 }
 
 /**
- * How far to place the handle from the row it belongs to.
- *
- * A top level row sits inside the editor's own 64px gutter, so its handle goes just
- * outside the row. A row inside a nesting column carries the gutter as its own
- * leading padding instead (see NESTING_GUTTER_PX), which is what lets a pointer in
- * the gutter still resolve to that row -- so the handle has to come *back* across
- * the row's edge to land in that padding rather than out over the column beside it.
+ * A top level row's handle sits outside it, in the editor's own gutter. A row in a
+ * column carries that gutter as its own leading padding, so the handle moves back
+ * across the row's edge to land inside it. Must agree with NESTING_GUTTER_PX.
  */
 export function _dragHandleOffset(insideColumn: boolean): number {
 	return insideColumn ? -(NESTING_GUTTER_PX - 4) : 4;
 }
 
 /**
- * Is the row at `pos` a child of a nesting column?
- *
- * Read from the document, because the handle positions against a virtual element
- * that carries only a rect with no way back to the node it came from. `pos` is the
- * position before the row, so its parent is the column that would hold it -- only
- * the immediate parent is checked, because only a direct child of a column is ever
- * a drag target.
+ * Resolved from the document rather than the hovered element, which is virtual and
+ * carries only a rect. `pos` is the position before the row, so its parent is the
+ * column that would hold it.
  */
 export function _isInsideNestingColumn(editor: Editor, pos: number): boolean {
 	if (pos < 0) return false;
@@ -70,29 +62,8 @@ export function _isInsideNestingColumn(editor: Editor, pos: number): boolean {
 }
 
 /**
- * The draggable unit is a row: a child of the document, or a child of a nesting
- * column, which is the same thing one level down.
- *
- * This is what the editor already did. With nested targeting off, TipTap targets
- * top level blocks, so a list drags as one block and its items do not drag at all.
- * The rule restates that and extends it into columns, which is why behaviour
- * outside a container is unchanged by enabling nesting.
- *
- * It deliberately replaces TipTap's default rules rather than joining them, and
- * that is a choice about units, not a claim that they are wrong. Their defaults
- * resolve the unit *inside* a structure: for a list, `listItemFirstChild` and
- * `listWrapperDeprioritize` between them exclude the paragraph and the wrapper so
- * the list item wins. That is right for a plain document and wrong for a page
- * built from containers, where a list is one row a page is composed of and its
- * items are the row's internals. The two cannot both hold, and picking theirs
- * means a list can no longer be moved as a block anywhere in the document.
- *
- * Nothing the defaults guard is lost. Table internals and inline content are
- * never children of the document or of a column, so they are excluded here by
- * construction; the tests assert that rather than assuming it.
- *
- * Columns themselves are never a target either: `selectable: false`, and they are
- * added and removed from the container's toolbar.
+ * Drag unit: direct children of the document or of a nesting column.
+ * Table internals and inline content are excluded by the schema.
  */
 export const _rowsOnlyRule: DragHandleRule = {
 	id: "emdashRowsOnly",
@@ -104,25 +75,7 @@ export const _rowsOnlyRule: DragHandleRule = {
 	},
 };
 
-/**
- * Nested targeting, so a block inside a nesting column can be reordered.
- *
- * Edge detection is off, and follows from the same choice. It resolves ambiguity by
- * pointer position, deducting by depth near a node's edge so the parent wins there.
- * With rows as the unit there is no ambiguity left to resolve: a column is never a
- * target, so the only candidates are the row and its container, and the container
- * has an explicit grab point of its own in its header. Leaving it on only breaks
- * things, because the deduction excludes a candidate outright at the depth a row
- * sits inside a column, and rows are often short enough that the 12px band covers
- * half of one.
- *
- * The handle's placement depends on this too. The gutter it sits in belongs to the
- * row, so a target deeper than a row is measured from the wrong box and the handle
- * lands over the content instead of beside it.
- *
- * Module level so the reference is stable: DragHandle's effect depends on it, and a
- * fresh object each render re-registers the plugin.
- */
+/** Module level: DragHandle re-registers its plugin if this identity changes. */
 export const _nestedDragOptions = {
 	rules: [_rowsOnlyRule],
 	defaultRules: false,
@@ -207,8 +160,7 @@ export function DragHandleWrapper({ editor, onInsertBlock }: DragHandleWrapperPr
 		editor.commands.setMeta("lockDragHandle", false);
 	}, [editor]);
 
-	// Written synchronously here and read by the offset middleware, which the drag
-	// handle invokes immediately afterwards to reposition.
+	// Set in onNodeChange, read by the offset middleware that runs straight after it.
 	const insideColumnRef = React.useRef(false);
 
 	// Handle node change from drag handle

packages/admin/src/components/editor/HtmlBlockNode.tsx

@@ -85,9 +85,6 @@ function HtmlBlockNodeView({ node, updateAttributes, selected, deleteNode }: Nod
 			contentEditable={false}
 			data-drag-handle
 		>
-			{/* No grip of its own -- see PluginBlockNode. The editor's drag handle covers
-			    every block, and -start-8 puts this one in a gutter a nesting column does
-			    not have. */}
 			<div className="relative group">
 				{/* Main block */}
 				<div

packages/admin/src/components/editor/NestingBlockNode.tsx

@@ -13,6 +13,7 @@
  */
 
 import { Button, Select } from "@cloudflare/kumo";
+import { plural } from "@lingui/core/macro";
 import { useLingui } from "@lingui/react/macro";
 import { CaretDown, CaretRight, Plus, Rows, SquaresFour, Trash, X } from "@phosphor-icons/react";
 import { Node, mergeAttributes } from "@tiptap/core";
@@ -45,9 +46,8 @@ const MIN_COLUMNS = 1;
 const MAX_COLUMNS = 6;
 
 /**
- * Width reserved inside every row of a column for its drag handle, wide enough for
- * the handle cluster plus a small gap. Exported so the drag handle can offset itself
- * into it -- the two numbers have to agree or the handle lands on the row's content.
+ * Gutter reserved inside each row of a column for its drag handle. Must agree with
+ * `_dragHandleOffset` or the handle lands on the row's content.
  */
 export const NESTING_GUTTER_PX = 52;
 
@@ -84,24 +84,14 @@ const NESTING_STYLES = `
 .nesting-column-content {
 	min-height: 2.5rem;
 }
-/*
- * The drag handle's gutter belongs to each row, not to the column.
- *
- * Padding the column instead would put the gutter outside every row's box, so a
- * pointer there resolves to the column, which is never a drag target, and the
- * container wins instead. In use that reads as the handle jumping away exactly
- * as you reach for it. Owning the padding keeps a pointer in the gutter inside
- * the row it belongs to.
- */
+/* Padded onto each row, not the column, so a pointer in the gutter still
+ * resolves to the row it belongs to. */
 .nesting-column-content > [data-node-view-content-react] > * {
 	padding-inline-start: var(--nesting-gutter);
 }
-/*
- * A row that indents its own content keeps that indent on top of the gutter.
- * Setting the gutter alone replaces it, which pulls list markers and a quote's
- * rule back into the gutter and leaves them under the drag handle. The added
- * values are the editor's own defaults for these elements.
- */
+/* A row with its own indent adds it to the gutter; setting the gutter alone
+ * replaces it and draws markers under the handle. Added values are the
+ * editor's defaults for these elements. */
 .nesting-column-content > [data-node-view-content-react] > :is(ul, ol) {
 	padding-inline-start: calc(var(--nesting-gutter) + 1.625rem);
 }
@@ -125,9 +115,9 @@ function ensureNestingStyles(): void {
 }
 
 /**
- * `grid-template-columns` for a width preset. Mirrors `nestingTemplateColumns`
- * in @emdash-cms/core so the editor preview matches what the site renders; the
- * admin does not depend on core, hence the duplication (as with GAP_TO_CSS).
+ * `grid-template-columns` for a width preset. Mirrors `nestingTemplateColumns` in
+ * @emdash-cms/core, which the admin cannot import; they have to stay in step or the
+ * editor preview and the rendered page disagree.
  */
 function templateColumns(widths: NestingWidths, columnCount: number): string {
 	const n = Math.max(MIN_COLUMNS, Math.min(MAX_COLUMNS, columnCount));
@@ -182,8 +172,7 @@ function NestingColumnNodeView({ editor, getPos, node }: NodeViewProps) {
 
 	return (
 		<NodeViewWrapper
-			// The drag handle's gutter is padded onto each row rather than onto the column
-			// -- see NESTING_STYLES for why.
+			// The handle's gutter is padded onto each row, not here. See NESTING_STYLES.
 			className="nesting-column group/col relative rounded-md border border-kumo-line p-2"
 			data-emdash-nesting-column
 		>
@@ -258,18 +247,12 @@ function NestingBlockNodeView({
 
 	const columnCount = node.childCount;
 
-	/**
-	 * Collapsed is view state, not content: it is deliberately not a node attribute,
-	 * so folding a container away is never a document change and never lands in a
-	 * revision. It resets on reload, which matches how editors expect a disclosure to
-	 * behave.
-	 */
+	// View state, not a node attribute: folding a container is not a document change
+	// and must not reach a revision.
 	const [collapsed, setCollapsed] = React.useState(false);
 
-	// Ties the disclosure button to the region it shows and hides.
 	const contentId = React.useId();
 
-	// What the container holds, for the summary shown when it is folded away.
 	const blockCount = React.useMemo(() => {
 		let total = 0;
 		node.forEach((column) => {
@@ -301,12 +284,6 @@ function NestingBlockNodeView({
 				className="flex flex-wrap items-center gap-2 border-b border-kumo-line px-3 py-2"
 				contentEditable={false}
 			>
-				{/* The title is the grab area, the way a window is dragged by its bar. There
-				    was a separate grip here, permanently visible, which nothing else in the
-				    editor has: every other row is dragged from a handle that appears in the
-				    gutter on hover. A container is a row too, but hovering one resolves to
-				    the deepest row inside it, so it still needs somewhere of its own to be
-				    picked up -- its header, rather than an icon that is always on screen. */}
 				<Button
 					type="button"
 					variant="ghost"
@@ -332,22 +309,13 @@ function NestingBlockNodeView({
 				>
 					{layout === "grid" ? <SquaresFour className="h-4 w-4" /> : <Rows className="h-4 w-4" />}
 					<span className="text-sm font-medium">{t`Nesting container`}</span>
-					{/* Counts sit outside the translated words rather than inside a plural
-					    message, so the summary reads correctly before catalogs are extracted.
-					    A `<Plural>` here renders its own ICU source until then, which is worse
-					    to look at than an unagreed plural. */}
 					{collapsed && (
 						<span className="text-kumo-subtle/70 text-sm">
-							{columnCount} {t`columns`}
-							{", "}
-							{blockCount} {t`blocks`}
+							{t`${plural(columnCount, { one: "# column", other: "# columns" })}, ${plural(blockCount, { one: "# block", other: "# blocks" })}`}
 						</span>
 					)}
 				</div>
 
-				{/* Layout controls describe content that is not on screen while collapsed, so
-				    they fold away with it. Delete stays: an unwanted container should not have
-				    to be opened first. */}
 				<div className="ms-auto flex flex-wrap items-center gap-2">
 					{!collapsed && (
 						<>
@@ -374,11 +342,8 @@ function NestingBlockNodeView({
 									stretch: t`Stretch`,
 								}}
 							/>
-							{/* Equal columns cannot express a content-plus-sidebar page, which is the
-					    most common two-column layout, so the weighted presets exist for that.
-					    Grid only: a flex container sizes its columns from their content, and
-					    the site renderer ignores widths there too, so it is disabled rather
-					    than left to look like it works. */}
+							{/* Grid only: a flex container sizes columns from their content, and the
+							    site renderer ignores widths there. */}
 							<Select
 								label={t`Widths`}
 								value={widths}
@@ -419,9 +384,8 @@ function NestingBlockNodeView({
 					</Button>
 				</div>
 			</div>
-			{/* Hidden rather than unmounted: ProseMirror owns this element as the node's
-			    contentDOM, and removing it detaches the container's content from the
-			    document. */}
+			{/* Hidden, not unmounted: ProseMirror owns this element as the node's
+			    contentDOM. */}
 			<NodeViewContent
 				id={contentId}
 				className={cn("nesting-block-content p-3", collapsed && "hidden")}

packages/admin/src/components/editor/PluginBlockNode.tsx

@@ -264,12 +264,6 @@ function PluginBlockNodeView({
 			contentEditable={false}
 			data-drag-handle
 		>
-			{/* No grip of its own: the editor's drag handle already offers one for every
-			    block, and the wrapper above carries data-drag-handle, so a third one only
-			    ever sat underneath the other two. It was positioned at -start-8, in the
-			    editor's left gutter, which does not exist inside a nesting column -- there
-			    the grip hung outside the column, over its neighbour or outside the
-			    container entirely. */}
 			<div className="relative group">
 				{/* Main block content */}
 				<div
@@ -278,10 +272,8 @@ function PluginBlockNodeView({
 						selected ? "border-kumo-brand/50 bg-kumo-tint/30" : "hover:border-kumo-line",
 					)}
 				>
-					{/* Header with icon, label, and actions.
-					    Wraps because the action buttons hold their width even while hidden: in a
-					    narrow container (a sidebar column of a nesting block) they left the label
-					    a few pixels and it broke one character per line. */}
+					{/* Wraps because the action buttons hold their width while hidden, leaving
+					    the label almost none in a narrow column. */}
 					<div className="flex flex-wrap items-center gap-3 px-4 py-3">
 						{/* Icon */}
 						<div

packages/admin/tests/editor/DragHandleWrapper.interactions.test.tsx

@@ -21,10 +21,7 @@ vi.mock("@tiptap/extension-drag-handle-react", () => ({
 			className="drag-handle"
 			draggable="true"
 			data-placement={computePositionConfig.placement}
-			// The offset is a function so it can differ for a row inside a nesting
-			// column, which carries the handle's gutter as its own padding. Resolve it
-			// the way floating-ui would; with nothing hovered it reports the top level
-			// value. `_dragHandleOffset` is unit tested directly for both cases.
+			// Resolve the offset the way floating-ui would: it may be a function.
 			data-offset={(() => {
 				const option = computePositionConfig.middleware?.find(({ name }) => name === "offset")
 					?.options?.[0];

packages/admin/tests/editor/DragHandleWrapper.test.ts

@@ -60,15 +60,6 @@ describe("DragHandleWrapper", () => {
 	});
 });
 
-/**
- * The draggable unit is a row, and these assert both halves of that claim.
- *
- * The rule replaces TipTap's default rules rather than joining them, so the tests
- * have to show that nothing the defaults guarded is lost: table internals and
- * inline content must still be excluded, by the rule rather than by assumption.
- * They also pin the behaviour outside a container, which enabling nested targeting
- * must not change.
- */
 describe("rows are the draggable unit", () => {
 	// Node views are React renderers and are not needed to exercise the schema.
 	const Block = NestingBlockExtension.extend({ addNodeView: undefined });
@@ -138,9 +129,6 @@ describe("rows are the draggable unit", () => {
 		`<div data-emdash-nesting-block><div data-emdash-nesting-column>${inner}</div></div>`;
 
 	it("targets a list as one row, not its items, in the body", () => {
-		// This is what the editor did before nested targeting was enabled. Picking
-		// TipTap's defaults instead would target the item and make a list impossible
-		// to move as a block anywhere in the document.
 		withEditor("<ul><li><p>Top item</p></li></ul>", (editor) => {
 			expect(targetFor(editor, "Top item")).toBe("bulletList");
 		});
@@ -167,7 +155,7 @@ describe("rows are the draggable unit", () => {
 		});
 	});
 
-	it("never targets table internals, which the default rules used to guard", () => {
+	it("never targets table internals", () => {
 		withEditor("<table><tbody><tr><td><p>Cell</p></td></tr></tbody></table>", (editor) => {
 			// The table is the row; everything inside it is the row's structure.
 			expect(targetFor(editor, "Cell")).toBe("table");
@@ -194,8 +182,6 @@ describe("nested drag options", () => {
 	const normalized = normalizeNestedOptions(_nestedDragOptions);
 
 	it("replaces the default rules rather than joining them", () => {
-		// Deliberate: the defaults resolve the unit inside a structure, this resolves
-		// which structures are units. See _rowsOnlyRule for why both cannot hold.
 		expect(normalized.defaultRules).toBe(false);
 		expect(normalized.rules.map((rule) => rule.id)).toEqual(["emdashRowsOnly"]);
 	});

packages/admin/tests/editor/nesting-block-conversion.test.ts

@@ -1,11 +1,7 @@
 /**
- * Nesting Block Conversion Tests (admin editor seam)
- *
- * The admin editor carries its own Portable Text converters, separate from the
- * ones in @emdash-cms/core. Core's round-trip tests therefore say nothing about
- * what an editor actually saves: a nesting attribute can be handled correctly in
- * core, pass its tests, and still be dropped on every real save. That is what
- * happened to `widths` -- the container reverted to equal columns on reload.
+ * The admin editor carries its own Portable Text converters, separate from the ones
+ * in @emdash-cms/core, and a save goes through these. Core's round-trip tests do not
+ * cover them: an attribute can round-trip in core and still be dropped on every save.
  */
 
 import { describe, it, expect } from "vitest";

packages/core/src/content/converters/types.ts

@@ -132,10 +132,9 @@ export type NestingLayout = "grid" | "flex";
 export type NestingGap = "none" | "sm" | "md" | "lg";
 export type NestingAlign = "start" | "center" | "end" | "stretch";
 /**
- * Relative column widths. `equal` keeps every column the same size; the others
- * weight the first or last column, which is what a content-plus-sidebar page needs.
- * Ratios are applied to the first/last column and the rest stay equal, so the value
- * stays meaningful at any column count.
+ * Relative column widths. `equal` sizes every column the same; the others weight the
+ * first or last column and leave the rest equal, so the value stays meaningful at any
+ * column count.
  */
 export type NestingWidths = "equal" | "wide-first" | "wide-last" | "narrow-first" | "narrow-last";