Skip to content

Commit ce11d6f

Browse files
nperez0111nperez0111
committed
refactor(core): address review feedback on the BlockInfo API surface
- Rename `insertBlocks` placements "start"/"end" to "first-child"/"last-child" (clearer about nesting; docs, tests, and jsdoc updated). - containerNav helpers now take a BlockInfo and share one shape: `descendToFirst/LastInsertionPos` both return `number | null` and accept `SealOpts` instead of the `crossedSeal` flag (callers that need "was a seal the only blocker" ask with a second seal-blind call); `getFirstLeafBlock` takes and returns BlockInfo. `ascendToInsertablePos` and `getAncestorContainers` stay position-based on purpose (their inputs are arbitrary gap positions, not blocks) and now say so in jsdoc. - Inline single-use indirections: `canMerge` + `mergeBlocks` fold into `mergeBlocksCommand` (the boolean guard makes the defensive throws statically unreachable, so they are gone); `movedNodeType` folds into `checkPlacementIsValid`; `seedRefillChildren` folds into `refillContainer`; `seedDefaultChildren` + `createContainerChildrenNode` fold into `blockToNode`; the `descend` closure folds into `getInsertionPos`. - Drop the unreachable exotic-shape guard in `getInsertionPos`: `getBlockRegions` already throws for bnBlock nodes that are neither containers nor blockContainer. - Split the inline/table-content conversion layer out of `blockToNode.ts` into `contentToNodes.ts`. - Add jsdocs to `fixContainersById` and `flattenNonInsertableBlocks`.
1 parent 9bcb0ad commit ce11d6f

19 files changed

Lines changed: 670 additions & 685 deletions

File tree

‎docs/content/docs/features/custom-schemas/container-blocks.mdx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -169,8 +169,8 @@ editor.insertBlocks([{ type: "paragraph" }], calloutId, "before");
169169
editor.insertBlocks([{ type: "paragraph" }], calloutId, "after");
170170

171171
// Nested inside it, as its first or last child:
172-
editor.insertBlocks([{ type: "paragraph" }], calloutId, "start");
173-
editor.insertBlocks([{ type: "paragraph" }], calloutId, "end");
172+
editor.insertBlocks([{ type: "paragraph" }], calloutId, "first-child");
173+
editor.insertBlocks([{ type: "paragraph" }], calloutId, "last-child");
174174
```
175175

176176
The nested placements are what addresses a container with no children to point at. A `min: 0` container that is currently empty has no child block to insert before or after. Whether a block fits is answered by the schema, so it's your `children` config that decides.

‎docs/content/docs/reference/editor/manipulating-content.mdx‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -141,11 +141,11 @@ editor.forEachBlock((block) => {
141141
insertBlocks(
142142
blocksToInsert: PartialBlock[],
143143
referenceBlock: BlockIdentifier,
144-
placement: "before" | "after" | "start" | "end" = "before"
144+
placement: "before" | "after" | "first-child" | "last-child" = "before"
145145
): void
146146
```
147147

