Skip to content

Commit 2f1af8f

Browse files
YousefEDclaude
andcommitted
fix: don't collapse emptied columns when moving blocks
Moves are rearrangements rather than deletions, so columns that a move empties out are deliberately left as-is instead of being collapsed. This keeps moves free of side effects (and reversible by moving back), matches dragging a block out of a column, and fixes the original error: collapsing a transiently-empty column mid-move broke the re-insertion. Also simplifies `removeAndInsertBlocks` (plain `fixColumns` opt-out, no `affectedColumnLists` return) and removes the columnList reference anchoring workaround in `moveBlocks`, which guarded against mid-move collapsing that can no longer happen. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent d0ea3cc commit 2f1af8f

3 files changed

Lines changed: 398 additions & 272 deletions

File tree

packages/core/src/api/blockManipulation/commands/moveBlocks/moveBlocks.ts

Lines changed: 9 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import { getNearestBlockPos } from "../../../getBlockInfoFromPos.js";
1313
import { getNodeById } from "../../../nodeUtil.js";
1414
import { insertBlocks } from "../insertBlocks/insertBlocks.js";
1515
import { removeAndInsertBlocks } from "../replaceBlocks/replaceBlocks.js";
16-
import { fixColumnList } from "../replaceBlocks/util/fixColumnList.js";
1716

1817
type BlockSelectionData = (
1918
| {
@@ -152,40 +151,20 @@ export function moveBlocks(
152151
placement: "before" | "after",
153152
) {
154153
editor.transact((tr) => {
155-
// A `columnList` reference can be dissolved by `fixColumnList` when its
156-
// `column`s are removed, leaving its ID invalid for re-insertion. Anchor
157-
// to an adjacent block instead, which is unaffected by the removal.
158-
const refBlock = editor.getBlock(referenceBlock);
159-
if (refBlock?.type === "columnList") {
160-
const adjacent =
161-
placement === "after"
162-
? editor.getNextBlock(refBlock)
163-
: editor.getPrevBlock(refBlock);
164-
if (adjacent) {
165-
referenceBlock = adjacent;
166-
placement = placement === "after" ? "before" : "after";
167-
}
168-
}
169-
170-
// Don't fix columns/columnLists in the removal step. Otherwise, the
171-
// following case breaks:
154+
// Don't fix columns/columnLists in the removal step. Since a move is a
155+
// rearrangement rather than a deletion, columns that it empties out are
156+
// deliberately left as-is instead of being collapsed - this keeps moves
157+
// free of side effects (and reversible by moving back), and matches
158+
// dragging a block out of a column, which doesn't collapse it either.
159+
// Fixing them mid-move also broke the following case:
172160
// <column>
173161
// <paragraph></paragraph>
174162
// <paragraph>Paragraph</paragraph>
175163
// </column>
176-
// When the non-empty block is moved up, the column is now seen as empty
177-
// and collapsed in the removal step, so the following insertion fails.
178-
const { affectedColumnLists } = removeAndInsertBlocks(tr, blocks, [], {
179-
fixColumns: false,
180-
});
164+
// When the non-empty block is moved up, the column is seen as empty and
165+
// collapsed in the removal step, so the following insertion fails.
166+
removeAndInsertBlocks(tr, blocks, [], { fixColumns: false });
181167
insertBlocks(tr, flattenColumns(blocks), referenceBlock, placement);
182-
183-
affectedColumnLists.forEach((id) => {
184-
const posInfo = getNodeById(id, tr.doc);
185-
if (posInfo) {
186-
fixColumnList(tr, posInfo.posBeforeNode);
187-
}
188-
});
189168
});
190169
}
191170

packages/core/src/api/blockManipulation/commands/replaceBlocks/replaceBlocks.ts

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ export function removeAndInsertBlocks<
2626
): {
2727
insertedBlocks: Block<BSchema, I, S>[];
2828
removedBlocks: Block<BSchema, I, S>[];
29-
affectedColumnLists: string[];
3029
} {
3130
const pmSchema = getPmSchema(tr);
3231
// Converts the `PartialBlock`s to ProseMirror nodes to insert them into the
@@ -116,17 +115,9 @@ export function removeAndInsertBlocks<
116115
);
117116
}
118117

119-
// Saves IDs of columnLists containing removed blocks. If `fixColumns` is
120-
// explicitly false, these are needed to run `fixColumnList` manually later.
121-
const affectedColumnLists: string[] = [];
122-
columnListPositions.forEach((pos) => {
123-
const columnList = tr.doc.resolve(pos).nodeAfter;
124-
if (columnList?.type.name === "columnList") {
125-
affectedColumnLists.push(columnList.attrs.id);
126-
}
127-
});
128-
129-
// Collapses empty columns/columnLists
118+
// Collapses empty columns/columnLists. Callers where the removal isn't a
119+
// deletion can opt out - e.g. `moveBlocks` re-inserts the blocks elsewhere
120+
// and deliberately leaves emptied columns as-is.
130121
if (options.fixColumns !== false) {
131122
columnListPositions.forEach((pos) => fixColumnList(tr, pos));
132123
}
@@ -136,5 +127,5 @@ export function removeAndInsertBlocks<
136127
nodeToBlock(node, pmSchema),
137128
) as Block<BSchema, I, S>[];
138129

139-
return { insertedBlocks, removedBlocks, affectedColumnLists };
130+
return { insertedBlocks, removedBlocks };
140131
}

0 commit comments

Comments
 (0)