Skip to content

Commit d0ea3cc

Browse files
matthewlipskimatthewlipski
committed
Fixed error triggered when moving blocks in some cases with multi-columns
1 parent ec9c151 commit d0ea3cc

4 files changed

Lines changed: 266 additions & 6 deletions

File tree

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

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ import type { BlockNoteEditor } from "../../../../editor/BlockNoteEditor";
1111
import { BlockIdentifier } from "../../../../schema/index.js";
1212
import { getNearestBlockPos } from "../../../getBlockInfoFromPos.js";
1313
import { getNodeById } from "../../../nodeUtil.js";
14+
import { insertBlocks } from "../insertBlocks/insertBlocks.js";
15+
import { removeAndInsertBlocks } from "../replaceBlocks/replaceBlocks.js";
16+
import { fixColumnList } from "../replaceBlocks/util/fixColumnList.js";
1417

1518
type BlockSelectionData = (
1619
| {
@@ -148,7 +151,7 @@ export function moveBlocks(
148151
referenceBlock: BlockIdentifier,
149152
placement: "before" | "after",
150153
) {
151-
editor.transact(() => {
154+
editor.transact((tr) => {
152155
// A `columnList` reference can be dissolved by `fixColumnList` when its
153156
// `column`s are removed, leaving its ID invalid for re-insertion. Anchor
154157
// to an adjacent block instead, which is unaffected by the removal.
@@ -164,8 +167,25 @@ export function moveBlocks(
164167
}
165168
}
166169

167-
editor.removeBlocks(blocks);
168-
editor.insertBlocks(flattenColumns(blocks), referenceBlock, placement);
170+
// Don't fix columns/columnLists in the removal step. Otherwise, the
171+
// following case breaks:
172+
// <column>
173+
// <paragraph></paragraph>
174+
// <paragraph>Paragraph</paragraph>
175+
// </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+
});
181+
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+
});
169189
});
170190
}
171191

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

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,13 @@ export function removeAndInsertBlocks<
2020
tr: Transaction,
2121
blocksToRemove: BlockIdentifier[],
2222
blocksToInsert: PartialBlock<BSchema, I, S>[],
23+
options: {
24+
fixColumns?: boolean;
25+
} = {},
2326
): {
2427
insertedBlocks: Block<BSchema, I, S>[];
2528
removedBlocks: Block<BSchema, I, S>[];
29+
affectedColumnLists: string[];
2630
} {
2731
const pmSchema = getPmSchema(tr);
2832
// Converts the `PartialBlock`s to ProseMirror nodes to insert them into the
@@ -112,12 +116,25 @@ export function removeAndInsertBlocks<
112116
);
113117
}
114118

115-
columnListPositions.forEach((pos) => fixColumnList(tr, pos));
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
130+
if (options.fixColumns !== false) {
131+
columnListPositions.forEach((pos) => fixColumnList(tr, pos));
132+
}
116133

117134
// Converts the nodes created from `blocksToInsert` into full `Block`s.
118135
const insertedBlocks = nodesToInsert.map((node) =>
119136
nodeToBlock(node, pmSchema),
120137
) as Block<BSchema, I, S>[];
121138

122-
return { insertedBlocks, removedBlocks };
139+
return { insertedBlocks, removedBlocks, affectedColumnLists };
123140
}

‎packages/xl-multi-column/src/test/commands/__snapshots__/moveBlocks.test.ts.snap‎

Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,183 @@
11
// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html
22

