From d0ea3cca32f92bc561eeebe51972621789f0c75f Mon Sep 17 00:00:00 2001 From: Matthew Lipski Date: Mon, 8 Jun 2026 10:38:22 +0200 Subject: [PATCH] Fixed error triggered when moving blocks in some cases with multi-columns --- .../commands/moveBlocks/moveBlocks.ts | 26 ++- .../commands/replaceBlocks/replaceBlocks.ts | 21 ++- .../__snapshots__/moveBlocks.test.ts.snap | 178 ++++++++++++++++++ .../src/test/commands/moveBlocks.test.ts | 47 ++++- 4 files changed, 266 insertions(+), 6 deletions(-) diff --git a/packages/core/src/api/blockManipulation/commands/moveBlocks/moveBlocks.ts b/packages/core/src/api/blockManipulation/commands/moveBlocks/moveBlocks.ts index bb2f08dfca..1ba40df652 100644 --- a/packages/core/src/api/blockManipulation/commands/moveBlocks/moveBlocks.ts +++ b/packages/core/src/api/blockManipulation/commands/moveBlocks/moveBlocks.ts @@ -11,6 +11,9 @@ import type { BlockNoteEditor } from "../../../../editor/BlockNoteEditor"; import { BlockIdentifier } from "../../../../schema/index.js"; import { getNearestBlockPos } from "../../../getBlockInfoFromPos.js"; import { getNodeById } from "../../../nodeUtil.js"; +import { insertBlocks } from "../insertBlocks/insertBlocks.js"; +import { removeAndInsertBlocks } from "../replaceBlocks/replaceBlocks.js"; +import { fixColumnList } from "../replaceBlocks/util/fixColumnList.js"; type BlockSelectionData = ( | { @@ -148,7 +151,7 @@ export function moveBlocks( referenceBlock: BlockIdentifier, placement: "before" | "after", ) { - editor.transact(() => { + editor.transact((tr) => { // A `columnList` reference can be dissolved by `fixColumnList` when its // `column`s are removed, leaving its ID invalid for re-insertion. Anchor // to an adjacent block instead, which is unaffected by the removal. @@ -164,8 +167,25 @@ export function moveBlocks( } } - editor.removeBlocks(blocks); - editor.insertBlocks(flattenColumns(blocks), referenceBlock, placement); + // Don't fix columns/columnLists in the removal step. Otherwise, the + // following case breaks: + // + // + // Paragraph + // + // When the non-empty block is moved up, the column is now seen as empty + // and collapsed in the removal step, so the following insertion fails. + const { affectedColumnLists } = removeAndInsertBlocks(tr, blocks, [], { + fixColumns: false, + }); + insertBlocks(tr, flattenColumns(blocks), referenceBlock, placement); + + affectedColumnLists.forEach((id) => { + const posInfo = getNodeById(id, tr.doc); + if (posInfo) { + fixColumnList(tr, posInfo.posBeforeNode); + } + }); }); } diff --git a/packages/core/src/api/blockManipulation/commands/replaceBlocks/replaceBlocks.ts b/packages/core/src/api/blockManipulation/commands/replaceBlocks/replaceBlocks.ts index f1e946f909..4942689570 100644 --- a/packages/core/src/api/blockManipulation/commands/replaceBlocks/replaceBlocks.ts +++ b/packages/core/src/api/blockManipulation/commands/replaceBlocks/replaceBlocks.ts @@ -20,9 +20,13 @@ export function removeAndInsertBlocks< tr: Transaction, blocksToRemove: BlockIdentifier[], blocksToInsert: PartialBlock[], + options: { + fixColumns?: boolean; + } = {}, ): { insertedBlocks: Block[]; removedBlocks: Block[]; + affectedColumnLists: string[]; } { const pmSchema = getPmSchema(tr); // Converts the `PartialBlock`s to ProseMirror nodes to insert them into the @@ -112,12 +116,25 @@ export function removeAndInsertBlocks< ); } - columnListPositions.forEach((pos) => fixColumnList(tr, pos)); + // Saves IDs of columnLists containing removed blocks. If `fixColumns` is + // explicitly false, these are needed to run `fixColumnList` manually later. + const affectedColumnLists: string[] = []; + columnListPositions.forEach((pos) => { + const columnList = tr.doc.resolve(pos).nodeAfter; + if (columnList?.type.name === "columnList") { + affectedColumnLists.push(columnList.attrs.id); + } + }); + + // Collapses empty columns/columnLists + if (options.fixColumns !== false) { + columnListPositions.forEach((pos) => fixColumnList(tr, pos)); + } // Converts the nodes created from `blocksToInsert` into full `Block`s. const insertedBlocks = nodesToInsert.map((node) => nodeToBlock(node, pmSchema), ) as Block[]; - return { insertedBlocks, removedBlocks }; + return { insertedBlocks, removedBlocks, affectedColumnLists }; } diff --git a/packages/xl-multi-column/src/test/commands/__snapshots__/moveBlocks.test.ts.snap b/packages/xl-multi-column/src/test/commands/__snapshots__/moveBlocks.test.ts.snap index 4d6a2cf1f7..6eaa4edfd0 100644 --- a/packages/xl-multi-column/src/test/commands/__snapshots__/moveBlocks.test.ts.snap +++ b/packages/xl-multi-column/src/test/commands/__snapshots__/moveBlocks.test.ts.snap @@ -1,5 +1,183 @@ // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html +exports[`Move past empty sibling within a column > Move down below empty sibling 1`] = ` +[ + { + "children": [ + { + "children": [ + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Text 0", + "type": "text", + }, + ], + "id": "text-0", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + { + "children": [], + "content": [], + "id": "empty-0", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + ], + "content": undefined, + "id": "column-empty-0", + "props": { + "width": 1, + }, + "type": "column", + }, + { + "children": [ + { + "children": [], + "content": [], + "id": "empty-1", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Text 1", + "type": "text", + }, + ], + "id": "text-1", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + ], + "content": undefined, + "id": "column-empty-1", + "props": { + "width": 1, + }, + "type": "column", + }, + ], + "content": undefined, + "id": "column-list-empty", + "props": {}, + "type": "columnList", + }, +] +`; + +exports[`Move past empty sibling within a column > Move up above empty sibling 1`] = ` +[ + { + "children": [ + { + "children": [ + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Text 0", + "type": "text", + }, + ], + "id": "text-0", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + { + "children": [], + "content": [], + "id": "empty-0", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + ], + "content": undefined, + "id": "column-empty-0", + "props": { + "width": 1, + }, + "type": "column", + }, + { + "children": [ + { + "children": [], + "content": [], + "id": "empty-1", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Text 1", + "type": "text", + }, + ], + "id": "text-1", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + ], + "content": undefined, + "id": "column-empty-1", + "props": { + "width": 1, + }, + "type": "column", + }, + ], + "content": undefined, + "id": "column-list-empty", + "props": {}, + "type": "columnList", + }, +] +`; + exports[`Test moveBlocksDown > Move into column list 1`] = ` [ { diff --git a/packages/xl-multi-column/src/test/commands/moveBlocks.test.ts b/packages/xl-multi-column/src/test/commands/moveBlocks.test.ts index 156895be44..6970c6c036 100644 --- a/packages/xl-multi-column/src/test/commands/moveBlocks.test.ts +++ b/packages/xl-multi-column/src/test/commands/moveBlocks.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it } from "vite-plus/test"; +import { beforeEach, describe, expect, it } from "vite-plus/test"; import { setupTestEnv } from "../setupTestEnv.js"; @@ -151,3 +151,48 @@ describe("Test moveBlocksDown", () => { expect(getEditor().document).toMatchSnapshot(); }); }); + +describe("Move past empty sibling within a column", () => { + beforeEach(() => { + getEditor().replaceBlocks(getEditor().document, [ + { + id: "column-list-empty", + type: "columnList", + children: [ + { + id: "column-empty-0", + type: "column", + children: [ + { id: "empty-0", type: "paragraph" }, + { id: "text-0", type: "paragraph", content: "Text 0" }, + ], + }, + { + id: "column-empty-1", + type: "column", + children: [ + { id: "empty-1", type: "paragraph" }, + { id: "text-1", type: "paragraph", content: "Text 1" }, + ], + }, + ], + }, + ]); + }); + + it("Move up above empty sibling", () => { + getEditor().setTextCursorPosition("text-0"); + + expect(() => getEditor().moveBlocksUp()).not.toThrow(); + + expect(getEditor().document).toMatchSnapshot(); + }); + + it("Move down below empty sibling", () => { + getEditor().setTextCursorPosition("empty-0"); + + expect(() => getEditor().moveBlocksDown()).not.toThrow(); + + expect(getEditor().document).toMatchSnapshot(); + }); +});