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, + ); + }); });