From e696f3752577c13638cd117716ba8ccba6e9e27a Mon Sep 17 00:00:00 2001 From: yousefed Date: Mon, 21 Sep 2026 20:00:00 +0200 Subject: [PATCH] fix(core): unwrap a dissolving container in place When a container drops below `min`, `fixContainer` rebuilt it from its surviving child with `tr.replaceWith`, so a caret inside that child collapsed out of it. `fixColumnList` used a `ReplaceAroundStep` here before the rewrite; this restores it for the single-survivor case, which is the only one any container in the repo can reach. Also covers the column layout edges that used to throw out of the keydown handler and had no tests. Co-Authored-By: Claude Opus 5 --- .../containers/containers.test.ts | 64 ++++++++++ .../containers/fixContainer.ts | 43 ++++++- .../src/test/commands/backspace.test.ts | 109 ++++++++++++++++++ .../src/test/commands/removeBlocks.test.ts | 16 +++ 4 files changed, 228 insertions(+), 4 deletions(-) diff --git a/packages/core/src/api/blockManipulation/containers/containers.test.ts b/packages/core/src/api/blockManipulation/containers/containers.test.ts index db5606501c..0f6ea25348 100644 --- a/packages/core/src/api/blockManipulation/containers/containers.test.ts +++ b/packages/core/src/api/blockManipulation/containers/containers.test.ts @@ -320,6 +320,70 @@ describe("children repair", () => { expect(() => editor.prosemirrorState.doc.check()).not.toThrow(); }); + // Dissolving is an unwrap, not a rewrite: the survivor's content is the same + // content, in place, so a caret sitting in it stays where the user put it + // (and a collaborator's concurrent edit still maps onto it). Replacing the + // container with freshly built copies of its children would collapse the + // selection onto the edge of the replaced range instead. + it("keeps the caret in the survivor a container unwraps to", () => { + editor.replaceBlocks(editor.document, [ + { + type: "pair", + id: "pair-0", + children: [ + { id: "pair-a", type: "paragraph", content: "A" }, + { id: "pair-b", type: "paragraph", content: "Survivor" }, + ], + }, + { id: "trailing", type: "paragraph", content: "" }, + ]); + + editor.setTextCursorPosition("pair-b", "end"); + const offsetBefore = editor.prosemirrorState.selection.$from.parentOffset; + + editor.removeBlocks(["pair-a"]); + + expect(editor.getBlock("pair-0")).toBeUndefined(); + expect(editor.getTextCursorPosition().block.id).toBe("pair-b"); + expect(editor.prosemirrorState.selection.$from.parentOffset).toBe( + offsetBefore, + ); + }); + + // The same, for a survivor that is lifted out of two wrappers at once. + it("keeps the caret in a named-only survivor's content", () => { + editor.replaceBlocks(editor.document, [ + { + type: "grid", + id: "g-0", + children: [ + { + type: "gridCell", + id: "cell-a", + children: [{ id: "cell-a-p", type: "paragraph", content: "A" }], + }, + { + type: "gridCell", + id: "cell-b", + children: [{ id: "cell-b-p", type: "paragraph", content: "B" }], + }, + ], + }, + { id: "trailing", type: "paragraph", content: "" }, + ]); + + editor.setTextCursorPosition("cell-b-p", "end"); + const offsetBefore = editor.prosemirrorState.selection.$from.parentOffset; + + editor.removeBlocks(["cell-a-p"]); + + expect(editor.getBlock("g-0")).toBeUndefined(); + expect(editor.getTextCursorPosition().block.id).toBe("cell-b-p"); + expect(editor.prosemirrorState.selection.$from.parentOffset).toBe( + offsetBefore, + ); + }); + // An emptied child of the container is dropped even when the container // stays at or above `min`: an emptied column disappears rather than // lingering. The multicolumn e2e snapshots pin the same behavior from the diff --git a/packages/core/src/api/blockManipulation/containers/fixContainer.ts b/packages/core/src/api/blockManipulation/containers/fixContainer.ts index ea467db1e1..6cff1615a2 100644 --- a/packages/core/src/api/blockManipulation/containers/fixContainer.ts +++ b/packages/core/src/api/blockManipulation/containers/fixContainer.ts @@ -1,5 +1,6 @@ -import { Fragment, type Node } from "prosemirror-model"; +import { Fragment, type Node, Slice } from "prosemirror-model"; import { type Transaction } from "prosemirror-state"; +import { ReplaceAroundStep } from "prosemirror-transform"; import { isContainerNode, @@ -68,11 +69,11 @@ export function fixContainer(tr: Transaction, containerPos: number) { const childrenConfig = container.type.spec.blockConfig?.children; const min = childrenConfig ? (childrenConfig.min ?? 1) : 1; - const survivors: Node[] = []; + const survivors: { node: Node; offset: number }[] = []; const emptied: { from: number; to: number }[] = []; container.forEach((child, offset) => { if (!isEmptyContainerChild(child)) { - survivors.push(child); + survivors.push({ node: child, offset }); } else if (isContainerNode(child.type)) { const from = containerPos + 1 + offset; emptied.push({ from, to: from + child.nodeSize }); @@ -90,8 +91,42 @@ export function fixContainer(tr: Transaction, containerPos: number) { // Too few children left for the container to mean anything, so it is // replaced by its surviving children. + + // A single survivor supplying the whole replacement is an unwrap, and doing + // it as a `ReplaceAroundStep` leaves that content in place in the document: + // positions inside it - a caret, a collaborator's concurrent edit - map + // through it, where deleting and reinserting the same content would collapse + // them onto the edge of the replaced range. + if (survivors.length === 1) { + const [{ node: survivor, offset }] = survivors; + const survivorPos = containerPos + 1 + offset; + // A `namedOnly` survivor is lifted out of two wrappers at once - the column + // and the container - and any other survivor out of just the container. + const gap = isNamedOnly(survivor.type) + ? { from: survivorPos + 1, to: survivorPos + survivor.nodeSize - 1 } + : { from: survivorPos, to: survivorPos + survivor.nodeSize }; + + tr.step( + new ReplaceAroundStep( + containerPos, + containerPos + container.nodeSize, + gap.from, + gap.to, + Slice.empty, + 0, + // Not a pure unwrap: emptied siblings sit outside the gap and go with + // the container, which the `structure` check would refuse. + false, + ), + ); + return; + } + + // Several survivors are not one contiguous range once an emptied child sits + // between them, so this path rebuilds them and positions inside them + // collapse. It takes a container with a `min` of 3 or more to reach. const replacement: Node[] = []; - for (const survivor of survivors) { + for (const { node: survivor } of survivors) { if (isNamedOnly(survivor.type)) { // The survivor can't stand on its own either (a column only exists // inside a column list), so what it holds is what's left. diff --git a/packages/xl-multi-column/src/test/commands/backspace.test.ts b/packages/xl-multi-column/src/test/commands/backspace.test.ts index 87156e81a2..5737f3249b 100644 --- a/packages/xl-multi-column/src/test/commands/backspace.test.ts +++ b/packages/xl-multi-column/src/test/commands/backspace.test.ts @@ -83,6 +83,115 @@ const threeColumnsWithParagraphAbove = [ }, ]; +const columnsOnly = [ + { + type: "columnList" as const, + children: [ + { + type: "column" as const, + children: [ + { id: "col1-para", type: "paragraph" as const, content: "col1" }, + ], + }, + { + type: "column" as const, + children: [ + { id: "col2-para", type: "paragraph" as const, content: "col2" }, + ], + }, + ], + }, +]; + +const columnsWithEmptyFirst = [ + { + type: "columnList" as const, + children: [ + { + type: "column" as const, + children: [ + { id: "empty-para", type: "paragraph" as const, content: "" }, + ], + }, + { + type: "column" as const, + children: [ + { id: "col2-para", type: "paragraph" as const, content: "col2" }, + { id: "col2-trailing", type: "paragraph" as const, content: "" }, + ], + }, + ], + }, +]; + +/** Every bit of text in the document, so a fix can't quietly drop content. */ +function texts(editor: BlockNoteEditor): string[] { + const out: string[] = []; + const walk = (blocks: any[]) => { + for (const block of blocks) { + if (Array.isArray(block.content)) { + const text = block.content.map((c: any) => c.text ?? "").join(""); + if (text) { + out.push(text); + } + } + walk(block.children ?? []); + } + }; + walk(editor.document); + return out.sort(); +} + +// A column layout with nothing after it, and one whose first column is empty, +// both used to throw out of the keydown handler: there is no following block +// to pull in, and no non-empty first column to merge with. The document is +// left alone (or repaired) instead. +describe("Delete and Backspace at a column layout's edges", () => { + it("Delete at the end of the last column, with the layout ending the document", () => { + const editor = getEditor(); + editor.replaceBlocks(editor.document, columnsOnly); + + editor.setTextCursorPosition("col2-para", "end"); + + expect(() => pressDelete(editor)).not.toThrow(); + expect(() => editor.prosemirrorState.doc.check()).not.toThrow(); + expect(texts(editor)).toEqual(["col1", "col2"]); + }); + + it("Delete at the start of an empty first column", () => { + const editor = getEditor(); + editor.replaceBlocks(editor.document, columnsWithEmptyFirst); + + editor.setTextCursorPosition("empty-para", "start"); + + expect(() => pressDelete(editor)).not.toThrow(); + expect(() => editor.prosemirrorState.doc.check()).not.toThrow(); + expect(texts(editor)).toEqual(["col2"]); + }); + + it("Delete at the end of a trailing empty block in the last column", () => { + const editor = getEditor(); + editor.replaceBlocks(editor.document, columnsWithEmptyFirst); + + editor.setTextCursorPosition("col2-trailing", "end"); + + expect(() => pressDelete(editor)).not.toThrow(); + expect(() => editor.prosemirrorState.doc.check()).not.toThrow(); + expect(texts(editor)).toEqual(["col2"]); + }); + + it("Backspace at the start of a column following an empty column", () => { + const editor = getEditor(); + editor.replaceBlocks(editor.document, columnsWithEmptyFirst); + + editor.setTextCursorPosition("col2-para", "start"); + + expect(() => pressBackspace(editor)).not.toThrow(); + expect(() => editor.prosemirrorState.doc.check()).not.toThrow(); + expect(texts(editor)).toEqual(["col2"]); + }); +}); + describe("Backspace with multi-column", () => { // TODO: When migrating to vitest browser mode, replace this test with // a version that presses Backspace 5 times from offset 5 in "hello world" diff --git a/packages/xl-multi-column/src/test/commands/removeBlocks.test.ts b/packages/xl-multi-column/src/test/commands/removeBlocks.test.ts index 031ebfe33d..9a7d18c212 100644 --- a/packages/xl-multi-column/src/test/commands/removeBlocks.test.ts +++ b/packages/xl-multi-column/src/test/commands/removeBlocks.test.ts @@ -79,4 +79,20 @@ describe("Test removeBlocks", () => { expect(getEditor().document).toMatchSnapshot(); }); + + // Removing the other column leaves the column list with one column, so it + // dissolves and the surviving column's blocks are lifted out. That is an + // unwrap of content that never moves, so a caret in it survives untouched. + it("Keeps the cursor in place when removing a column dissolves the columnList", () => { + const editor = getEditor(); + editor.setTextCursorPosition("column-paragraph-2", "end"); + const offsetBefore = editor.prosemirrorState.selection.$from.parentOffset; + + editor.removeBlocks(["column-0"]); + + expect(editor.getTextCursorPosition().block.id).toBe("column-paragraph-2"); + expect(editor.prosemirrorState.selection.$from.parentOffset).toBe( + offsetBefore, + ); + }); });