3+
exports[`Move past empty sibling within a column > Move down below empty sibling 1`] = `
4+
[
5+
{
6+
"children": [
7+
{
8+
"children": [
9+
{
10+
"children": [],
11+
"content": [
12+
{
13+
"styles": {},
14+
"text": "Text 0",
15+
"type": "text",
16+
},
17+
],
18+
"id": "text-0",
19+
"props": {
20+
"backgroundColor": "default",
21+
"textAlignment": "left",
22+
"textColor": "default",
23+
},
24+
"type": "paragraph",
25+
},
26+
{
27+
"children": [],
28+
"content": [],
29+
"id": "empty-0",
30+
"props": {
31+
"backgroundColor": "default",
32+
"textAlignment": "left",
33+
"textColor": "default",
34+
},
35+
"type": "paragraph",
36+
},
37+
],
38+
"content": undefined,
39+
"id": "column-empty-0",
40+
"props": {
41+
"width": 1,
42+
},
43+
"type": "column",
44+
},
45+
{
46+
"children": [
47+
{
48+
"children": [],
49+
"content": [],
50+
"id": "empty-1",
51+
"props": {
52+
"backgroundColor": "default",
53+
"textAlignment": "left",
54+
"textColor": "default",
55+
},
56+
"type": "paragraph",
57+
},
58+
{
59+
"children": [],
60+
"content": [
61+
{
62+
"styles": {},
63+
"text": "Text 1",
64+
"type": "text",
65+
},
66+
],
67+
"id": "text-1",
68+
"props": {
69+
"backgroundColor": "default",
70+
"textAlignment": "left",
71+
"textColor": "default",
72+
},
73+
"type": "paragraph",
74+
},
75+
],
76+
"content": undefined,
77+
"id": "column-empty-1",
78+
"props": {
79+
"width": 1,
80+
},
81+
"type": "column",
82+
},
83+
],
84+
"content": undefined,
85+
"id": "column-list-empty",
86+
"props": {},
87+
"type": "columnList",
88+
},
89+
]
90+
`;
91+
92+
exports[`Move past empty sibling within a column > Move up above empty sibling 1`] = `
93+
[
94+
{
95+
"children": [
96+
{
97+
"children": [
98+
{
99+
"children": [],
100+
"content": [
101+
{
102+
"styles": {},
103+
"text": "Text 0",
104+
"type": "text",
105+
},
106+
],
107+
"id": "text-0",
108+
"props": {
109+
"backgroundColor": "default",
110+
"textAlignment": "left",
111+
"textColor": "default",
112+
},
113+
"type": "paragraph",
114+
},
115+
{
116+
"children": [],
117+
"content": [],
118+
"id": "empty-0",
119+
"props": {
120+
"backgroundColor": "default",
121+
"textAlignment": "left",
122+
"textColor": "default",
123+
},
124+
"type": "paragraph",
125+
},
126+
],
127+
"content": undefined,
128+
"id": "column-empty-0",
129+
"props": {
130+
"width": 1,
131+
},
132+
"type": "column",
133+
},
134+
{
135+
"children": [
136+
{
137+
"children": [],
138+
"content": [],
139+
"id": "empty-1",
140+
"props": {
141+
"backgroundColor": "default",
142+
"textAlignment": "left",
143+
"textColor": "default",
144+
},
145+
"type": "paragraph",
146+
},
147+
{
148+
"children": [],
149+
"content": [
150+
{
151+
"styles": {},
152+
"text": "Text 1",
153+
"type": "text",
154+
},
155+
],
156+
"id": "text-1",
157+
"props": {
158+
"backgroundColor": "default",
159+
"textAlignment": "left",
160+
"textColor": "default",
161+
},
162+
"type": "paragraph",
163+
},
164+
],
165+
"content": undefined,
166+
"id": "column-empty-1",
167+
"props": {
168+
"width": 1,
169+
},
170+
"type": "column",
171+
},
172+
],
173+
"content": undefined,
174+
"id": "column-list-empty",
175+
"props": {},
176+
"type": "columnList",
177+
},
178+
]
179+
`;
180+
3181
exports[`Test moveBlocksDown > Move into column list 1`] = `
4182
[
5183
{

‎packages/xl-multi-column/src/test/commands/moveBlocks.test.ts‎

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { describe, expect, it } from "vite-plus/test";
1+
import { beforeEach, describe, expect, it } from "vite-plus/test";
22

33
import { setupTestEnv } from "../setupTestEnv.js";
44

@@ -151,3 +151,48 @@ describe("Test moveBlocksDown", () => {
151151
expect(getEditor().document).toMatchSnapshot();
152152
});
153153
});
154+
155+
describe("Move past empty sibling within a column", () => {
156+
beforeEach(() => {
157+
getEditor().replaceBlocks(getEditor().document, [
158+
{
159+
id: "column-list-empty",
160+
type: "columnList",
161+
children: [
162+
{
163+
id: "column-empty-0",
164+
type: "column",
165+
children: [
166+
{ id: "empty-0", type: "paragraph" },
167+
{ id: "text-0", type: "paragraph", content: "Text 0" },
168+
],
169+
},
170+
{
171+
id: "column-empty-1",
172+
type: "column",
173+
children: [
174+
{ id: "empty-1", type: "paragraph" },
175+
{ id: "text-1", type: "paragraph", content: "Text 1" },
176+
],
177+
},
178+
],
179+
},
180+
]);
181+
});
182+
183+
it("Move up above empty sibling", () => {
184+
getEditor().setTextCursorPosition("text-0");
185+
186+
expect(() => getEditor().moveBlocksUp()).not.toThrow();
187+
188+
expect(getEditor().document).toMatchSnapshot();
189+
});
190+
191+
it("Move down below empty sibling", () => {
192+
getEditor().setTextCursorPosition("empty-0");
193+
194+
expect(() => getEditor().moveBlocksDown()).not.toThrow();
195+
196+
expect(getEditor().document).toMatchSnapshot();
197+
});
198+
});

0 commit comments

Comments
 (0)