Skip to content

fix: Deleting last block in column #2110

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
+2,047 −229
Open

fix: Deleting last block in column #2110

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
bd6519b
Fixed deleting last block in column not collapsing column/column list
matthewlipski Oct 15, 2025
226020d
Fixed and added tests
matthewlipski Oct 15, 2025
b96e9e5
Added one more test
matthewlipski Oct 15, 2025
aabf534
Added logic for collapsing `columns`/`columnList` nodes when entire c…
matthewlipski Oct 16, 2025
a521b8f
Implemented PR feedback
matthewlipski Oct 16, 2025
ff199d9
feat: Clean up invalid multi-column handling (#2113)
matthewlipski Oct 27, 2025
4ff328f
Implemented PR feedback
matthewlipski Oct 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
178 changes: 171 additions & 7 deletions packages/core/src/api/blockManipulation/commands/replaceBlocks/replaceBlocks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,165 @@
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,
BlockSchema,
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(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 </column><column> 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,
Expand Down Expand Up @@ -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);
Expand All @@ -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");

Expand Down
141 changes: 7 additions & 134 deletions packages/core/src/extensions/KeyboardShortcuts/KeyboardShortcutsExtension.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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";
Expand All @@ -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<any, any, any>;
Expand Down Expand Up @@ -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 </column><column> 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;
Expand Down
Loading
Loading