148-
Inserts new blocks relative to an existing block. `"before"` and `"after"` make the new blocks siblings of the reference block; `"start"` and `"end"` nest them inside it, as its first or last children. See [Inserting into a container](/docs/features/custom-schemas/container-blocks#inserting-into-a-container).
148+
Inserts new blocks relative to an existing block. `"before"` and `"after"` make the new blocks siblings of the reference block; `"first-child"` and `"last-child"` nest them inside it. See [Inserting into a container](/docs/features/custom-schemas/container-blocks#inserting-into-a-container).
149149

150150
```typescript
151151
// Insert a paragraph before an existing block
@@ -169,7 +169,7 @@ editor.insertBlocks(
169169
editor.insertBlocks(
170170
[{ type: "paragraph", content: "Nested paragraph" }],
171171
"container-block-id",
172-
"end",
172+
"last-child",
173173
);
174174
```
175175

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

Lines changed: 11 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import {
88
InlineContentSchema,
99
StyleSchema,
1010
} from "../../../../schema/index.js";
11-
import { isContainerNode } from "../../../../schema/blocks/children.js";
1211
import { getBlockInfoFromNode } from "../../../getBlockInfoFromPos.js";
1312
import { blockToNode } from "../../../nodeConversions/blockToNode.js";
1413
import { nodeToBlock } from "../../../nodeConversions/nodeToBlock.js";
@@ -20,15 +19,14 @@ import {
2019
} from "../../containers/containerNav.js";
2120

2221
/**
23-
* Where blocks go relative to a reference block. `"before"`/`"after"` make them
24-
* siblings of it; `"start"`/`"end"` nest them inside it, as its first or last
25-
* children.
22+
* Where blocks go relative to a reference block. `"before"`/`"after"` make
23+
* them siblings of it; `"first-child"`/`"last-child"` nest them inside it.
2624
*
2725
* The nested placements cover containers that have no children to point at:
2826
* a `min: 0` container that is currently empty has no child block to insert
2927
* before or after.
3028
*/
31-
export type BlockPlacement = "before" | "after" | "start" | "end";
29+
export type BlockPlacement = "before" | "after" | "first-child" | "last-child";
3230

3331
/**
3432
* Resolves a `placement` against a reference block into the document position
@@ -52,11 +50,6 @@ export function getInsertionPos(
5250
): { pos: number; wrapIn?: NodeType } | null {
5351
const { node, posBeforeNode } = reference;
5452

55-
const descend = (holder: { node: Node; beforePos: number }) =>
56-
placement === "start"
57-
? descendToFirstInsertionPos(holder, nodeType)
58-
: descendToLastInsertionPos(holder, nodeType).pos;
59-
6053
if (placement === "before" || placement === "after") {
6154
const pos =
6255
placement === "before" ? posBeforeNode : posBeforeNode + node.nodeSize;
@@ -67,18 +60,16 @@ export function getInsertionPos(
6760
: null;
6861
}
6962

70-
// Neither a container nor a `blockContainer` (possible only for exotic
71-
// hand-written specs): nothing can nest inside it.
72-
if (!isContainerNode(node.type) && node.type.name !== "blockContainer") {
73-
return null;
74-
}
75-
7663
const info = getBlockInfoFromNode(node, posBeforeNode);
7764

7865
if (info.children) {
79-
// The descent helpers report sealed boundaries but this caller ignores
80-
// them: an explicit `insertBlocks` placement is an intentional crossing.
81-
const pos = descend(info.children);
66+
// The descent helpers can stop at sealed boundaries but this caller lets
67+
// them cross: an explicit `insertBlocks` placement is an intentional
68+
// crossing.
69+
const pos =
70+
placement === "first-child"
71+
? descendToFirstInsertionPos(info, nodeType)
72+
: descendToLastInsertionPos(info, nodeType);
8273

8374
return pos === null ? null : { pos };
8475
}
@@ -133,7 +124,7 @@ export function insertBlocks<
133124
`Cannot insert a block of type "${blocksToInsert[0].type ?? "paragraph"}" ` +
134125
(placement === "before" || placement === "after"
135126
? `${placement} block with ID ${id}: its parent does not accept it.`
136-
: `at the ${placement} of block with ID ${id}: the block does not accept it as a child.`),
127+
: `as the ${placement} of block with ID ${id}: the block does not accept it as a child.`),
137128
);
138129
}
139130

‎packages/core/src/api/blockManipulation/commands/insertBlocks/insertPlacement.test.ts‎

Lines changed: 45 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,9 @@ const container = (type: string, config: Record<string, unknown>) =>
2525
const schema = BlockNoteSchema.create().extend({
2626
blockSpecs: {
2727
...defaultBlockSpecs,
28-
// Why `"start"`/`"end"` exist: a container that may legally hold nothing
29-
// has no child block to address, so `"before"`/`"after"` cannot reach
30-
// inside it.
28+
// Why `"first-child"`/`"last-child"` exist: a container that may legally
29+
// hold nothing has no child block to address, so `"before"`/`"after"`
30+
// cannot reach inside it.
3131
box: container("box", {
3232
content: "none",
3333
children: { allow: "any", min: 0 },
@@ -68,16 +68,24 @@ beforeEach(() => {
6868
]);
6969
});
7070

71-
describe('insertBlocks "start" / "end"', () => {
71+
describe('insertBlocks "first-child" / "last-child"', () => {
7272
it("inserts into a childless container", () => {
7373
editor.replaceBlocks(editor.document, [
7474
{ id: "b-0", type: "box" },
7575
{ id: "trailing", type: "paragraph", content: "" },
7676
]);
7777
expect(editor.getBlock("b-0")!.children).toHaveLength(0);
7878

79-
editor.insertBlocks([{ id: "first", type: "paragraph" }], "b-0", "start");
80-
editor.insertBlocks([{ id: "last", type: "paragraph" }], "b-0", "end");
79+
editor.insertBlocks(
80+
[{ id: "first", type: "paragraph" }],
81+
"b-0",
82+
"first-child",
83+
);
84+
editor.insertBlocks(
85+
[{ id: "last", type: "paragraph" }],
86+
"b-0",
87+
"last-child",
88+
);
8189

8290
expect(editor.getBlock("b-0")!.children.map((child) => child.id)).toEqual([
8391
"first",
@@ -95,8 +103,16 @@ describe('insertBlocks "start" / "end"', () => {
95103
{ id: "trailing", type: "paragraph", content: "" },
96104
]);
97105

98-
editor.insertBlocks([{ id: "first", type: "paragraph" }], "b-0", "start");
99-
editor.insertBlocks([{ id: "last", type: "paragraph" }], "b-0", "end");
106+
editor.insertBlocks(
107+
[{ id: "first", type: "paragraph" }],
108+
"b-0",
109+
"first-child",
110+
);
111+
editor.insertBlocks(
112+
[{ id: "last", type: "paragraph" }],
113+
"b-0",
114+
"last-child",
115+
);
100116

101117
expect(editor.getBlock("b-0")!.children.map((child) => child.id)).toEqual([
102118
"first",
@@ -120,8 +136,16 @@ describe('insertBlocks "start" / "end"', () => {
120136

121137
// `grid` itself only accepts `cell`s, so both placements have to find the
122138
// leading/trailing cell rather than giving up.
123-
editor.insertBlocks([{ id: "first", type: "paragraph" }], "g-0", "start");
124-
editor.insertBlocks([{ id: "last", type: "paragraph" }], "g-0", "end");
139+
editor.insertBlocks(
140+
[{ id: "first", type: "paragraph" }],
141+
"g-0",
142+
"first-child",
143+
);
144+
editor.insertBlocks(
145+
[{ id: "last", type: "paragraph" }],
146+
"g-0",
147+
"last-child",
148+
);
125149

126150
const grid = editor.getBlock("g-0")!;
127151
expect(grid.children[0].children.map((child: any) => child.id)).toContain(
@@ -137,8 +161,16 @@ describe('insertBlocks "start" / "end"', () => {
137161
{ id: "p-0", type: "paragraph", content: "Paragraph 0" },
138162
]);
139163

140-
editor.insertBlocks([{ id: "existing", type: "paragraph" }], "p-0", "end");
141-
editor.insertBlocks([{ id: "first", type: "paragraph" }], "p-0", "start");
164+
editor.insertBlocks(
165+
[{ id: "existing", type: "paragraph" }],
166+
"p-0",
167+
"last-child",
168+
);
169+
editor.insertBlocks(
170+
[{ id: "first", type: "paragraph" }],
171+
"p-0",
172+
"first-child",
173+
);
142174

143175
expect(editor.getBlock("p-0")!.children.map((child) => child.id)).toEqual([
144176
"first",
@@ -157,7 +189,7 @@ describe('insertBlocks "start" / "end"', () => {
157189
]);
158190

159191
expect(() =>
160-
editor.insertBlocks([{ type: "paragraph" }], "s-0", "end"),
192+
editor.insertBlocks([{ type: "paragraph" }], "s-0", "last-child"),
161193
).toThrow(/does not accept it as a child/);
162194
});
163195

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

Lines changed: 45 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -1,75 +1,11 @@
11
import { EditorState } from "prosemirror-state";
22

33
import {
4-
BlockInfo,
54
getBlockInfoAt,
65
getLastDescendantBlockInfo,
76
getPrevBlockInfo,
87
} from "../../../getBlockInfoFromPos.js";
98

10-
const canMerge = (prevBlockInfo: BlockInfo, nextBlockInfo: BlockInfo) => {
11-
return (
12-
prevBlockInfo.hasContent &&
13-
prevBlockInfo.contentKind === "inline" &&
14-
!prevBlockInfo.isContentEmpty &&
15-
nextBlockInfo.hasContent &&
16-
nextBlockInfo.contentKind === "inline"
17-
);
18-
};
19-
20-
const mergeBlocks = (
21-
state: EditorState,
22-
dispatch: ((args?: any) => any) | undefined,
23-
prevBlockInfo: BlockInfo,
24-
nextBlockInfo: BlockInfo,
25-
) => {
26-
// Un-nests all children of the next block.
27-
if (!nextBlockInfo.hasContent) {
28-
throw new Error(
29-
`Attempted to merge block at position ${nextBlockInfo.block.beforePos} into previous block at position ${prevBlockInfo.block.beforePos}, but next block is not a block container`,
30-
);
31-
}
32-
33-
// Removes a level of nesting all children of the next block by 1 level, if it contains both content and block
34-
// group nodes.
35-
if (nextBlockInfo.children) {
36-
const childBlocksStart = state.doc.resolve(
37-
nextBlockInfo.children.childrenStart,
38-
);
39-
const childBlocksEnd = state.doc.resolve(
40-
nextBlockInfo.children.childrenEnd,
41-
);
42-
const childBlocksRange = childBlocksStart.blockRange(childBlocksEnd);
43-
44-
if (dispatch) {
45-
const pos = state.doc.resolve(nextBlockInfo.block.beforePos);
46-
state.tr.lift(childBlocksRange!, pos.depth);
47-
}
48-
}
49-
50-
// Deletes the boundary between the two blocks. Can be thought of as
51-
// removing the closing tags of the first block and the opening tags of the
52-
// second one to stitch them together.
53-
if (dispatch) {
54-
if (!prevBlockInfo.hasContent) {
55-
throw new Error(
56-
`Attempted to merge block at position ${nextBlockInfo.block.beforePos} into previous block at position ${prevBlockInfo.block.beforePos}, but previous block is not a block container`,
57-
);
58-
}
59-
60-
// Merging into or out of container blocks (columnLists, callouts, ...)
61-
// is intentionally unsupported; `canMerge` refuses it above. The
62-
// container-boundary Backspace/Delete branches in
63-
// `KeyboardShortcutsExtension` handle those cases by moving blocks
64-
// across the boundary instead of merging their content.
65-
dispatch(
66-
state.tr.delete(prevBlockInfo.contentEnd, nextBlockInfo.contentStart),
67-
);
68-
}
69-
70-
return true;
71-
};
72-
739
export const mergeBlocksCommand =
7410
(posBetweenBlocks: number) =>
7511
({
@@ -90,14 +26,57 @@ export const mergeBlocksCommand =
9026
return false;
9127
}
9228

29+
// The block we merge into is the last descendant of the previous block:
30+
// visually, that's the block directly above the boundary.
9331
const bottomNestedBlockInfo = getLastDescendantBlockInfo(
9432
state.doc,
9533
prevBlockInfo,
9634
);
9735

98-
if (!canMerge(bottomNestedBlockInfo, nextBlockInfo)) {
36+
// Only inline-content blocks can merge, and merging into an empty block
37+
// is handled elsewhere (by deleting the empty block instead). Merging
38+
// into or out of container blocks (columnLists, callouts, ...) is
39+
// intentionally unsupported; the container-boundary Backspace/Delete
40+
// branches in `KeyboardShortcutsExtension` handle those cases by moving
41+
// blocks across the boundary instead of merging their content.
42+
if (
43+
!bottomNestedBlockInfo.hasContent ||
44+
bottomNestedBlockInfo.contentKind !== "inline" ||
45+
bottomNestedBlockInfo.isContentEmpty ||
46+
!nextBlockInfo.hasContent ||
47+
nextBlockInfo.contentKind !== "inline"
48+
) {
9949
return false;
10050
}
10151

102-
return mergeBlocks(state, dispatch, bottomNestedBlockInfo, nextBlockInfo);
52+
// Removes a level of nesting all children of the next block by 1 level, if
53+
// it contains both content and block group nodes.
54+
if (nextBlockInfo.children) {
55+
const childBlocksStart = state.doc.resolve(
56+
nextBlockInfo.children.childrenStart,
57+
);
58+
const childBlocksEnd = state.doc.resolve(
59+
nextBlockInfo.children.childrenEnd,
60+
);
61+
const childBlocksRange = childBlocksStart.blockRange(childBlocksEnd);
62+
63+
if (dispatch) {
64+
const pos = state.doc.resolve(nextBlockInfo.block.beforePos);
65+
state.tr.lift(childBlocksRange!, pos.depth);
66+
}
67+
}
68+
69+
// Deletes the boundary between the two blocks. Can be thought of as
70+
// removing the closing tags of the first block and the opening tags of the
71+
// second one to stitch them together.
72+
if (dispatch) {
73+
dispatch(
74+
state.tr.delete(
75+
bottomNestedBlockInfo.contentEnd,
76+
nextBlockInfo.contentStart,
77+
),
78+
);
79+
}
80+
81+
return true;
10382
};

0 commit comments

Comments
 (0)