From bd6519bfb90f9c614820dc6e63bf92b4d040c5c7 Mon Sep 17 00:00:00 2001 From: Matthew Lipski Date: Wed, 15 Oct 2025 18:21:35 +0200 Subject: [PATCH 1/7] Fixed deleting last block in column not collapsing column/column list --- .../commands/replaceBlocks/replaceBlocks.ts | 178 +++++++++++++++++- .../KeyboardShortcutsExtension.ts | 141 +------------- 2 files changed, 178 insertions(+), 141 deletions(-) diff --git a/packages/core/src/api/blockManipulation/commands/replaceBlocks/replaceBlocks.ts b/packages/core/src/api/blockManipulation/commands/replaceBlocks/replaceBlocks.ts index 4ba8107636..3b80c15f9d 100644 --- a/packages/core/src/api/blockManipulation/commands/replaceBlocks/replaceBlocks.ts +++ b/packages/core/src/api/blockManipulation/commands/replaceBlocks/replaceBlocks.ts @@ -1,5 +1,6 @@ -import type { Node } from "prosemirror-model"; -import type { Transaction } from "prosemirror-state"; +import { ResolvedPos, Slice, type Node } from "prosemirror-model"; +import { TextSelection, type Transaction } from "prosemirror-state"; +import { ReplaceAroundStep } from "prosemirror-transform"; import type { Block, PartialBlock } from "../../../../blocks/defaultBlocks.js"; import type { BlockIdentifier, @@ -7,9 +8,158 @@ import type { InlineContentSchema, StyleSchema, } from "../../../../schema/index.js"; +import { getBlockInfoFromResolvedPos } from "../../../getBlockInfoFromPos.js"; import { blockToNode } from "../../../nodeConversions/blockToNode.js"; import { nodeToBlock } from "../../../nodeConversions/nodeToBlock.js"; import { getPmSchema } from "../../../pmUtil.js"; +import { + getParentBlockInfo, + getPrevBlockInfo, +} from "../mergeBlocks/mergeBlocks.js"; + +// TODO: Where should this function go? +/** + * Moves the first block in a column to the previous/next column and handles + * all necessary collapsing of `column`/`columnList` nodes. Only moves the + * block to the start of the next column if it's in the first column. + * Otherwise, moves the block to the end of the previous column. + * @param tr The transaction to apply changes to. + * @param blockBeforePos The position just before the first block in the column. + * @returns The position just before the block, after it's moved. + */ +export function moveFirstBlockInColumn( + tr: Transaction, + blockBeforePos: ResolvedPos, +): ResolvedPos { + const blockInfo = getBlockInfoFromResolvedPos(blockBeforePos); + if (!blockInfo.isBlockContainer) { + throw new Error( + "Invalid blockBeforePos passed to moveFirstBlockInColumn: does not point to blockContainer node.", + ); + } + + const prevBlockInfo = getPrevBlockInfo(tr.doc, blockInfo.bnBlock.beforePos); + if (prevBlockInfo) { + throw new Error( + "Invalid blockBeforePos passed to moveFirstBlockInColumn: does not point to first blockContainer node in column.", + ); + } + + const parentBlockInfo = getParentBlockInfo( + tr.doc, + blockInfo.bnBlock.beforePos, + ); + if (parentBlockInfo?.blockNoteType !== "column") { + throw new Error( + "Invalid blockBeforePos passed to moveFirstBlockInColumn: blockContainer node is not child of column.", + ); + } + + const column = parentBlockInfo; + const columnList = getParentBlockInfo(tr.doc, column.bnBlock.beforePos); + if (columnList?.blockNoteType !== "columnList") { + throw new Error( + "Invalid blockBeforePos passed to moveFirstBlockInColumn: blockContainer node is child of column, but column is not child of columnList node.", + ); + } + + const shouldRemoveColumn = column.childContainer!.node.childCount === 1; + + const shouldRemoveColumnList = + shouldRemoveColumn && columnList.childContainer!.node.childCount === 2; + + const isFirstColumn = + columnList.childContainer!.node.firstChild === column.bnBlock.node; + + const blockToMove = tr.doc.slice( + blockInfo.bnBlock.beforePos, + blockInfo.bnBlock.afterPos, + false, + ); + + /* + There are 3 different cases: + a) remove entire column list (if no columns would be remaining) + b) remove just a column (if no blocks inside a column would be remaining) + c) keep columns (if there are blocks remaining inside a column) + + Each of these 3 cases has 2 sub-cases, depending on whether the backspace happens at the start of the first (most-left) column, + or at the start of a non-first column. + */ + if (shouldRemoveColumnList) { + if (isFirstColumn) { + tr.step( + new ReplaceAroundStep( + // replace entire column list + columnList.bnBlock.beforePos, + columnList.bnBlock.afterPos, + // select content of remaining column: + column.bnBlock.afterPos + 1, + columnList.bnBlock.afterPos - 2, + blockToMove, + blockToMove.size, // append existing content to blockToMove + false, + ), + ); + const pos = tr.doc.resolve(column.bnBlock.beforePos); + tr.setSelection(TextSelection.between(pos, pos)); + + return pos; + } else { + // replaces the column list with the blockToMove slice, prepended with the content of the remaining column + tr.step( + new ReplaceAroundStep( + // replace entire column list + columnList.bnBlock.beforePos, + columnList.bnBlock.afterPos, + // select content of existing column: + columnList.bnBlock.beforePos + 2, + column.bnBlock.beforePos - 1, + blockToMove, + 0, // prepend existing content to blockToMove + false, + ), + ); + const pos = tr.doc.resolve(tr.mapping.map(column.bnBlock.beforePos - 1)); + tr.setSelection(TextSelection.between(pos, pos)); + + return pos; + } + } else if (shouldRemoveColumn) { + if (isFirstColumn) { + // delete column + tr.delete(column.bnBlock.beforePos, column.bnBlock.afterPos); + + // move before columnlist + tr.insert(columnList.bnBlock.beforePos, blockToMove.content); + + const pos = tr.doc.resolve(columnList.bnBlock.beforePos); + tr.setSelection(TextSelection.between(pos, pos)); + + return pos; + } else { + // just delete the closing and opening tags to merge the columns + tr.delete(column.bnBlock.beforePos - 1, column.bnBlock.beforePos + 1); + const pos = tr.doc.resolve(column.bnBlock.beforePos - 1); + + return pos; + } + } else { + // delete block + tr.delete(blockInfo.bnBlock.beforePos, blockInfo.bnBlock.afterPos); + if (isFirstColumn) { + // move before columnlist + tr.insert(columnList.bnBlock.beforePos - 1, blockToMove.content); + } else { + // append block to previous column + tr.insert(column.bnBlock.beforePos - 1, blockToMove.content); + } + const pos = tr.doc.resolve(column.bnBlock.beforePos - 1); + tr.setSelection(TextSelection.between(pos, pos)); + + return pos; + } +} export function removeAndInsertBlocks< BSchema extends BlockSchema, @@ -70,15 +220,29 @@ export function removeAndInsertBlocks< } const oldDocSize = tr.doc.nodeSize; - // Checks if the block is the only child of its parent. In this case, we - // need to delete the parent `blockGroup` node instead of just the - // `blockContainer`. + const $pos = tr.doc.resolve(pos - removedSize); - if ( + if ($pos.node().type.name === "column" && $pos.node().childCount === 1) { + // Checks if the block is the only child of a parent `column` node. In + // this case, we need to collapse the `column` or parent `columnList`, + // depending on if the `columnList` has more than 2 children. This is + // handled by `moveFirstBlockInColumn`. + const $newPos = moveFirstBlockInColumn(tr, $pos); + // Instead of deleting it, `moveFirstBlockInColumn` moves the block in + // order to handle the columns after, so we have to delete it manually. + tr.replace( + $newPos.pos, + $newPos.pos + $newPos.nodeAfter!.nodeSize, + Slice.empty, + ); + } else if ( $pos.node().type.name === "blockGroup" && $pos.node($pos.depth - 1).type.name !== "doc" && $pos.node().childCount === 1 ) { + // Checks if the block is the only child of a parent `blockGroup` node. + // In this case, we need to delete the parent `blockGroup` node instead + // of just the `blockContainer`. tr.delete($pos.before(), $pos.after()); } else { tr.delete(pos - removedSize, pos - removedSize + node.nodeSize); @@ -89,7 +253,7 @@ export function removeAndInsertBlocks< return false; }); - // Throws an error if now all blocks could be found. + // Throws an error if not all blocks could be found. if (idsOfBlocksToRemove.size > 0) { const notFoundIds = [...idsOfBlocksToRemove].join("\n"); diff --git a/packages/core/src/extensions/KeyboardShortcuts/KeyboardShortcutsExtension.ts b/packages/core/src/extensions/KeyboardShortcuts/KeyboardShortcutsExtension.ts index 94230143db..a7180afed3 100644 --- a/packages/core/src/extensions/KeyboardShortcuts/KeyboardShortcutsExtension.ts +++ b/packages/core/src/extensions/KeyboardShortcuts/KeyboardShortcutsExtension.ts @@ -1,10 +1,8 @@ import { Extension } from "@tiptap/core"; import { TextSelection } from "prosemirror-state"; -import { ReplaceAroundStep } from "prosemirror-transform"; import { getBottomNestedBlockInfo, - getParentBlockInfo, getPrevBlockInfo, mergeBlocksCommand, } from "../../api/blockManipulation/commands/mergeBlocks/mergeBlocks.js"; @@ -13,6 +11,7 @@ import { splitBlockCommand } from "../../api/blockManipulation/commands/splitBlo import { updateBlockCommand } from "../../api/blockManipulation/commands/updateBlock/updateBlock.js"; import { getBlockInfoFromSelection } from "../../api/getBlockInfoFromPos.js"; import { BlockNoteEditor } from "../../editor/BlockNoteEditor.js"; +import { moveFirstBlockInColumn } from "../../api/blockManipulation/commands/replaceBlocks/replaceBlocks.js"; export const KeyboardShortcutsExtension = Extension.create<{ editor: BlockNoteEditor; @@ -107,151 +106,25 @@ export const KeyboardShortcutsExtension = Extension.create<{ const selectionAtBlockStart = state.selection.from === blockInfo.blockContent.beforePos + 1; - if (!selectionAtBlockStart) { return false; } - const prevBlockInfo = getPrevBlockInfo( - state.doc, - blockInfo.bnBlock.beforePos, - ); + const $pos = state.doc.resolve(blockInfo.bnBlock.beforePos); - if (prevBlockInfo) { + const prevBlock = $pos.nodeBefore; + if (prevBlock) { // should be no previous block return false; } - const parentBlockInfo = getParentBlockInfo( - state.doc, - blockInfo.bnBlock.beforePos, - ); - - if (parentBlockInfo?.blockNoteType !== "column") { + const parentBlock = $pos.node(); + if (parentBlock.type.name !== "column") { return false; } - const column = parentBlockInfo; - - const columnList = getParentBlockInfo( - state.doc, - column.bnBlock.beforePos, - ); - if (columnList?.blockNoteType !== "columnList") { - throw new Error("parent of column is not a column list"); - } - - const shouldRemoveColumn = - column.childContainer!.node.childCount === 1; - - const shouldRemoveColumnList = - shouldRemoveColumn && - columnList.childContainer!.node.childCount === 2; - - const isFirstColumn = - columnList.childContainer!.node.firstChild === - column.bnBlock.node; - if (dispatch) { - const blockToMove = state.doc.slice( - blockInfo.bnBlock.beforePos, - blockInfo.bnBlock.afterPos, - false, - ); - - /* - There are 3 different cases: - a) remove entire column list (if no columns would be remaining) - b) remove just a column (if no blocks inside a column would be remaining) - c) keep columns (if there are blocks remaining inside a column) - - Each of these 3 cases has 2 sub-cases, depending on whether the backspace happens at the start of the first (most-left) column, - or at the start of a non-first column. - */ - if (shouldRemoveColumnList) { - if (isFirstColumn) { - state.tr.step( - new ReplaceAroundStep( - // replace entire column list - columnList.bnBlock.beforePos, - columnList.bnBlock.afterPos, - // select content of remaining column: - column.bnBlock.afterPos + 1, - columnList.bnBlock.afterPos - 2, - blockToMove, - blockToMove.size, // append existing content to blockToMove - false, - ), - ); - const pos = state.tr.doc.resolve(column.bnBlock.beforePos); - state.tr.setSelection(TextSelection.between(pos, pos)); - } else { - // replaces the column list with the blockToMove slice, prepended with the content of the remaining column - state.tr.step( - new ReplaceAroundStep( - // replace entire column list - columnList.bnBlock.beforePos, - columnList.bnBlock.afterPos, - // select content of existing column: - columnList.bnBlock.beforePos + 2, - column.bnBlock.beforePos - 1, - blockToMove, - 0, // prepend existing content to blockToMove - false, - ), - ); - const pos = state.tr.doc.resolve( - state.tr.mapping.map(column.bnBlock.beforePos - 1), - ); - state.tr.setSelection(TextSelection.between(pos, pos)); - } - } else if (shouldRemoveColumn) { - if (isFirstColumn) { - // delete column - state.tr.delete( - column.bnBlock.beforePos, - column.bnBlock.afterPos, - ); - - // move before columnlist - state.tr.insert( - columnList.bnBlock.beforePos, - blockToMove.content, - ); - - const pos = state.tr.doc.resolve( - columnList.bnBlock.beforePos, - ); - state.tr.setSelection(TextSelection.between(pos, pos)); - } else { - // just delete the closing and opening tags to merge the columns - state.tr.delete( - column.bnBlock.beforePos - 1, - column.bnBlock.beforePos + 1, - ); - } - } else { - // delete block - state.tr.delete( - blockInfo.bnBlock.beforePos, - blockInfo.bnBlock.afterPos, - ); - if (isFirstColumn) { - // move before columnlist - state.tr.insert( - columnList.bnBlock.beforePos - 1, - blockToMove.content, - ); - } else { - // append block to previous column - state.tr.insert( - column.bnBlock.beforePos - 1, - blockToMove.content, - ); - } - const pos = state.tr.doc.resolve(column.bnBlock.beforePos - 1); - state.tr.setSelection(TextSelection.between(pos, pos)); - } + moveFirstBlockInColumn(state.tr, $pos); } return true; From 226020db2581afeec647d2edd5b6ce9646f512e9 Mon Sep 17 00:00:00 2001 From: Matthew Lipski Date: Wed, 15 Oct 2025 19:29:06 +0200 Subject: [PATCH 2/7] Fixed and added tests --- .../commands/replaceBlocks/replaceBlocks.ts | 2 +- .../__snapshots__/removeBlocks.test.ts.snap | 166 +++++++++++------- .../__snapshots__/replaceBlocks.test.ts.snap | 102 +++++++++++ .../src/test/commands/removeBlocks.test.ts | 11 ++ .../src/test/commands/replaceBlocks.test.ts | 19 ++ 5 files changed, 239 insertions(+), 61 deletions(-) diff --git a/packages/core/src/api/blockManipulation/commands/replaceBlocks/replaceBlocks.ts b/packages/core/src/api/blockManipulation/commands/replaceBlocks/replaceBlocks.ts index 3b80c15f9d..c8063425ec 100644 --- a/packages/core/src/api/blockManipulation/commands/replaceBlocks/replaceBlocks.ts +++ b/packages/core/src/api/blockManipulation/commands/replaceBlocks/replaceBlocks.ts @@ -101,7 +101,7 @@ export function moveFirstBlockInColumn( false, ), ); - const pos = tr.doc.resolve(column.bnBlock.beforePos); + const pos = tr.doc.resolve(columnList.bnBlock.beforePos); tr.setSelection(TextSelection.between(pos, pos)); return pos; diff --git a/packages/xl-multi-column/src/test/commands/__snapshots__/removeBlocks.test.ts.snap b/packages/xl-multi-column/src/test/commands/__snapshots__/removeBlocks.test.ts.snap index f6902f31bc..764aba94b0 100644 --- a/packages/xl-multi-column/src/test/commands/__snapshots__/removeBlocks.test.ts.snap +++ b/packages/xl-multi-column/src/test/commands/__snapshots__/removeBlocks.test.ts.snap @@ -54,78 +54,124 @@ exports[`Test removeBlocks > Remove all blocks in column 1`] = ` }, "type": "paragraph", }, + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Column Paragraph 2", + "type": "text", + }, + ], + "id": "column-paragraph-2", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Column Paragraph 3", + "type": "text", + }, + ], + "id": "column-paragraph-3", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Paragraph 2", + "type": "text", + }, + ], + "id": "paragraph-2", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + { + "children": [], + "content": [], + "id": "trailing-paragraph", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, +] +`; + +exports[`Test removeBlocks > Remove all blocks in columns 1`] = ` +[ { "children": [ { - "children": [ + "children": [], + "content": [ { - "children": [], - "content": [], - "id": "0", - "props": { - "backgroundColor": "default", - "textAlignment": "left", - "textColor": "default", - }, - "type": "paragraph", + "styles": {}, + "text": "Nested Paragraph 0", + "type": "text", }, ], - "content": undefined, - "id": "column-0", + "id": "nested-paragraph-0", "props": { - "width": 1, + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", }, - "type": "column", + "type": "paragraph", }, + ], + "content": [ { - "children": [ - { - "children": [], - "content": [ - { - "styles": {}, - "text": "Column Paragraph 2", - "type": "text", - }, - ], - "id": "column-paragraph-2", - "props": { - "backgroundColor": "default", - "textAlignment": "left", - "textColor": "default", - }, - "type": "paragraph", - }, - { - "children": [], - "content": [ - { - "styles": {}, - "text": "Column Paragraph 3", - "type": "text", - }, - ], - "id": "column-paragraph-3", - "props": { - "backgroundColor": "default", - "textAlignment": "left", - "textColor": "default", - }, - "type": "paragraph", - }, - ], - "content": undefined, - "id": "column-1", - "props": { - "width": 1, - }, - "type": "column", + "styles": {}, + "text": "Paragraph 0", + "type": "text", }, ], - "content": undefined, - "id": "column-list-0", - "props": {}, - "type": "columnList", + "id": "paragraph-0", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Paragraph 1", + "type": "text", + }, + ], + "id": "paragraph-1", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", }, { "children": [], diff --git a/packages/xl-multi-column/src/test/commands/__snapshots__/replaceBlocks.test.ts.snap b/packages/xl-multi-column/src/test/commands/__snapshots__/replaceBlocks.test.ts.snap index 94782491fe..8c6818447d 100644 --- a/packages/xl-multi-column/src/test/commands/__snapshots__/replaceBlocks.test.ts.snap +++ b/packages/xl-multi-column/src/test/commands/__snapshots__/replaceBlocks.test.ts.snap @@ -1,5 +1,107 @@ // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html +exports[`Test replaceBlocks > Replace all blocks in columns with single block 1`] = ` +[ + { + "children": [ + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Nested Paragraph 0", + "type": "text", + }, + ], + "id": "nested-paragraph-0", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + ], + "content": [ + { + "styles": {}, + "text": "Paragraph 0", + "type": "text", + }, + ], + "id": "paragraph-0", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Paragraph 1", + "type": "text", + }, + ], + "id": "paragraph-1", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + { + "children": [], + "content": [ + { + "styles": {}, + "text": "New Paragraph", + "type": "text", + }, + ], + "id": "0", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Paragraph 2", + "type": "text", + }, + ], + "id": "paragraph-2", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + { + "children": [], + "content": [], + "id": "trailing-paragraph", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, +] +`; + exports[`Test replaceBlocks > Replace paragraph with column list above column list 1`] = ` [ { 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 2e5e4977e7..8ae0504392 100644 --- a/packages/xl-multi-column/src/test/commands/removeBlocks.test.ts +++ b/packages/xl-multi-column/src/test/commands/removeBlocks.test.ts @@ -11,6 +11,17 @@ describe("Test removeBlocks", () => { expect(getEditor().document).toMatchSnapshot(); }); + it("Remove all blocks in columns", () => { + getEditor().removeBlocks([ + "column-paragraph-0", + "column-paragraph-1", + "column-paragraph-2", + "column-paragraph-3", + ]); + + expect(getEditor().document).toMatchSnapshot(); + }); + it("Remove all columns in columnList", () => { getEditor().removeBlocks(["column-0", "column-1"]); diff --git a/packages/xl-multi-column/src/test/commands/replaceBlocks.test.ts b/packages/xl-multi-column/src/test/commands/replaceBlocks.test.ts index 6839873dd5..706d7ec347 100644 --- a/packages/xl-multi-column/src/test/commands/replaceBlocks.test.ts +++ b/packages/xl-multi-column/src/test/commands/replaceBlocks.test.ts @@ -37,4 +37,23 @@ describe("Test replaceBlocks", () => { expect(getEditor().document).toMatchSnapshot(); }); + + it("Replace all blocks in columns with single block", () => { + getEditor().replaceBlocks( + [ + "column-paragraph-0", + "column-paragraph-1", + "column-paragraph-2", + "column-paragraph-3", + ], + [ + { + type: "paragraph", + content: "New Paragraph", + }, + ], + ); + + expect(getEditor().document).toMatchSnapshot(); + }); }); From b96e9e56f3fa2f21024081acd4c352177a94b5a2 Mon Sep 17 00:00:00 2001 From: Matthew Lipski Date: Wed, 15 Oct 2025 19:30:27 +0200 Subject: [PATCH 3/7] Added one more test --- .../__snapshots__/removeBlocks.test.ts.snap | 119 ++++++++++++++++++ .../src/test/commands/removeBlocks.test.ts | 6 + 2 files changed, 125 insertions(+) diff --git a/packages/xl-multi-column/src/test/commands/__snapshots__/removeBlocks.test.ts.snap b/packages/xl-multi-column/src/test/commands/__snapshots__/removeBlocks.test.ts.snap index 764aba94b0..9daa30a606 100644 --- a/packages/xl-multi-column/src/test/commands/__snapshots__/removeBlocks.test.ts.snap +++ b/packages/xl-multi-column/src/test/commands/__snapshots__/removeBlocks.test.ts.snap @@ -204,6 +204,125 @@ exports[`Test removeBlocks > Remove all blocks in columns 1`] = ` ] `; +exports[`Test removeBlocks > Remove all blocks in second column 1`] = ` +[ + { + "children": [ + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Nested Paragraph 0", + "type": "text", + }, + ], + "id": "nested-paragraph-0", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + ], + "content": [ + { + "styles": {}, + "text": "Paragraph 0", + "type": "text", + }, + ], + "id": "paragraph-0", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Paragraph 1", + "type": "text", + }, + ], + "id": "paragraph-1", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Column Paragraph 0", + "type": "text", + }, + ], + "id": "column-paragraph-0", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Column Paragraph 1", + "type": "text", + }, + ], + "id": "column-paragraph-1", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Paragraph 2", + "type": "text", + }, + ], + "id": "paragraph-2", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + { + "children": [], + "content": [], + "id": "trailing-paragraph", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, +] +`; + exports[`Test removeBlocks > Remove all columns in columnList 1`] = ` [ { 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 8ae0504392..227cce609e 100644 --- a/packages/xl-multi-column/src/test/commands/removeBlocks.test.ts +++ b/packages/xl-multi-column/src/test/commands/removeBlocks.test.ts @@ -11,6 +11,12 @@ describe("Test removeBlocks", () => { expect(getEditor().document).toMatchSnapshot(); }); + it("Remove all blocks in second column", () => { + getEditor().removeBlocks(["column-paragraph-2", "column-paragraph-3"]); + + expect(getEditor().document).toMatchSnapshot(); + }); + it("Remove all blocks in columns", () => { getEditor().removeBlocks([ "column-paragraph-0", From aabf534783782717d8e3fce38a9ac4d746f045bb Mon Sep 17 00:00:00 2001 From: Matthew Lipski Date: Thu, 16 Oct 2025 12:34:05 +0200 Subject: [PATCH 4/7] Added logic for collapsing `columns`/`columnList` nodes when entire columns are removed + added more tests --- .../commands/replaceBlocks/replaceBlocks.ts | 69 +- .../__snapshots__/removeBlocks.test.ts.snap | 589 ++++++++++++++++-- .../src/test/commands/removeBlocks.test.ts | 46 ++ 3 files changed, 664 insertions(+), 40 deletions(-) diff --git a/packages/core/src/api/blockManipulation/commands/replaceBlocks/replaceBlocks.ts b/packages/core/src/api/blockManipulation/commands/replaceBlocks/replaceBlocks.ts index c8063425ec..6c2f957409 100644 --- a/packages/core/src/api/blockManipulation/commands/replaceBlocks/replaceBlocks.ts +++ b/packages/core/src/api/blockManipulation/commands/replaceBlocks/replaceBlocks.ts @@ -1,4 +1,4 @@ -import { ResolvedPos, Slice, type Node } from "prosemirror-model"; +import { Fragment, ResolvedPos, Slice, type Node } from "prosemirror-model"; import { TextSelection, type Transaction } from "prosemirror-state"; import { ReplaceAroundStep } from "prosemirror-transform"; import type { Block, PartialBlock } from "../../../../blocks/defaultBlocks.js"; @@ -235,6 +235,73 @@ export function removeAndInsertBlocks< $newPos.pos + $newPos.nodeAfter!.nodeSize, Slice.empty, ); + } else if ( + $pos.node().type.name === "columnList" && + $pos.node().childCount === 2 + ) { + // Checks whether removing the entire column would leave only a single + // remaining `column` node in the columnList. In this case, we need to + // collapse the column list. + const column = getBlockInfoFromResolvedPos($pos); + if (column.blockNoteType !== "column") { + throw new Error( + `Block of type ${column.blockNoteType} was found as child of columnList.`, + ); + } + const columnList = getParentBlockInfo(tr.doc, column.bnBlock.beforePos); + if (!columnList) { + throw new Error( + `Block of type column was found without a parent columnList.`, + ); + } + if (columnList?.blockNoteType !== "columnList") { + throw new Error( + `Block of type ${columnList.blockNoteType} was found as a parent of column.`, + ); + } + + if ($pos.node().childCount === 1) { + tr.replaceWith( + columnList.bnBlock.beforePos, + columnList.bnBlock.afterPos, + Fragment.empty, + ); + } + + tr.replaceWith( + columnList.bnBlock.beforePos, + columnList.bnBlock.afterPos, + $pos.index() === 0 + ? columnList.bnBlock.node.lastChild!.content + : columnList.bnBlock.node.firstChild!.content, + ); + } else if ( + node.type.name === "column" && + node.attrs.id !== $pos.nodeAfter?.attrs.id + ) { + // This is a hacky work around to handle an edge case with the previous + // `if else` block. When each `column` of a `columnList` is in the + // `blocksToRemove` array, this is what happens once all but the last 2 + // columns are removed: + // + // 1. The second-to-last `column` is removed. + // 2. The last `column` and wrapping `columnList` are collapsed. + // 3. `removedSize` increases by the size of the removed column, and more + // due to positions at the starts/ends of the last `column` and wrapping + // `columnList` also getting removed. + // 3. `tr.doc.descendants` traverses to the last `column`. + // 4. `removedSize` now includes positions that were removed after the + // last `column`. In order for `pos - removedSize` to correctly point to + // the start of the nodes that were previously wrapped by the last + // `column`, `removedPos` must only include positions removed before it. + // 5. The deletion is offset by 3, because of those removed positions + // included in `removedSize` that occur after the last `column`. + // + // Hence why we have to shift the start of the deletion range back by 3. + // The offset for the end of the range is smaller as `node.nodeSize` is + // the size of the whole second `column`, whereas now we are left with + // just its children since it's collapsed - a difference of 2 positions. + tr.delete(pos - removedSize + 3, pos - removedSize + node.nodeSize + 1); } else if ( $pos.node().type.name === "blockGroup" && $pos.node($pos.depth - 1).type.name !== "doc" && diff --git a/packages/xl-multi-column/src/test/commands/__snapshots__/removeBlocks.test.ts.snap b/packages/xl-multi-column/src/test/commands/__snapshots__/removeBlocks.test.ts.snap index 9daa30a606..e67cdf0be4 100644 --- a/packages/xl-multi-column/src/test/commands/__snapshots__/removeBlocks.test.ts.snap +++ b/packages/xl-multi-column/src/test/commands/__snapshots__/removeBlocks.test.ts.snap @@ -119,6 +119,108 @@ exports[`Test removeBlocks > Remove all blocks in column 1`] = ` ] `; +exports[`Test removeBlocks > Remove all blocks in column and block after 1`] = ` +[ + { + "children": [ + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Nested Paragraph 0", + "type": "text", + }, + ], + "id": "nested-paragraph-0", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + ], + "content": [ + { + "styles": {}, + "text": "Paragraph 0", + "type": "text", + }, + ], + "id": "paragraph-0", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Paragraph 1", + "type": "text", + }, + ], + "id": "paragraph-1", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Column Paragraph 2", + "type": "text", + }, + ], + "id": "column-paragraph-2", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Column Paragraph 3", + "type": "text", + }, + ], + "id": "column-paragraph-3", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + { + "children": [], + "content": [], + "id": "trailing-paragraph", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, +] +`; + exports[`Test removeBlocks > Remove all blocks in columns 1`] = ` [ { @@ -204,6 +306,74 @@ exports[`Test removeBlocks > Remove all blocks in columns 1`] = ` ] `; +exports[`Test removeBlocks > Remove all blocks in columns and block after 1`] = ` +[ + { + "children": [ + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Nested Paragraph 0", + "type": "text", + }, + ], + "id": "nested-paragraph-0", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + ], + "content": [ + { + "styles": {}, + "text": "Paragraph 0", + "type": "text", + }, + ], + "id": "paragraph-0", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Paragraph 1", + "type": "text", + }, + ], + "id": "paragraph-1", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + { + "children": [], + "content": [], + "id": "trailing-paragraph", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, +] +`; + exports[`Test removeBlocks > Remove all blocks in second column 1`] = ` [ { @@ -377,66 +547,407 @@ exports[`Test removeBlocks > Remove all columns in columnList 1`] = ` }, "type": "paragraph", }, + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Paragraph 2", + "type": "text", + }, + ], + "id": "paragraph-2", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + { + "children": [], + "content": [], + "id": "trailing-paragraph", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, +] +`; + +exports[`Test removeBlocks > Remove all columns in columnList and block after 1`] = ` +[ { "children": [ { - "children": [ + "children": [], + "content": [ { - "children": [], - "content": [], - "id": "1", - "props": { - "backgroundColor": "default", - "textAlignment": "left", - "textColor": "default", - }, - "type": "paragraph", + "styles": {}, + "text": "Nested Paragraph 0", + "type": "text", }, ], - "content": undefined, - "id": "0", + "id": "nested-paragraph-0", "props": { - "width": 1, + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", }, - "type": "column", + "type": "paragraph", }, + ], + "content": [ { - "children": [ - { - "children": [], - "content": [], - "id": "3", - "props": { - "backgroundColor": "default", - "textAlignment": "left", - "textColor": "default", - }, - "type": "paragraph", - }, - ], - "content": undefined, - "id": "2", - "props": { - "width": 1, - }, - "type": "column", + "styles": {}, + "text": "Paragraph 0", + "type": "text", }, ], - "content": undefined, - "id": "column-list-0", - "props": {}, - "type": "columnList", + "id": "paragraph-0", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", }, { "children": [], "content": [ { "styles": {}, - "text": "Paragraph 2", + "text": "Paragraph 1", "type": "text", }, ], - "id": "paragraph-2", + "id": "paragraph-1", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + { + "children": [], + "content": [], + "id": "trailing-paragraph", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, +] +`; + +exports[`Test removeBlocks > Remove column and and block in column after 1`] = ` +[ + { + "children": [ + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Nested Paragraph 0", + "type": "text", + }, + ], + "id": "nested-paragraph-0", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + ], + "content": [ + { + "styles": {}, + "text": "Paragraph 0", + "type": "text", + }, + ], + "id": "paragraph-0", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Paragraph 1", + "type": "text", + }, + ], + "id": "paragraph-1", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Column Paragraph 3", + "type": "text", + }, + ], + "id": "column-paragraph-3", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Paragraph 2", + "type": "text", + }, + ], + "id": "paragraph-2", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + { + "children": [], + "content": [], + "id": "trailing-paragraph", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, +] +`; + +exports[`Test removeBlocks > Remove column in columnList 1`] = ` +[ + { + "children": [ + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Nested Paragraph 0", + "type": "text", + }, + ], + "id": "nested-paragraph-0", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + ], + "content": [ + { + "styles": {}, + "text": "Paragraph 0", + "type": "text", + }, + ], + "id": "paragraph-0", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Paragraph 1", + "type": "text", + }, + ], + "id": "paragraph-1", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Column Paragraph 2", + "type": "text", + }, + ], + "id": "column-paragraph-2", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Column Paragraph 3", + "type": "text", + }, + ], + "id": "column-paragraph-3", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Paragraph 2", + "type": "text", + }, + ], + "id": "paragraph-2", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + { + "children": [], + "content": [], + "id": "trailing-paragraph", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, +] +`; + +exports[`Test removeBlocks > Remove column in columnList and block after 1`] = ` +[ + { + "children": [ + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Nested Paragraph 0", + "type": "text", + }, + ], + "id": "nested-paragraph-0", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + ], + "content": [ + { + "styles": {}, + "text": "Paragraph 0", + "type": "text", + }, + ], + "id": "paragraph-0", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Paragraph 1", + "type": "text", + }, + ], + "id": "paragraph-1", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Column Paragraph 2", + "type": "text", + }, + ], + "id": "column-paragraph-2", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Column Paragraph 3", + "type": "text", + }, + ], + "id": "column-paragraph-3", "props": { "backgroundColor": "default", "textAlignment": "left", 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 227cce609e..6fb58e9306 100644 --- a/packages/xl-multi-column/src/test/commands/removeBlocks.test.ts +++ b/packages/xl-multi-column/src/test/commands/removeBlocks.test.ts @@ -28,9 +28,55 @@ describe("Test removeBlocks", () => { expect(getEditor().document).toMatchSnapshot(); }); + it("Remove column in columnList", () => { + getEditor().removeBlocks(["column-0"]); + + expect(getEditor().document).toMatchSnapshot(); + }); + it("Remove all columns in columnList", () => { getEditor().removeBlocks(["column-0", "column-1"]); expect(getEditor().document).toMatchSnapshot(); }); + + it("Remove all blocks in column and block after", () => { + getEditor().removeBlocks([ + "column-paragraph-0", + "column-paragraph-1", + "paragraph-2", + ]); + + expect(getEditor().document).toMatchSnapshot(); + }); + + it("Remove all blocks in columns and block after", () => { + getEditor().removeBlocks([ + "column-paragraph-0", + "column-paragraph-1", + "column-paragraph-2", + "column-paragraph-3", + "paragraph-2", + ]); + + expect(getEditor().document).toMatchSnapshot(); + }); + + it("Remove column in columnList and block after", () => { + getEditor().removeBlocks(["column-0", "paragraph-2"]); + + expect(getEditor().document).toMatchSnapshot(); + }); + + it("Remove all columns in columnList and block after", () => { + getEditor().removeBlocks(["column-0", "column-1", "paragraph-2"]); + + expect(getEditor().document).toMatchSnapshot(); + }); + + it("Remove column and and block in column after", () => { + getEditor().removeBlocks(["column-0", "column-paragraph-2"]); + + expect(getEditor().document).toMatchSnapshot(); + }); }); From a521b8f649e652515b400b8cf7ba795ccbd3bd82 Mon Sep 17 00:00:00 2001 From: Matthew Lipski Date: Thu, 16 Oct 2025 12:49:23 +0200 Subject: [PATCH 5/7] Implemented PR feedback --- .../commands/replaceBlocks/replaceBlocks.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/core/src/api/blockManipulation/commands/replaceBlocks/replaceBlocks.ts b/packages/core/src/api/blockManipulation/commands/replaceBlocks/replaceBlocks.ts index 6c2f957409..d41a20230f 100644 --- a/packages/core/src/api/blockManipulation/commands/replaceBlocks/replaceBlocks.ts +++ b/packages/core/src/api/blockManipulation/commands/replaceBlocks/replaceBlocks.ts @@ -34,14 +34,14 @@ export function moveFirstBlockInColumn( const blockInfo = getBlockInfoFromResolvedPos(blockBeforePos); if (!blockInfo.isBlockContainer) { throw new Error( - "Invalid blockBeforePos passed to moveFirstBlockInColumn: does not point to blockContainer node.", + "Invalid blockBeforePos: does not point to blockContainer node.", ); } const prevBlockInfo = getPrevBlockInfo(tr.doc, blockInfo.bnBlock.beforePos); if (prevBlockInfo) { throw new Error( - "Invalid blockBeforePos passed to moveFirstBlockInColumn: does not point to first blockContainer node in column.", + "Invalid blockBeforePos: does not point to first blockContainer node in column.", ); } @@ -51,7 +51,7 @@ export function moveFirstBlockInColumn( ); if (parentBlockInfo?.blockNoteType !== "column") { throw new Error( - "Invalid blockBeforePos passed to moveFirstBlockInColumn: blockContainer node is not child of column.", + "Invalid blockBeforePos: blockContainer node is not child of column.", ); } @@ -59,7 +59,7 @@ export function moveFirstBlockInColumn( const columnList = getParentBlockInfo(tr.doc, column.bnBlock.beforePos); if (columnList?.blockNoteType !== "columnList") { throw new Error( - "Invalid blockBeforePos passed to moveFirstBlockInColumn: blockContainer node is child of column, but column is not child of columnList node.", + "Invalid blockBeforePos: blockContainer node is child of column, but column is not child of columnList node.", ); } @@ -245,18 +245,18 @@ export function removeAndInsertBlocks< const column = getBlockInfoFromResolvedPos($pos); if (column.blockNoteType !== "column") { throw new Error( - `Block of type ${column.blockNoteType} was found as child of columnList.`, + `Invalid block: ${column.blockNoteType} was found as child of columnList.`, ); } const columnList = getParentBlockInfo(tr.doc, column.bnBlock.beforePos); if (!columnList) { throw new Error( - `Block of type column was found without a parent columnList.`, + `Invalid block: column was found without a parent columnList.`, ); } if (columnList?.blockNoteType !== "columnList") { throw new Error( - `Block of type ${columnList.blockNoteType} was found as a parent of column.`, + `Invalid block: ${columnList.blockNoteType} was found as a parent of column.`, ); } From ff199d9ee097e4c5be88c9ac1cf0775368f60ad9 Mon Sep 17 00:00:00 2001 From: Matthew Lipski <50169049+matthewlipski@users.noreply.github.com> Date: Mon, 27 Oct 2025 13:58:02 +0100 Subject: [PATCH 6/7] feat: Clean up invalid multi-column handling (#2113) * Added `fixColumnList` function * Updated keyboard shortcut * Implemented PR feedback --- .../commands/replaceBlocks/replaceBlocks.ts | 243 ++--------- .../replaceBlocks/util/fixColumnList.ts | 163 +++++++ .../KeyboardShortcutsExtension.ts | 39 +- packages/core/src/index.ts | 1 + .../__snapshots__/fixColumnLists.test.ts.snap | 408 ++++++++++++++++++ .../test/commands/util/fixColumnLists.test.ts | 283 ++++++++++++ 6 files changed, 913 insertions(+), 224 deletions(-) create mode 100644 packages/core/src/api/blockManipulation/commands/replaceBlocks/util/fixColumnList.ts create mode 100644 packages/xl-multi-column/src/test/commands/util/__snapshots__/fixColumnLists.test.ts.snap create mode 100644 packages/xl-multi-column/src/test/commands/util/fixColumnLists.test.ts diff --git a/packages/core/src/api/blockManipulation/commands/replaceBlocks/replaceBlocks.ts b/packages/core/src/api/blockManipulation/commands/replaceBlocks/replaceBlocks.ts index d41a20230f..579c4d8c6b 100644 --- a/packages/core/src/api/blockManipulation/commands/replaceBlocks/replaceBlocks.ts +++ b/packages/core/src/api/blockManipulation/commands/replaceBlocks/replaceBlocks.ts @@ -1,6 +1,5 @@ -import { Fragment, ResolvedPos, Slice, type Node } from "prosemirror-model"; -import { TextSelection, type Transaction } from "prosemirror-state"; -import { ReplaceAroundStep } from "prosemirror-transform"; +import { type Node } from "prosemirror-model"; +import { type Transaction } from "prosemirror-state"; import type { Block, PartialBlock } from "../../../../blocks/defaultBlocks.js"; import type { BlockIdentifier, @@ -8,158 +7,10 @@ import type { InlineContentSchema, StyleSchema, } from "../../../../schema/index.js"; -import { getBlockInfoFromResolvedPos } from "../../../getBlockInfoFromPos.js"; import { blockToNode } from "../../../nodeConversions/blockToNode.js"; import { nodeToBlock } from "../../../nodeConversions/nodeToBlock.js"; import { getPmSchema } from "../../../pmUtil.js"; -import { - getParentBlockInfo, - getPrevBlockInfo, -} from "../mergeBlocks/mergeBlocks.js"; - -// TODO: Where should this function go? -/** - * Moves the first block in a column to the previous/next column and handles - * all necessary collapsing of `column`/`columnList` nodes. Only moves the - * block to the start of the next column if it's in the first column. - * Otherwise, moves the block to the end of the previous column. - * @param tr The transaction to apply changes to. - * @param blockBeforePos The position just before the first block in the column. - * @returns The position just before the block, after it's moved. - */ -export function moveFirstBlockInColumn( - tr: Transaction, - blockBeforePos: ResolvedPos, -): ResolvedPos { - const blockInfo = getBlockInfoFromResolvedPos(blockBeforePos); - if (!blockInfo.isBlockContainer) { - throw new Error( - "Invalid blockBeforePos: does not point to blockContainer node.", - ); - } - - const prevBlockInfo = getPrevBlockInfo(tr.doc, blockInfo.bnBlock.beforePos); - if (prevBlockInfo) { - throw new Error( - "Invalid blockBeforePos: does not point to first blockContainer node in column.", - ); - } - - const parentBlockInfo = getParentBlockInfo( - tr.doc, - blockInfo.bnBlock.beforePos, - ); - if (parentBlockInfo?.blockNoteType !== "column") { - throw new Error( - "Invalid blockBeforePos: blockContainer node is not child of column.", - ); - } - - const column = parentBlockInfo; - const columnList = getParentBlockInfo(tr.doc, column.bnBlock.beforePos); - if (columnList?.blockNoteType !== "columnList") { - throw new Error( - "Invalid blockBeforePos: blockContainer node is child of column, but column is not child of columnList node.", - ); - } - - const shouldRemoveColumn = column.childContainer!.node.childCount === 1; - - const shouldRemoveColumnList = - shouldRemoveColumn && columnList.childContainer!.node.childCount === 2; - - const isFirstColumn = - columnList.childContainer!.node.firstChild === column.bnBlock.node; - - const blockToMove = tr.doc.slice( - blockInfo.bnBlock.beforePos, - blockInfo.bnBlock.afterPos, - false, - ); - - /* - There are 3 different cases: - a) remove entire column list (if no columns would be remaining) - b) remove just a column (if no blocks inside a column would be remaining) - c) keep columns (if there are blocks remaining inside a column) - - Each of these 3 cases has 2 sub-cases, depending on whether the backspace happens at the start of the first (most-left) column, - or at the start of a non-first column. - */ - if (shouldRemoveColumnList) { - if (isFirstColumn) { - tr.step( - new ReplaceAroundStep( - // replace entire column list - columnList.bnBlock.beforePos, - columnList.bnBlock.afterPos, - // select content of remaining column: - column.bnBlock.afterPos + 1, - columnList.bnBlock.afterPos - 2, - blockToMove, - blockToMove.size, // append existing content to blockToMove - false, - ), - ); - const pos = tr.doc.resolve(columnList.bnBlock.beforePos); - tr.setSelection(TextSelection.between(pos, pos)); - - return pos; - } else { - // replaces the column list with the blockToMove slice, prepended with the content of the remaining column - tr.step( - new ReplaceAroundStep( - // replace entire column list - columnList.bnBlock.beforePos, - columnList.bnBlock.afterPos, - // select content of existing column: - columnList.bnBlock.beforePos + 2, - column.bnBlock.beforePos - 1, - blockToMove, - 0, // prepend existing content to blockToMove - false, - ), - ); - const pos = tr.doc.resolve(tr.mapping.map(column.bnBlock.beforePos - 1)); - tr.setSelection(TextSelection.between(pos, pos)); - - return pos; - } - } else if (shouldRemoveColumn) { - if (isFirstColumn) { - // delete column - tr.delete(column.bnBlock.beforePos, column.bnBlock.afterPos); - - // move before columnlist - tr.insert(columnList.bnBlock.beforePos, blockToMove.content); - - const pos = tr.doc.resolve(columnList.bnBlock.beforePos); - tr.setSelection(TextSelection.between(pos, pos)); - - return pos; - } else { - // just delete the closing and opening tags to merge the columns - tr.delete(column.bnBlock.beforePos - 1, column.bnBlock.beforePos + 1); - const pos = tr.doc.resolve(column.bnBlock.beforePos - 1); - - return pos; - } - } else { - // delete block - tr.delete(blockInfo.bnBlock.beforePos, blockInfo.bnBlock.afterPos); - if (isFirstColumn) { - // move before columnlist - tr.insert(columnList.bnBlock.beforePos - 1, blockToMove.content); - } else { - // append block to previous column - tr.insert(column.bnBlock.beforePos - 1, blockToMove.content); - } - const pos = tr.doc.resolve(column.bnBlock.beforePos - 1); - tr.setSelection(TextSelection.between(pos, pos)); - - return pos; - } -} +import { fixColumnList } from "./util/fixColumnList.js"; export function removeAndInsertBlocks< BSchema extends BlockSchema, @@ -222,85 +73,32 @@ export function removeAndInsertBlocks< const oldDocSize = tr.doc.nodeSize; const $pos = tr.doc.resolve(pos - removedSize); - if ($pos.node().type.name === "column" && $pos.node().childCount === 1) { - // Checks if the block is the only child of a parent `column` node. In - // this case, we need to collapse the `column` or parent `columnList`, - // depending on if the `columnList` has more than 2 children. This is - // handled by `moveFirstBlockInColumn`. - const $newPos = moveFirstBlockInColumn(tr, $pos); - // Instead of deleting it, `moveFirstBlockInColumn` moves the block in - // order to handle the columns after, so we have to delete it manually. - tr.replace( - $newPos.pos, - $newPos.pos + $newPos.nodeAfter!.nodeSize, - Slice.empty, - ); - } else if ( - $pos.node().type.name === "columnList" && - $pos.node().childCount === 2 - ) { - // Checks whether removing the entire column would leave only a single - // remaining `column` node in the columnList. In this case, we need to - // collapse the column list. - const column = getBlockInfoFromResolvedPos($pos); - if (column.blockNoteType !== "column") { - throw new Error( - `Invalid block: ${column.blockNoteType} was found as child of columnList.`, - ); - } - const columnList = getParentBlockInfo(tr.doc, column.bnBlock.beforePos); - if (!columnList) { - throw new Error( - `Invalid block: column was found without a parent columnList.`, - ); - } - if (columnList?.blockNoteType !== "columnList") { - throw new Error( - `Invalid block: ${columnList.blockNoteType} was found as a parent of column.`, - ); - } - - if ($pos.node().childCount === 1) { - tr.replaceWith( - columnList.bnBlock.beforePos, - columnList.bnBlock.afterPos, - Fragment.empty, - ); - } - - tr.replaceWith( - columnList.bnBlock.beforePos, - columnList.bnBlock.afterPos, - $pos.index() === 0 - ? columnList.bnBlock.node.lastChild!.content - : columnList.bnBlock.node.firstChild!.content, - ); - } else if ( + if ( node.type.name === "column" && node.attrs.id !== $pos.nodeAfter?.attrs.id ) { - // This is a hacky work around to handle an edge case with the previous - // `if else` block. When each `column` of a `columnList` is in the - // `blocksToRemove` array, this is what happens once all but the last 2 - // columns are removed: + // This is a hacky work around to handle removing all columns in a + // columnList. This is what happens when removing the last 2 columns: // // 1. The second-to-last `column` is removed. - // 2. The last `column` and wrapping `columnList` are collapsed. - // 3. `removedSize` increases by the size of the removed column, and more - // due to positions at the starts/ends of the last `column` and wrapping - // `columnList` also getting removed. + // 2. `fixColumnList` runs, removing the `columnList` and inserting the + // contents of the last column in its place. + // 3. `removedSize` increases not just by the size of the second-to-last + // `column`, but also by the positions removed due to running + // `fixColumnList`. Some of these positions are after the contents of the + // last `column`, namely just after the `column` and `columnList`. // 3. `tr.doc.descendants` traverses to the last `column`. // 4. `removedSize` now includes positions that were removed after the - // last `column`. In order for `pos - removedSize` to correctly point to - // the start of the nodes that were previously wrapped by the last - // `column`, `removedPos` must only include positions removed before it. + // last `column`. This causes `pos - removedSize` to point to an + // incorrect position, as it expects that the difference in document size + // accounted for by `removedSize` comes before the block being removed. // 5. The deletion is offset by 3, because of those removed positions // included in `removedSize` that occur after the last `column`. // // Hence why we have to shift the start of the deletion range back by 3. // The offset for the end of the range is smaller as `node.nodeSize` is - // the size of the whole second `column`, whereas now we are left with - // just its children since it's collapsed - a difference of 2 positions. + // the size of the second `column`. Since it's been removed, we actually + // care about the size of its children - a difference of 2 positions. tr.delete(pos - removedSize + 3, pos - removedSize + node.nodeSize + 1); } else if ( $pos.node().type.name === "blockGroup" && @@ -314,6 +112,13 @@ export function removeAndInsertBlocks< } else { tr.delete(pos - removedSize, pos - removedSize + node.nodeSize); } + + if ($pos.node().type.name === "column") { + fixColumnList(tr, $pos.before(-1)); + } else if ($pos.node().type.name === "columnList") { + fixColumnList(tr, $pos.before()); + } + const newDocSize = tr.doc.nodeSize; removedSize += oldDocSize - newDocSize; diff --git a/packages/core/src/api/blockManipulation/commands/replaceBlocks/util/fixColumnList.ts b/packages/core/src/api/blockManipulation/commands/replaceBlocks/util/fixColumnList.ts new file mode 100644 index 0000000000..adaf3d7f69 --- /dev/null +++ b/packages/core/src/api/blockManipulation/commands/replaceBlocks/util/fixColumnList.ts @@ -0,0 +1,163 @@ +import { Slice, type Node } from "prosemirror-model"; +import { type Transaction } from "prosemirror-state"; +import { ReplaceAroundStep } from "prosemirror-transform"; + +/** + * Checks if a `column` node is empty, i.e. if it has only a single empty + * block. + * @param column The column to check. + * @returns Whether the column is empty. + */ +export function isEmptyColumn(column: Node) { + if (!column || column.type.name !== "column") { + throw new Error("Invalid columnPos: does not point to column node."); + } + + const blockContainer = column.firstChild; + if (!blockContainer) { + throw new Error("Invalid column: does not have child node."); + } + + const blockContent = blockContainer.firstChild; + if (!blockContent) { + throw new Error("Invalid blockContainer: does not have child node."); + } + + return ( + column.childCount === 1 && + blockContainer.childCount === 1 && + blockContent.type.spec.content === "inline*" && + blockContent.content.content.length === 0 + ); +} + +/** + * Removes all empty `column` nodes in a `columnList`. A `column` node is empty + * if it has only a single empty block. If, however, removing the `column`s + * leaves the `columnList` that has fewer than two, ProseMirror will re-add + * empty columns. + * @param tr The `Transaction` to add the changes to. + * @param columnListPos The position just before the `columnList` node. + */ +export function removeEmptyColumns(tr: Transaction, columnListPos: number) { + const $columnListPos = tr.doc.resolve(columnListPos); + const columnList = $columnListPos.nodeAfter; + if (!columnList || columnList.type.name !== "columnList") { + throw new Error( + "Invalid columnListPos: does not point to columnList node.", + ); + } + + for ( + let columnIndex = columnList.childCount - 1; + columnIndex >= 0; + columnIndex-- + ) { + const columnPos = tr.doc + .resolve($columnListPos.pos + 1) + .posAtIndex(columnIndex); + const $columnPos = tr.doc.resolve(columnPos); + const column = $columnPos.nodeAfter; + if (!column || column.type.name !== "column") { + throw new Error("Invalid columnPos: does not point to column node."); + } + + if (isEmptyColumn(column)) { + tr.delete(columnPos, columnPos + column?.nodeSize); + } + } +} + +/** + * Fixes potential issues in a `columnList` node after a + * `blockContainer`/`column` node is (re)moved from it: + * + * - Removes all empty `column` nodes. A `column` node is empty if it has only + * a single empty block. + * - If all but one `column` nodes are empty, replaces the `columnList` with + * the content of the non-empty `column`. + * - If all `column` nodes are empty, removes the `columnList` entirely. + * @param tr The `Transaction` to add the changes to. + * @param columnListPos + * @returns The position just before the `columnList` node. + */ +export function fixColumnList(tr: Transaction, columnListPos: number) { + removeEmptyColumns(tr, columnListPos); + + const $columnListPos = tr.doc.resolve(columnListPos); + const columnList = $columnListPos.nodeAfter; + if (!columnList || columnList.type.name !== "columnList") { + throw new Error( + "Invalid columnListPos: does not point to columnList node.", + ); + } + + if (columnList.childCount > 2) { + // Do nothing if the `columnList` has at least two non-empty `column`s. + return; + } + + if (columnList.childCount < 2) { + throw new Error("Invalid columnList: contains fewer than two children."); + } + + const firstColumnBeforePos = columnListPos + 1; + const $firstColumnBeforePos = tr.doc.resolve(firstColumnBeforePos); + const firstColumn = $firstColumnBeforePos.nodeAfter; + + const lastColumnAfterPos = columnListPos + columnList.nodeSize - 1; + const $lastColumnAfterPos = tr.doc.resolve(lastColumnAfterPos); + const lastColumn = $lastColumnAfterPos.nodeBefore; + + if (!firstColumn || !lastColumn) { + throw new Error("Invalid columnList: does not contain children."); + } + + const firstColumnEmpty = isEmptyColumn(firstColumn); + const lastColumnEmpty = isEmptyColumn(lastColumn); + + if (firstColumnEmpty && lastColumnEmpty) { + // Removes `columnList` + tr.delete(columnListPos, columnListPos + columnList.nodeSize); + + return; + } + + if (firstColumnEmpty) { + tr.step( + new ReplaceAroundStep( + // Replaces `columnList`. + columnListPos, + columnListPos + columnList.nodeSize, + // Replaces with content of last `column`. + lastColumnAfterPos - lastColumn.nodeSize + 1, + lastColumnAfterPos - 1, + // Doesn't append anything. + Slice.empty, + 0, + false, + ), + ); + + return; + } + + if (lastColumnEmpty) { + tr.step( + new ReplaceAroundStep( + // Replaces `columnList`. + columnListPos, + columnListPos + columnList.nodeSize, + // Replaces with content of first `column`. + firstColumnBeforePos + 1, + firstColumnBeforePos + firstColumn.nodeSize - 1, + // Doesn't append anything. + Slice.empty, + 0, + false, + ), + ); + + return; + } +} diff --git a/packages/core/src/extensions/KeyboardShortcuts/KeyboardShortcutsExtension.ts b/packages/core/src/extensions/KeyboardShortcuts/KeyboardShortcutsExtension.ts index a7180afed3..da63452923 100644 --- a/packages/core/src/extensions/KeyboardShortcuts/KeyboardShortcutsExtension.ts +++ b/packages/core/src/extensions/KeyboardShortcuts/KeyboardShortcutsExtension.ts @@ -11,7 +11,7 @@ import { splitBlockCommand } from "../../api/blockManipulation/commands/splitBlo import { updateBlockCommand } from "../../api/blockManipulation/commands/updateBlock/updateBlock.js"; import { getBlockInfoFromSelection } from "../../api/getBlockInfoFromPos.js"; import { BlockNoteEditor } from "../../editor/BlockNoteEditor.js"; -import { moveFirstBlockInColumn } from "../../api/blockManipulation/commands/replaceBlocks/replaceBlocks.js"; +import { fixColumnList } from "../../api/blockManipulation/commands/replaceBlocks/util/fixColumnList.js"; export const KeyboardShortcutsExtension = Extension.create<{ editor: BlockNoteEditor; @@ -97,7 +97,7 @@ export const KeyboardShortcutsExtension = Extension.create<{ return false; }), () => - commands.command(({ state, dispatch }) => { + commands.command(({ state, tr, dispatch }) => { // when at the start of a first block in a column const blockInfo = getBlockInfoFromSelection(state); if (!blockInfo.isBlockContainer) { @@ -105,12 +105,12 @@ export const KeyboardShortcutsExtension = Extension.create<{ } const selectionAtBlockStart = - state.selection.from === blockInfo.blockContent.beforePos + 1; + tr.selection.from === blockInfo.blockContent.beforePos + 1; if (!selectionAtBlockStart) { return false; } - const $pos = state.doc.resolve(blockInfo.bnBlock.beforePos); + const $pos = tr.doc.resolve(blockInfo.bnBlock.beforePos); const prevBlock = $pos.nodeBefore; if (prevBlock) { @@ -123,8 +123,37 @@ export const KeyboardShortcutsExtension = Extension.create<{ return false; } + const $blockPos = tr.doc.resolve(blockInfo.bnBlock.beforePos); + const $columnPos = tr.doc.resolve($blockPos.before()); + const columnListPos = $columnPos.before(); + if (dispatch) { - moveFirstBlockInColumn(state.tr, $pos); + const fragment = tr.doc.slice( + blockInfo.bnBlock.beforePos, + blockInfo.bnBlock.afterPos, + ).content; + + tr.delete( + blockInfo.bnBlock.beforePos, + blockInfo.bnBlock.afterPos, + ); + + if ($columnPos.index() === 0) { + // Fix `columnList` and insert the block before it. + fixColumnList(tr, columnListPos); + tr.insert(columnListPos, fragment); + tr.setSelection( + TextSelection.near(tr.doc.resolve(columnListPos)), + ); + } else { + // Insert the block at the end of the first column and fix + // `columnList`. + tr.insert($columnPos.pos - 1, fragment); + tr.setSelection( + TextSelection.near(tr.doc.resolve($columnPos.pos - 1)), + ); + fixColumnList(tr, columnListPos); + } } return true; diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 11fe0e5460..4b6994d5d4 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -1,6 +1,7 @@ export * from "./api/blockManipulation/commands/insertBlocks/insertBlocks.js"; export * from "./api/blockManipulation/commands/replaceBlocks/replaceBlocks.js"; export * from "./api/blockManipulation/commands/updateBlock/updateBlock.js"; +export * from "./api/blockManipulation/commands/replaceBlocks/util/fixColumnList.js"; export * from "./api/exporters/html/externalHTMLExporter.js"; export * from "./api/exporters/html/internalHTMLSerializer.js"; export * from "./api/getBlockInfoFromPos.js"; diff --git a/packages/xl-multi-column/src/test/commands/util/__snapshots__/fixColumnLists.test.ts.snap b/packages/xl-multi-column/src/test/commands/util/__snapshots__/fixColumnLists.test.ts.snap new file mode 100644 index 0000000000..87b5f2e588 --- /dev/null +++ b/packages/xl-multi-column/src/test/commands/util/__snapshots__/fixColumnLists.test.ts.snap @@ -0,0 +1,408 @@ +// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html + +exports[`Test fixColumnList > First of two columns empty 1`] = ` +{ + "content": [ + { + "content": [ + { + "attrs": { + "id": null, + }, + "content": [ + { + "attrs": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "content": [ + { + "text": "Paragraph 1", + "type": "text", + }, + ], + "type": "paragraph", + }, + ], + "type": "blockContainer", + }, + ], + "type": "blockGroup", + }, + ], + "type": "doc", +} +`; + +exports[`Test fixColumnList > Last of two columns empty 1`] = ` +{ + "content": [ + { + "content": [ + { + "attrs": { + "id": null, + }, + "content": [ + { + "attrs": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "content": [ + { + "text": "Paragraph 1", + "type": "text", + }, + ], + "type": "paragraph", + }, + ], + "type": "blockContainer", + }, + ], + "type": "blockGroup", + }, + ], + "type": "doc", +} +`; + +exports[`Test fixColumnList > Two empty columns 1`] = ` +{ + "content": [ + { + "content": [ + { + "attrs": { + "id": null, + }, + "content": [ + { + "attrs": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + ], + "type": "blockContainer", + }, + ], + "type": "blockGroup", + }, + ], + "type": "doc", +} +`; + +exports[`Test removeEmptyColumns > First of two columns empty 1`] = ` +{ + "content": [ + { + "content": [ + { + "attrs": { + "id": null, + }, + "content": [ + { + "attrs": { + "id": null, + "width": 1, + }, + "content": [ + { + "attrs": { + "id": null, + }, + "content": [ + { + "attrs": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + ], + "type": "blockContainer", + }, + ], + "type": "column", + }, + { + "attrs": { + "id": null, + "width": 1, + }, + "content": [ + { + "attrs": { + "id": null, + }, + "content": [ + { + "attrs": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "content": [ + { + "text": "Paragraph 1", + "type": "text", + }, + ], + "type": "paragraph", + }, + ], + "type": "blockContainer", + }, + ], + "type": "column", + }, + ], + "type": "columnList", + }, + ], + "type": "blockGroup", + }, + ], + "type": "doc", +} +`; + +exports[`Test removeEmptyColumns > Last of two columns empty 1`] = ` +{ + "content": [ + { + "content": [ + { + "attrs": { + "id": null, + }, + "content": [ + { + "attrs": { + "id": null, + "width": 1, + }, + "content": [ + { + "attrs": { + "id": null, + }, + "content": [ + { + "attrs": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "content": [ + { + "text": "Paragraph 1", + "type": "text", + }, + ], + "type": "paragraph", + }, + ], + "type": "blockContainer", + }, + ], + "type": "column", + }, + { + "attrs": { + "id": null, + "width": 1, + }, + "content": [ + { + "attrs": { + "id": null, + }, + "content": [ + { + "attrs": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + ], + "type": "blockContainer", + }, + ], + "type": "column", + }, + ], + "type": "columnList", + }, + ], + "type": "blockGroup", + }, + ], + "type": "doc", +} +`; + +exports[`Test removeEmptyColumns > Start and end columns empty 1`] = ` +{ + "content": [ + { + "content": [ + { + "attrs": { + "id": null, + }, + "content": [ + { + "attrs": { + "id": null, + "width": 1, + }, + "content": [ + { + "attrs": { + "id": null, + }, + "content": [ + { + "attrs": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "content": [ + { + "text": "Paragraph 1", + "type": "text", + }, + ], + "type": "paragraph", + }, + ], + "type": "blockContainer", + }, + ], + "type": "column", + }, + { + "attrs": { + "id": null, + "width": 1, + }, + "content": [ + { + "attrs": { + "id": null, + }, + "content": [ + { + "attrs": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "content": [ + { + "text": "Paragraph 2", + "type": "text", + }, + ], + "type": "paragraph", + }, + ], + "type": "blockContainer", + }, + ], + "type": "column", + }, + ], + "type": "columnList", + }, + ], + "type": "blockGroup", + }, + ], + "type": "doc", +} +`; + +exports[`Test removeEmptyColumns > Two empty columns 1`] = ` +{ + "content": [ + { + "content": [ + { + "attrs": { + "id": null, + }, + "content": [ + { + "attrs": { + "id": null, + "width": 1, + }, + "content": [ + { + "attrs": { + "id": null, + }, + "content": [ + { + "attrs": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + ], + "type": "blockContainer", + }, + ], + "type": "column", + }, + { + "attrs": { + "id": null, + "width": 1, + }, + "content": [ + { + "attrs": { + "id": null, + }, + "content": [ + { + "attrs": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + ], + "type": "blockContainer", + }, + ], + "type": "column", + }, + ], + "type": "columnList", + }, + ], + "type": "blockGroup", + }, + ], + "type": "doc", +} +`; diff --git a/packages/xl-multi-column/src/test/commands/util/fixColumnLists.test.ts b/packages/xl-multi-column/src/test/commands/util/fixColumnLists.test.ts new file mode 100644 index 0000000000..5212030057 --- /dev/null +++ b/packages/xl-multi-column/src/test/commands/util/fixColumnLists.test.ts @@ -0,0 +1,283 @@ +import { describe, expect, it } from "vitest"; + +import { setupTestEnv } from "../../setupTestEnv.js"; +import { + fixColumnList, + isEmptyColumn, + removeEmptyColumns, +} from "@blocknote/core"; + +const getEditor = setupTestEnv(); + +describe("Test isEmptyColumn", () => { + it("Empty blocks", () => { + const schema = getEditor()._tiptapEditor.schema; + + const column = schema.nodes["column"].create(undefined, [ + schema.nodes["blockContainer"].create(undefined, [ + schema.nodes["paragraph"].create(), + ]), + ]); + + expect(isEmptyColumn(column)).toBeTruthy(); + }); + + it("Multiple blocks", () => { + const schema = getEditor()._tiptapEditor.schema; + + const column = schema.nodes["column"].create(undefined, [ + schema.nodes["blockContainer"].create(undefined, [ + schema.nodes["paragraph"].create(undefined), + ]), + schema.nodes["blockContainer"].create(undefined, [ + schema.nodes["paragraph"].create(), + ]), + ]); + + expect(isEmptyColumn(column)).toBeFalsy(); + }); + + it("Block with children", () => { + const schema = getEditor()._tiptapEditor.schema; + + const column = schema.nodes["column"].create(undefined, [ + schema.nodes["blockContainer"].create(undefined, [ + schema.nodes["paragraph"].create(undefined), + schema.nodes["blockGroup"].create(undefined, [ + schema.nodes["blockContainer"].create(undefined, [ + schema.nodes["paragraph"].create(), + ]), + ]), + ]), + ]); + + expect(isEmptyColumn(column)).toBeFalsy(); + }); + + it("Block with text", () => { + const schema = getEditor()._tiptapEditor.schema; + + const column = schema.nodes["column"].create(undefined, [ + schema.nodes["blockContainer"].create(undefined, [ + schema.nodes["paragraph"].create(undefined, [ + schema.text("Paragraph 1"), + ]), + ]), + ]); + + expect(isEmptyColumn(column)).toBeFalsy(); + }); + + it("Non-text block", () => { + const schema = getEditor()._tiptapEditor.schema; + + const column = schema.nodes["column"].create(undefined, [ + schema.nodes["blockContainer"].create(undefined, [ + schema.nodes["image"].create(), + ]), + ]); + + expect(isEmptyColumn(column)).toBeFalsy(); + }); +}); + +describe("Test removeEmptyColumns", () => { + it("Start and end columns empty", () => { + const editor = getEditor(); + const schema = editor._tiptapEditor.schema; + + const columnList = schema.nodes["columnList"].create(undefined, [ + schema.nodes["column"].create(undefined, [ + schema.nodes["blockContainer"].create(undefined, [ + schema.nodes["paragraph"].create(), + ]), + ]), + schema.nodes["column"].create(undefined, [ + schema.nodes["blockContainer"].create(undefined, [ + schema.nodes["paragraph"].create(undefined, [ + schema.text("Paragraph 1"), + ]), + ]), + ]), + schema.nodes["column"].create(undefined, [ + schema.nodes["blockContainer"].create(undefined, [ + schema.nodes["paragraph"].create(undefined, [ + schema.text("Paragraph 2"), + ]), + ]), + ]), + schema.nodes["column"].create(undefined, [ + schema.nodes["blockContainer"].create(undefined, [ + schema.nodes["paragraph"].create(), + ]), + ]), + ]); + + const tr = editor.prosemirrorState.tr; + + tr.replaceRangeWith(1, tr.doc.firstChild!.content.size, columnList); + removeEmptyColumns(tr, 1); + + expect(tr.doc).toMatchSnapshot(); + }); + + it("First of two columns empty", () => { + const editor = getEditor(); + const schema = editor._tiptapEditor.schema; + + const columnList = schema.nodes["columnList"].create(undefined, [ + schema.nodes["column"].create(undefined, [ + schema.nodes["blockContainer"].create(undefined, [ + schema.nodes["paragraph"].create(), + ]), + ]), + schema.nodes["column"].create(undefined, [ + schema.nodes["blockContainer"].create(undefined, [ + schema.nodes["paragraph"].create(undefined, [ + schema.text("Paragraph 1"), + ]), + ]), + ]), + ]); + + const tr = editor.prosemirrorState.tr; + + tr.replaceRangeWith(1, tr.doc.firstChild!.content.size, columnList); + removeEmptyColumns(tr, 1); + + expect(tr.doc).toMatchSnapshot(); + }); + + it("Last of two columns empty", () => { + const editor = getEditor(); + const schema = editor._tiptapEditor.schema; + + const columnList = schema.nodes["columnList"].create(undefined, [ + schema.nodes["column"].create(undefined, [ + schema.nodes["blockContainer"].create(undefined, [ + schema.nodes["paragraph"].create(undefined, [ + schema.text("Paragraph 1"), + ]), + ]), + ]), + schema.nodes["column"].create(undefined, [ + schema.nodes["blockContainer"].create(undefined, [ + schema.nodes["paragraph"].create(), + ]), + ]), + ]); + + const tr = editor.prosemirrorState.tr; + + tr.replaceRangeWith(1, tr.doc.firstChild!.content.size, columnList); + removeEmptyColumns(tr, 1); + + expect(tr.doc).toMatchSnapshot(); + }); + + it("Two empty columns", () => { + const editor = getEditor(); + const schema = editor._tiptapEditor.schema; + + const columnList = schema.nodes["columnList"].create(undefined, [ + schema.nodes["column"].create(undefined, [ + schema.nodes["blockContainer"].create(undefined, [ + schema.nodes["paragraph"].create(), + ]), + ]), + schema.nodes["column"].create(undefined, [ + schema.nodes["blockContainer"].create(undefined, [ + schema.nodes["paragraph"].create(), + ]), + ]), + ]); + + const tr = editor.prosemirrorState.tr; + + tr.replaceRangeWith(1, tr.doc.firstChild!.content.size, columnList); + removeEmptyColumns(tr, 1); + + expect(tr.doc).toMatchSnapshot(); + }); +}); + +describe("Test fixColumnList", () => { + it("First of two columns empty", () => { + const editor = getEditor(); + const schema = editor._tiptapEditor.schema; + + const columnList = schema.nodes["columnList"].create(undefined, [ + schema.nodes["column"].create(undefined, [ + schema.nodes["blockContainer"].create(undefined, [ + schema.nodes["paragraph"].create(), + ]), + ]), + schema.nodes["column"].create(undefined, [ + schema.nodes["blockContainer"].create(undefined, [ + schema.nodes["paragraph"].create(undefined, [ + schema.text("Paragraph 1"), + ]), + ]), + ]), + ]); + + const tr = editor.prosemirrorState.tr; + + tr.replaceRangeWith(1, tr.doc.firstChild!.content.size, columnList); + fixColumnList(tr, 1); + + expect(tr.doc).toMatchSnapshot(); + }); + + it("Last of two columns empty", () => { + const editor = getEditor(); + const schema = editor._tiptapEditor.schema; + + const columnList = schema.nodes["columnList"].create(undefined, [ + schema.nodes["column"].create(undefined, [ + schema.nodes["blockContainer"].create(undefined, [ + schema.nodes["paragraph"].create(undefined, [ + schema.text("Paragraph 1"), + ]), + ]), + ]), + schema.nodes["column"].create(undefined, [ + schema.nodes["blockContainer"].create(undefined, [ + schema.nodes["paragraph"].create(), + ]), + ]), + ]); + + const tr = editor.prosemirrorState.tr; + + tr.replaceRangeWith(1, tr.doc.firstChild!.content.size, columnList); + fixColumnList(tr, 1); + + expect(tr.doc).toMatchSnapshot(); + }); + + it("Two empty columns", () => { + const editor = getEditor(); + const schema = editor._tiptapEditor.schema; + + const columnList = schema.nodes["columnList"].create(undefined, [ + schema.nodes["column"].create(undefined, [ + schema.nodes["blockContainer"].create(undefined, [ + schema.nodes["paragraph"].create(), + ]), + ]), + schema.nodes["column"].create(undefined, [ + schema.nodes["blockContainer"].create(undefined, [ + schema.nodes["paragraph"].create(), + ]), + ]), + ]); + + const tr = editor.prosemirrorState.tr; + + tr.replaceRangeWith(1, tr.doc.firstChild!.content.size, columnList); + fixColumnList(tr, 1); + + expect(tr.doc).toMatchSnapshot(); + }); +}); From 4ff328f65bc2da60b53548342fbeb0a723ff160c Mon Sep 17 00:00:00 2001 From: Matthew Lipski Date: Fri, 31 Oct 2025 17:50:23 +0100 Subject: [PATCH 7/7] Implemented PR feedback --- .../commands/replaceBlocks/replaceBlocks.ts | 43 ++--- .../replaceBlocks/util/fixColumnList.ts | 18 +- .../__snapshots__/replaceBlocks.test.ts.snap | 164 ++++++++++++++++++ .../src/test/commands/replaceBlocks.test.ts | 14 ++ 4 files changed, 202 insertions(+), 37 deletions(-) diff --git a/packages/core/src/api/blockManipulation/commands/replaceBlocks/replaceBlocks.ts b/packages/core/src/api/blockManipulation/commands/replaceBlocks/replaceBlocks.ts index 579c4d8c6b..04a2425a33 100644 --- a/packages/core/src/api/blockManipulation/commands/replaceBlocks/replaceBlocks.ts +++ b/packages/core/src/api/blockManipulation/commands/replaceBlocks/replaceBlocks.ts @@ -37,6 +37,7 @@ export function removeAndInsertBlocks< ), ); const removedBlocks: Block[] = []; + const columnListPositions = new Set(); const idOfFirstBlock = typeof blocksToRemove[0] === "string" @@ -73,34 +74,14 @@ export function removeAndInsertBlocks< const oldDocSize = tr.doc.nodeSize; const $pos = tr.doc.resolve(pos - removedSize); + + if ($pos.node().type.name === "column") { + columnListPositions.add($pos.before(-1)); + } else if ($pos.node().type.name === "columnList") { + columnListPositions.add($pos.before()); + } + if ( - node.type.name === "column" && - node.attrs.id !== $pos.nodeAfter?.attrs.id - ) { - // This is a hacky work around to handle removing all columns in a - // columnList. This is what happens when removing the last 2 columns: - // - // 1. The second-to-last `column` is removed. - // 2. `fixColumnList` runs, removing the `columnList` and inserting the - // contents of the last column in its place. - // 3. `removedSize` increases not just by the size of the second-to-last - // `column`, but also by the positions removed due to running - // `fixColumnList`. Some of these positions are after the contents of the - // last `column`, namely just after the `column` and `columnList`. - // 3. `tr.doc.descendants` traverses to the last `column`. - // 4. `removedSize` now includes positions that were removed after the - // last `column`. This causes `pos - removedSize` to point to an - // incorrect position, as it expects that the difference in document size - // accounted for by `removedSize` comes before the block being removed. - // 5. The deletion is offset by 3, because of those removed positions - // included in `removedSize` that occur after the last `column`. - // - // Hence why we have to shift the start of the deletion range back by 3. - // The offset for the end of the range is smaller as `node.nodeSize` is - // the size of the second `column`. Since it's been removed, we actually - // care about the size of its children - a difference of 2 positions. - tr.delete(pos - removedSize + 3, pos - removedSize + node.nodeSize + 1); - } else if ( $pos.node().type.name === "blockGroup" && $pos.node($pos.depth - 1).type.name !== "doc" && $pos.node().childCount === 1 @@ -113,12 +94,6 @@ export function removeAndInsertBlocks< tr.delete(pos - removedSize, pos - removedSize + node.nodeSize); } - if ($pos.node().type.name === "column") { - fixColumnList(tr, $pos.before(-1)); - } else if ($pos.node().type.name === "columnList") { - fixColumnList(tr, $pos.before()); - } - const newDocSize = tr.doc.nodeSize; removedSize += oldDocSize - newDocSize; @@ -135,6 +110,8 @@ export function removeAndInsertBlocks< ); } + columnListPositions.forEach((pos) => fixColumnList(tr, pos)); + // Converts the nodes created from `blocksToInsert` into full `Block`s. const insertedBlocks = nodesToInsert.map((node) => nodeToBlock(node, pmSchema), diff --git a/packages/core/src/api/blockManipulation/commands/replaceBlocks/util/fixColumnList.ts b/packages/core/src/api/blockManipulation/commands/replaceBlocks/util/fixColumnList.ts index adaf3d7f69..3097851f47 100644 --- a/packages/core/src/api/blockManipulation/commands/replaceBlocks/util/fixColumnList.ts +++ b/packages/core/src/api/blockManipulation/commands/replaceBlocks/util/fixColumnList.ts @@ -4,7 +4,7 @@ import { ReplaceAroundStep } from "prosemirror-transform"; /** * Checks if a `column` node is empty, i.e. if it has only a single empty - * block. + * paragraph. * @param column The column to check. * @returns Whether the column is empty. */ @@ -26,7 +26,7 @@ export function isEmptyColumn(column: Node) { return ( column.childCount === 1 && blockContainer.childCount === 1 && - blockContent.type.spec.content === "inline*" && + blockContent.type.name === "paragraph" && blockContent.content.content.length === 0 ); } @@ -63,7 +63,7 @@ export function removeEmptyColumns(tr: Transaction, columnListPos: number) { } if (isEmptyColumn(column)) { - tr.delete(columnPos, columnPos + column?.nodeSize); + tr.delete(columnPos, columnPos + column.nodeSize); } } } @@ -93,11 +93,21 @@ export function fixColumnList(tr: Transaction, columnListPos: number) { } if (columnList.childCount > 2) { - // Do nothing if the `columnList` has at least two non-empty `column`s. + // Do nothing if the `columnList` has more than two non-empty `column`s. In + // the case that the `columnList` has exactly two columns, we may need to + // still remove it, as it's possible that one or both columns are empty. + // This is because after `removeEmptyColumns` is called, if the + // `columnList` has fewer than two `column`s, ProseMirror will re-add empty + // `column`s until there are two total, in order to fit the schema. return; } if (columnList.childCount < 2) { + // Throw an error if the `columnList` has fewer than two columns. After + // `removeEmptyColumns` is called, if the `columnList` has fewer than two + // `column`s, ProseMirror will re-add empty `column`s until there are two + // total, in order to fit the schema. So if there are fewer than two here, + // either the schema, or ProseMirror's internals, must have changed. throw new Error("Invalid columnList: contains fewer than two children."); } diff --git a/packages/xl-multi-column/src/test/commands/__snapshots__/replaceBlocks.test.ts.snap b/packages/xl-multi-column/src/test/commands/__snapshots__/replaceBlocks.test.ts.snap index 8c6818447d..b19ccf1eec 100644 --- a/packages/xl-multi-column/src/test/commands/__snapshots__/replaceBlocks.test.ts.snap +++ b/packages/xl-multi-column/src/test/commands/__snapshots__/replaceBlocks.test.ts.snap @@ -1,5 +1,169 @@ // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html +exports[`Test replaceBlocks > Replace all blocks in column with single block 1`] = ` +[ + { + "children": [ + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Nested Paragraph 0", + "type": "text", + }, + ], + "id": "nested-paragraph-0", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + ], + "content": [ + { + "styles": {}, + "text": "Paragraph 0", + "type": "text", + }, + ], + "id": "paragraph-0", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Paragraph 1", + "type": "text", + }, + ], + "id": "paragraph-1", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + { + "children": [ + { + "children": [ + { + "children": [], + "content": [ + { + "styles": {}, + "text": "New Paragraph", + "type": "text", + }, + ], + "id": "0", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + ], + "content": undefined, + "id": "column-0", + "props": { + "width": 1, + }, + "type": "column", + }, + { + "children": [ + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Column Paragraph 2", + "type": "text", + }, + ], + "id": "column-paragraph-2", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Column Paragraph 3", + "type": "text", + }, + ], + "id": "column-paragraph-3", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + ], + "content": undefined, + "id": "column-1", + "props": { + "width": 1, + }, + "type": "column", + }, + ], + "content": undefined, + "id": "column-list-0", + "props": {}, + "type": "columnList", + }, + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Paragraph 2", + "type": "text", + }, + ], + "id": "paragraph-2", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + { + "children": [], + "content": [], + "id": "trailing-paragraph", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, +] +`; + exports[`Test replaceBlocks > Replace all blocks in columns with single block 1`] = ` [ { diff --git a/packages/xl-multi-column/src/test/commands/replaceBlocks.test.ts b/packages/xl-multi-column/src/test/commands/replaceBlocks.test.ts index 706d7ec347..5640804fc7 100644 --- a/packages/xl-multi-column/src/test/commands/replaceBlocks.test.ts +++ b/packages/xl-multi-column/src/test/commands/replaceBlocks.test.ts @@ -38,6 +38,20 @@ describe("Test replaceBlocks", () => { expect(getEditor().document).toMatchSnapshot(); }); + it("Replace all blocks in column with single block", () => { + getEditor().replaceBlocks( + ["column-paragraph-0", "column-paragraph-1"], + [ + { + type: "paragraph", + content: "New Paragraph", + }, + ], + ); + + expect(getEditor().document).toMatchSnapshot(); + }); + it("Replace all blocks in columns with single block", () => { getEditor().replaceBlocks( [