diff --git a/.claude/skills/testing-skill/SKILL.md b/.claude/skills/testing-skill/SKILL.md index 6ff6af56a9..969d4b0375 100644 --- a/.claude/skills/testing-skill/SKILL.md +++ b/.claude/skills/testing-skill/SKILL.md @@ -21,6 +21,21 @@ In most cases, once a feature, bug fix, or other modification has been written, `packages/*/src/**/*.browser.test.{ts,tsx}`: Unit tests for browser-only implementations (e.g. canvas or DOM-dependent code) live next to the code they test, with a `.browser.test` suffix. They run as part of the browser suite in Docker (the `tests` package's browser config includes them); the packages' own node-mode vitest configs exclude them. Use this when the unit under test genuinely needs a real browser — everything else should be a plain node unit test. +### Choosing between `/tests/src/unit` and a colocated test + +Both are unit tests, so "is this a unit or an integration test" is the wrong question. Pick by harness: + +- `/tests/src/unit` exists to fan a **single case** out across many output formats (BlockNote HTML, external HTML, Markdown, PM nodes) and across clipboard and selection behaviour, all against the one shared `testSchema`. You contribute a case by appending to a `*TestInstances.ts` array, not by adding a test file. If the schema needs a new block type to express the case, add it to `tests/src/unit/core/testSchema.ts` (or `react/testSchema.tsx`). +- A colocated test in `packages/*/src` pins the behaviour of one function or module, and is free to declare its own schema fixture. Use it when the assertion is about internal shape (node structure, transaction steps, return values) rather than about a serialization format. + +If a case belongs in both, prefer `/tests/src/unit`: one entry there produces coverage in every format at once. + +### Naming a colocated test file + +- When the suite covers one source file, mirror its name: `blockToNode.ts` gets `blockToNode.test.ts`. +- When it covers a behaviour spanning several modules, name it after the behaviour and put it in the directory that owns that behaviour: `containers/containers.test.ts`, `commands/insertBlocks/insertPlacement.test.ts`. This is common and fine; roughly a third of colocated test files have no same-named source file. +- Don't name a file after a schema or config feature (`contentContainers.test.ts`). Those names go stale when the feature is renamed or dropped, and the file is then stranded under a name that no longer maps to anything. Name it after the code that implements the feature instead. + ### End-to-End Tests `tests/src/end-to-end`: Tests that need a real browser and span multiple packages go here — chiefly tests which interact with the editor UI or simulate user interaction, but also browser integration tests that exercise complete flows without interaction (e.g. exporting a full document, static rendering). New subdirectories can be added if the functionality being tested is not covered by any of the existing ones. Important note about existing E2E tests - many are written poorly and should only loosely be used as reference. We want to avoid abstraction layers and `waitForTimeout` as much as possible. diff --git a/docs/content/docs/features/custom-schemas/container-blocks.mdx b/docs/content/docs/features/custom-schemas/container-blocks.mdx index 20f38fa447..0d03aace2e 100644 --- a/docs/content/docs/features/custom-schemas/container-blocks.mdx +++ b/docs/content/docs/features/custom-schemas/container-blocks.mdx @@ -83,7 +83,7 @@ The demo below puts this together: a callout block that can contain any other bl | `min` / `max` | `1` / unbounded | How many children are allowed. Compiled into the editor schema. | | `default` | none | Partial blocks to create the container with when it's inserted without an explicit `children` array, and the source of `"refill"` top-ups. Validated against the rest of the config when the schema is created. See [Defaults and refilling](#defaults-and-refilling). | | `whenEmptied` | `"refill"` | What happens when fewer non-empty children remain than `min`: `"refill"` tops the container back up from `default`; `"unwrap"` replaces the container with its surviving children, or removes it entirely when none are left. Column lists use `"unwrap"` so emptied columns disappear and a one-column list dissolves. | -| `boundary` | `"isolated"` | What crosses the container's edge: the caret, selections, or nothing. See [Boundaries](#boundaries). | +| `boundary` | `"open"` | Whether editing gestures cross the container's edge. See [Boundaries](#boundaries). | `placement` sits next to `children` on the block config rather than inside it, because it's a fact about *this* block rather than about its children: @@ -109,14 +109,15 @@ The same template drives `whenEmptied: "refill"`. When a refill container's non- ## Boundaries -`boundary` declares what may cross a container's edge. On an open or isolated edge, editing gestures move blocks across it: Backspace at the start of the first child moves that child out, and Enter on an empty last child escapes below the container. A sealed edge blocks all of that, so the container behaves as a single unit. +`boundary` declares whether editing gestures cross a container's edge. On an open edge they move blocks across it: Backspace at the start of the first child moves that child out, and Enter on an empty last child escapes below the container. A sealed edge blocks all of that, so the container behaves as a single unit. | Value | Crosses the edge | Use for | | --- | --- | --- | -| `"open"` | Caret, editing gestures, and text selections. A selection can span children and reach outside the container. | Flow regions where a selection should cross child boundaries, like the columns of a `columnList`. | -| `"isolated"` (default) | Caret and editing gestures, but not a text selection. | Most containers, like a callout. | +| `"open"` (default) | The caret and editing gestures. | Containers that are part of the surrounding flow of text, like a callout or the columns of a `columnList`. | | `"sealed"` | Nothing implicitly. The caret won't wander in, and from outside the container selects and deletes as one unit. | Compartments that should stay put, like a table cell. | +A seal binds gestures only. A text selection may span any edge, so a drag out of a sealed container still selects across it. + ```typescript // A cell: holds any blocks, but nothing crosses its edge implicitly. children: { allow: "any", boundary: "sealed" }, @@ -169,8 +170,8 @@ editor.insertBlocks([{ type: "paragraph" }], calloutId, "before"); editor.insertBlocks([{ type: "paragraph" }], calloutId, "after"); // Nested inside it, as its first or last child: -editor.insertBlocks([{ type: "paragraph" }], calloutId, "start"); -editor.insertBlocks([{ type: "paragraph" }], calloutId, "end"); +editor.insertBlocks([{ type: "paragraph" }], calloutId, "first-child"); +editor.insertBlocks([{ type: "paragraph" }], calloutId, "last-child"); ``` 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. @@ -183,7 +184,9 @@ Configurations are checked when the schema is created, and fail up front with a - an `allow` array naming an unknown type, or naming a regular block type (per-type filtering of regular blocks is [not yet supported](#restricting-children)); - `children` combined with any `content` other than `"none"`; - a `placement: "containerOnly"` block that no container's `allow` array names, or `placement: "containerOnly"` on a regular block; -- container cycles: a container that (transitively) requires a child that requires it back could never be created. An `allow` that permits regular blocks breaks the cycle, since they're always satisfiable. +- container cycles: a container that (transitively) requires a child that requires it back could never be created. An `allow` that permits regular blocks breaks the cycle, since they're always satisfiable. `allow: "containers"` with `min: 1` is the same problem, since the container counts as a container itself. + +Documents are checked too. `initialContent` that doesn't fit the schema throws when the editor is created, rather than loading in a broken state. This matters when you change a `children` config on a schema whose documents are already saved somewhere: a stored document that no longer fits, say a `columnList` left with a single column, now fails at load. Migrate those documents before shipping the change. ## Parsing HTML into a container diff --git a/docs/content/docs/reference/editor/manipulating-content.mdx b/docs/content/docs/reference/editor/manipulating-content.mdx index 873a186f17..5cde60c29c 100644 --- a/docs/content/docs/reference/editor/manipulating-content.mdx +++ b/docs/content/docs/reference/editor/manipulating-content.mdx @@ -141,11 +141,11 @@ editor.forEachBlock((block) => { insertBlocks( blocksToInsert: PartialBlock[], referenceBlock: BlockIdentifier, - placement: "before" | "after" | "start" | "end" = "before" + placement: "before" | "after" | "first-child" | "last-child" = "before" ): void ``` -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). +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). ```typescript // Insert a paragraph before an existing block @@ -169,7 +169,7 @@ editor.insertBlocks( editor.insertBlocks( [{ type: "paragraph", content: "Nested paragraph" }], "container-block-id", - "end", + "last-child", ); ``` diff --git a/examples/06-custom-schema/09-container-block/src/styles.css b/examples/06-custom-schema/09-container-block/src/styles.css index 8ecdb8f8b9..e4489738f2 100644 --- a/examples/06-custom-schema/09-container-block/src/styles.css +++ b/examples/06-custom-schema/09-container-block/src/styles.css @@ -7,7 +7,7 @@ .item { border-radius: 0.5rem; flex: 1; - overflow: hidden; + overflow: auto; } .item.bordered { diff --git a/packages/core/src/api/blockManipulation/commands/insertBlocks/insertBlocks.ts b/packages/core/src/api/blockManipulation/commands/insertBlocks/insertBlocks.ts index 21a5006fad..18aedb2519 100644 --- a/packages/core/src/api/blockManipulation/commands/insertBlocks/insertBlocks.ts +++ b/packages/core/src/api/blockManipulation/commands/insertBlocks/insertBlocks.ts @@ -8,26 +8,22 @@ import { InlineContentSchema, StyleSchema, } from "../../../../schema/index.js"; -import { isContainerNode } from "../../../../schema/blocks/children.js"; +import { getBlockInfoFromNode } from "../../../getBlockInfoFromPos.js"; import { blockToNode } from "../../../nodeConversions/blockToNode.js"; import { nodeToBlock } from "../../../nodeConversions/nodeToBlock.js"; import { getNodeById } from "../../../nodeUtil.js"; import { getPmSchema } from "../../../pmUtil.js"; -import { - descendToFirstInsertionPos, - descendToLastInsertionPos, -} from "../../containers/containerNav.js"; +import { descendToInsertionPos } from "../../containers/containerNav.js"; /** - * Where blocks go relative to a reference block. `"before"`/`"after"` make them - * siblings of it; `"start"`/`"end"` nest them inside it, as its first or last - * children. + * Where blocks go relative to a reference block. `"before"`/`"after"` make + * them siblings of it; `"first-child"`/`"last-child"` nest them inside it. * * The nested placements cover containers that have no children to point at: * a `min: 0` container that is currently empty has no child block to insert * before or after. */ -export type BlockPlacement = "before" | "after" | "start" | "end"; +export type BlockPlacement = "before" | "after" | "first-child" | "last-child"; /** * Resolves a `placement` against a reference block into the document position @@ -51,48 +47,43 @@ export function getInsertionPos( ): { pos: number; wrapIn?: NodeType } | null { const { node, posBeforeNode } = reference; - const descend = (holder: Node, pos: number) => - placement === "start" - ? descendToFirstInsertionPos(holder, pos, nodeType) - : descendToLastInsertionPos(holder, pos, nodeType); - if (placement === "before" || placement === "after") { const pos = placement === "before" ? posBeforeNode : posBeforeNode + node.nodeSize; const $pos = doc.resolve(pos); - return $pos.parent.contentMatchAt($pos.index()).matchType(nodeType) + // `canReplaceWith` rather than a bare content match: the nodes already + // after the position have to still fit once the new one is spliced in. + return $pos.parent.canReplaceWith($pos.index(), $pos.index(), nodeType) ? { pos } : null; } - // A container holds its children itself. The descent helpers ignore sealed - // boundaries by default, which is correct here: an explicit `insertBlocks` - // placement is an intentional crossing. - if (isContainerNode(node.type)) { - const pos = descend(node, posBeforeNode); + const info = getBlockInfoFromNode(node, posBeforeNode); + + if (info.children) { + // The navigation helpers stop at sealed boundaries but this caller lets + // them cross: an explicit `insertBlocks` placement is an intentional + // crossing. + const { pos } = descendToInsertionPos( + info, + nodeType, + placement === "first-child" ? "first" : "last", + { allowCrossingSeals: true }, + ); - return pos === null ? null : { pos }; + return pos === undefined ? null : { pos }; } - // A regular block keeps its children in a `blockGroup` that only exists once - // it has some. + // No children holder implies a `blockContainer` with no children yet + // (containers always have one): its `blockGroup` is lazy (`blockContent + // blockGroup?`), so the position after the content node only becomes valid + // once the nodes are wrapped in a new group. const blockGroupType = nodeType.schema.nodes["blockGroup"]; - if (node.type.name !== "blockContainer" || !blockGroupType) { - return null; - } - - const blockGroupPos = posBeforeNode + 1 + node.firstChild!.nodeSize; - - if (node.childCount < 2) { - return blockGroupType.contentMatch.matchType(nodeType) - ? { pos: blockGroupPos, wrapIn: blockGroupType } - : null; - } - - const pos = descend(node.lastChild!, blockGroupPos); - return pos === null ? null : { pos }; + return info.hasContent && blockGroupType?.contentMatch.matchType(nodeType) + ? { pos: info.content.afterPos, wrapIn: blockGroupType } + : null; } export function insertBlocks< @@ -134,7 +125,7 @@ export function insertBlocks< `Cannot insert a block of type "${blocksToInsert[0].type ?? "paragraph"}" ` + (placement === "before" || placement === "after" ? `${placement} block with ID ${id}: its parent does not accept it.` - : `at the ${placement} of block with ID ${id}: the block does not accept it as a child.`), + : `as the ${placement} of block with ID ${id}: the block does not accept it as a child.`), ); } diff --git a/packages/core/src/api/blockManipulation/commands/insertBlocks/insertPlacement.test.ts b/packages/core/src/api/blockManipulation/commands/insertBlocks/insertPlacement.test.ts index 132eafe1b1..36dc2419e5 100644 --- a/packages/core/src/api/blockManipulation/commands/insertBlocks/insertPlacement.test.ts +++ b/packages/core/src/api/blockManipulation/commands/insertBlocks/insertPlacement.test.ts @@ -25,9 +25,9 @@ const container = (type: string, config: Record) => const schema = BlockNoteSchema.create().extend({ blockSpecs: { ...defaultBlockSpecs, - // Why `"start"`/`"end"` exist: a container that may legally hold nothing - // has no child block to address, so `"before"`/`"after"` cannot reach - // inside it. + // Why `"first-child"`/`"last-child"` exist: a container that may legally + // hold nothing has no child block to address, so `"before"`/`"after"` + // cannot reach inside it. box: container("box", { content: "none", children: { allow: "any", min: 0 }, @@ -68,7 +68,7 @@ beforeEach(() => { ]); }); -describe('insertBlocks "start" / "end"', () => { +describe('insertBlocks "first-child" / "last-child"', () => { it("inserts into a childless container", () => { editor.replaceBlocks(editor.document, [ { id: "b-0", type: "box" }, @@ -76,8 +76,16 @@ describe('insertBlocks "start" / "end"', () => { ]); expect(editor.getBlock("b-0")!.children).toHaveLength(0); - editor.insertBlocks([{ id: "first", type: "paragraph" }], "b-0", "start"); - editor.insertBlocks([{ id: "last", type: "paragraph" }], "b-0", "end"); + editor.insertBlocks( + [{ id: "first", type: "paragraph" }], + "b-0", + "first-child", + ); + editor.insertBlocks( + [{ id: "last", type: "paragraph" }], + "b-0", + "last-child", + ); expect(editor.getBlock("b-0")!.children.map((child) => child.id)).toEqual([ "first", @@ -95,8 +103,16 @@ describe('insertBlocks "start" / "end"', () => { { id: "trailing", type: "paragraph", content: "" }, ]); - editor.insertBlocks([{ id: "first", type: "paragraph" }], "b-0", "start"); - editor.insertBlocks([{ id: "last", type: "paragraph" }], "b-0", "end"); + editor.insertBlocks( + [{ id: "first", type: "paragraph" }], + "b-0", + "first-child", + ); + editor.insertBlocks( + [{ id: "last", type: "paragraph" }], + "b-0", + "last-child", + ); expect(editor.getBlock("b-0")!.children.map((child) => child.id)).toEqual([ "first", @@ -120,8 +136,16 @@ describe('insertBlocks "start" / "end"', () => { // `grid` itself only accepts `cell`s, so both placements have to find the // leading/trailing cell rather than giving up. - editor.insertBlocks([{ id: "first", type: "paragraph" }], "g-0", "start"); - editor.insertBlocks([{ id: "last", type: "paragraph" }], "g-0", "end"); + editor.insertBlocks( + [{ id: "first", type: "paragraph" }], + "g-0", + "first-child", + ); + editor.insertBlocks( + [{ id: "last", type: "paragraph" }], + "g-0", + "last-child", + ); const grid = editor.getBlock("g-0")!; expect(grid.children[0].children.map((child: any) => child.id)).toContain( @@ -137,8 +161,16 @@ describe('insertBlocks "start" / "end"', () => { { id: "p-0", type: "paragraph", content: "Paragraph 0" }, ]); - editor.insertBlocks([{ id: "existing", type: "paragraph" }], "p-0", "end"); - editor.insertBlocks([{ id: "first", type: "paragraph" }], "p-0", "start"); + editor.insertBlocks( + [{ id: "existing", type: "paragraph" }], + "p-0", + "last-child", + ); + editor.insertBlocks( + [{ id: "first", type: "paragraph" }], + "p-0", + "first-child", + ); expect(editor.getBlock("p-0")!.children.map((child) => child.id)).toEqual([ "first", @@ -157,7 +189,7 @@ describe('insertBlocks "start" / "end"', () => { ]); expect(() => - editor.insertBlocks([{ type: "paragraph" }], "s-0", "end"), + editor.insertBlocks([{ type: "paragraph" }], "s-0", "last-child"), ).toThrow(/does not accept it as a child/); }); diff --git a/packages/core/src/api/blockManipulation/commands/mergeBlocks/mergeBlocks.test.ts b/packages/core/src/api/blockManipulation/commands/mergeBlocks/mergeBlocks.test.ts index ebe8ae9eff..532da5aaef 100644 --- a/packages/core/src/api/blockManipulation/commands/mergeBlocks/mergeBlocks.test.ts +++ b/packages/core/src/api/blockManipulation/commands/mergeBlocks/mergeBlocks.test.ts @@ -2,7 +2,10 @@ import { describe, expect, it } from "vite-plus/test"; import { getBlockInfoFromSelection } from "../../../getBlockInfoFromPos.js"; import { setupTestEnv } from "../../setupTestEnv.js"; -import { getParentBlockInfo, mergeBlocksCommand } from "./mergeBlocks.js"; +import { getParentBlockInfo } from "../../../getBlockInfoFromPos.js"; +import { getNodeById } from "../../../nodeUtil.js"; +import { containerSchema } from "../../containers/containers.fixture.js"; +import { mergeBlocksCommand } from "./mergeBlocks.js"; const getEditor = setupTestEnv(); @@ -14,7 +17,7 @@ function mergeBlocks(posBetweenBlocks: number) { function getPosBeforeSelectedBlock() { return getEditor().transact( - (tr) => getBlockInfoFromSelection(tr).bnBlock.beforePos, + (tr) => getBlockInfoFromSelection(tr).block.beforePos, ); } @@ -145,3 +148,87 @@ describe("Test mergeBlocks", () => { expect(ret).toBeFalsy(); }); }); + +describe("Test mergeBlocks at container boundaries", () => { + const getContainerEditor = setupTestEnv({ + schema: containerSchema, + document: [ + { id: "before-callout", type: "paragraph", content: "Before callout" }, + { + id: "callout-0", + type: "callout", + children: [ + { + id: "callout-child-0", + type: "paragraph", + content: "Callout child 0", + }, + { + id: "callout-child-1", + type: "paragraph", + content: "Callout child 1", + }, + ], + }, + { id: "after-callout", type: "paragraph", content: "After callout" }, + ], + }); + + function mergeContainerBlocks(posBetweenBlocks: number) { + return getContainerEditor()._tiptapEditor.commands.command( + mergeBlocksCommand(posBetweenBlocks), + ); + } + + function getPosBefore(id: string) { + return getContainerEditor().transact((tr) => { + const node = getNodeById(id, tr.doc); + if (!node) { + throw new Error(`No block with id "${id}" in the test document`); + } + return node.posBeforeNode; + }); + } + + // A container's first child has no previous sibling, so there is nothing to + // merge it into. The block above it on screen sits outside the container. + it("Does not merge a container's first child out of the container", () => { + const originalDocument = getContainerEditor().document; + const ret = mergeContainerBlocks(getPosBefore("callout-child-0")); + + expect(ret).toBeFalsy(); + expect(getContainerEditor().document).toEqual(originalDocument); + }); + + // A container has no content of its own, so there is nothing to merge. + it("Does not merge a container into the block above it", () => { + const originalDocument = getContainerEditor().document; + const ret = mergeContainerBlocks(getPosBefore("callout-0")); + + expect(ret).toBeFalsy(); + expect(getContainerEditor().document).toEqual(originalDocument); + }); + + // `mergeBlocksCommand` treats a container like any other block with children + // and merges into its last descendant, which puts the merged text inside the + // container. Backspace never produces this, because + // `KeyboardShortcutsExtension` bails out when the previous sibling has no + // inline content and moves the block into the container instead. So this is + // the command's behaviour on its own, not the editor's; it is pinned here + // because `mergeBlocks.ts` documents the opposite. + it("Merges a block into the last descendant of the container above it", () => { + const ret = mergeContainerBlocks(getPosBefore("after-callout")); + + expect(ret).toBeTruthy(); + + const document = getContainerEditor().document; + + expect(document.map((block) => block.id)).toEqual([ + "before-callout", + "callout-0", + ]); + expect(document[1].children[1].content).toEqual([ + { type: "text", text: "Callout child 1After callout", styles: {} }, + ]); + }); +}); diff --git a/packages/core/src/api/blockManipulation/commands/mergeBlocks/mergeBlocks.ts b/packages/core/src/api/blockManipulation/commands/mergeBlocks/mergeBlocks.ts index 5d1f0e3b51..c9ffb6003d 100644 --- a/packages/core/src/api/blockManipulation/commands/mergeBlocks/mergeBlocks.ts +++ b/packages/core/src/api/blockManipulation/commands/mergeBlocks/mergeBlocks.ts @@ -1,187 +1,11 @@ -import { Node } from "prosemirror-model"; import { EditorState } from "prosemirror-state"; -import { isSealed } from "../../../../schema/blocks/children.js"; import { - BlockInfo, - getBlockInfoFromResolvedPos, + getBlockInfoAt, + getLastDescendantBlockInfo, + getPrevBlockInfo, } from "../../../getBlockInfoFromPos.js"; -/** - * Returns the block info from the parent block - * or undefined if we're at the root - */ -export const getParentBlockInfo = ( - doc: Node, - beforePos: number, -): BlockInfo | undefined => { - const $pos = doc.resolve(beforePos); - const depth = $pos.depth - 1; - - if (depth < 1) { - return undefined; - } - - const parentBeforePos = $pos.before(depth); - const parentNode = doc.resolve(parentBeforePos).nodeAfter; - - if (!parentNode) { - return undefined; - } - - if (!parentNode.type.spec.group?.includes("bnBlock")) { - return getParentBlockInfo(doc, parentBeforePos); - } - - const parentBlockInfo = getBlockInfoFromResolvedPos( - doc.resolve(parentBeforePos), - ); - - return parentBlockInfo; -}; - -/** - * Returns the block info from the sibling block before (above) the given block, - * or undefined if the given block is the first sibling. - */ -export const getPrevBlockInfo = (doc: Node, beforePos: number) => { - const $pos = doc.resolve(beforePos); - - const indexInParent = $pos.index(); - - if (indexInParent === 0) { - return undefined; - } - - const prevBlockBeforePos = $pos.posAtIndex(indexInParent - 1); - - const prevBlockInfo = getBlockInfoFromResolvedPos( - doc.resolve(prevBlockBeforePos), - ); - return prevBlockInfo; -}; - -/** - * Returns the block info from the sibling block after (below) the given block, - * or undefined if the given block is the last sibling. - */ -export const getNextBlockInfo = (doc: Node, beforePos: number) => { - const $pos = doc.resolve(beforePos); - - const indexInParent = $pos.index(); - - if (indexInParent === $pos.node().childCount - 1) { - return undefined; - } - - const nextBlockBeforePos = $pos.posAtIndex(indexInParent + 1); - - const nextBlockInfo = getBlockInfoFromResolvedPos( - doc.resolve(nextBlockBeforePos), - ); - return nextBlockInfo; -}; - -/** - * If a block has children like this: - * A - * - B - * - C - * -- D - * - * Then the bottom nested block returned is D. - */ -export const getBottomNestedBlockInfo = ( - doc: Node, - blockInfo: BlockInfo, - // Callers that move content stop the descent at a sealed container, getting - // the container itself rather than a block inside it. Caret-only callers - // descend through. Sealed boundaries govern content, not navigation. - opts?: { stopAtSealed?: boolean }, -) => { - // A container that allows zero children can have an empty child container, - // in which case the block itself is the bottom one. - while (blockInfo.childContainer && blockInfo.childContainer.node.childCount) { - if (opts?.stopAtSealed && isSealed(blockInfo.childContainer.node)) { - break; - } - const group = blockInfo.childContainer.node; - - const newPos = doc - .resolve(blockInfo.childContainer.beforePos + 1) - .posAtIndex(group.childCount - 1); - blockInfo = getBlockInfoFromResolvedPos(doc.resolve(newPos)); - } - - return blockInfo; -}; - -const canMerge = (prevBlockInfo: BlockInfo, nextBlockInfo: BlockInfo) => { - return ( - prevBlockInfo.isWrappedBlock && - prevBlockInfo.blockContent.node.type.spec.content === "inline*" && - prevBlockInfo.blockContent.node.childCount > 0 && - nextBlockInfo.isWrappedBlock && - nextBlockInfo.blockContent.node.type.spec.content === "inline*" - ); -}; - -const mergeBlocks = ( - state: EditorState, - dispatch: ((args?: any) => any) | undefined, - prevBlockInfo: BlockInfo, - nextBlockInfo: BlockInfo, -) => { - // Un-nests all children of the next block. - if (!nextBlockInfo.isWrappedBlock) { - throw new Error( - `Attempted to merge block at position ${nextBlockInfo.bnBlock.beforePos} into previous block at position ${prevBlockInfo.bnBlock.beforePos}, but next block is not a block container`, - ); - } - - // Removes a level of nesting all children of the next block by 1 level, if it contains both content and block - // group nodes. - if (nextBlockInfo.childContainer) { - const childBlocksStart = state.doc.resolve( - nextBlockInfo.childContainer.beforePos + 1, - ); - const childBlocksEnd = state.doc.resolve( - nextBlockInfo.childContainer.afterPos - 1, - ); - const childBlocksRange = childBlocksStart.blockRange(childBlocksEnd); - - if (dispatch) { - const pos = state.doc.resolve(nextBlockInfo.bnBlock.beforePos); - state.tr.lift(childBlocksRange!, pos.depth); - } - } - - // Deletes the boundary between the two blocks. Can be thought of as - // removing the closing tags of the first block and the opening tags of the - // second one to stitch them together. - if (dispatch) { - if (!prevBlockInfo.isWrappedBlock) { - throw new Error( - `Attempted to merge block at position ${nextBlockInfo.bnBlock.beforePos} into previous block at position ${prevBlockInfo.bnBlock.beforePos}, but previous block is not a block container`, - ); - } - - // Merging into or out of container blocks (columnLists, callouts, ...) - // is intentionally unsupported; `canMerge` refuses it above. The - // container-boundary Backspace/Delete branches in - // `KeyboardShortcutsExtension` handle those cases by moving blocks - // across the boundary instead of merging their content. - dispatch( - state.tr.delete( - prevBlockInfo.blockContent.afterPos - 1, - nextBlockInfo.blockContent.beforePos + 1, - ), - ); - } - - return true; -}; - export const mergeBlocksCommand = (posBetweenBlocks: number) => ({ @@ -191,26 +15,75 @@ export const mergeBlocksCommand = state: EditorState; dispatch: ((args?: any) => any) | undefined; }) => { - const $pos = state.doc.resolve(posBetweenBlocks); - const nextBlockInfo = getBlockInfoFromResolvedPos($pos); + const nextBlockInfo = getBlockInfoAt(state.doc, posBetweenBlocks); const prevBlockInfo = getPrevBlockInfo( state.doc, - nextBlockInfo.bnBlock.beforePos, + nextBlockInfo.block.beforePos, ); if (!prevBlockInfo) { return false; } - const bottomNestedBlockInfo = getBottomNestedBlockInfo( + // The block we merge into is the last descendant of the previous block: + // visually, that's the block directly above the boundary. + const bottomNestedBlockInfo = getLastDescendantBlockInfo( state.doc, prevBlockInfo, ); - if (!canMerge(bottomNestedBlockInfo, nextBlockInfo)) { + // Only inline-content blocks can merge, and merging into an empty block + // is handled elsewhere (by deleting the empty block instead). Merging + // into or out of container blocks (columnLists, callouts, ...) is + // intentionally unsupported; the container-boundary Backspace/Delete + // branches in `KeyboardShortcutsExtension` handle those cases by moving + // blocks across the boundary instead of merging their content. + if ( + !bottomNestedBlockInfo.hasContent || + bottomNestedBlockInfo.contentKind !== "inline" || + bottomNestedBlockInfo.isContentEmpty || + !nextBlockInfo.hasContent || + nextBlockInfo.contentKind !== "inline" + ) { return false; } - return mergeBlocks(state, dispatch, bottomNestedBlockInfo, nextBlockInfo); + // Un-nests the next block's children by one level, so they survive as + // siblings of the merged block rather than as children of a block that no + // longer exists once the boundary below is deleted. + // + // Note `state.tr` is tiptap's chainable state, whose getter returns the one + // transaction shared by the command chain (not a fresh `Transaction` like + // `EditorState.tr`), so this lift carries over into the `dispatch` below. + if (dispatch && nextBlockInfo.children) { + const childBlocksRange = state.doc + .resolve(nextBlockInfo.children.childrenStart) + .blockRange(state.doc.resolve(nextBlockInfo.children.childrenEnd)); + + if (!childBlocksRange) { + throw new Error( + "Children of a block are expected to form a block range", + ); + } + + state.tr.lift( + childBlocksRange, + state.doc.resolve(nextBlockInfo.block.beforePos).depth, + ); + } + + // Deletes the boundary between the two blocks. Can be thought of as + // removing the closing tags of the first block and the opening tags of the + // second one to stitch them together. + if (dispatch) { + dispatch( + state.tr.delete( + bottomNestedBlockInfo.contentEnd, + nextBlockInfo.contentStart, + ), + ); + } + + return true; }; diff --git a/packages/core/src/api/blockManipulation/commands/moveBlocks/moveBlocks.test.ts b/packages/core/src/api/blockManipulation/commands/moveBlocks/moveBlocks.test.ts index f034506f44..f9bba17c3f 100644 --- a/packages/core/src/api/blockManipulation/commands/moveBlocks/moveBlocks.test.ts +++ b/packages/core/src/api/blockManipulation/commands/moveBlocks/moveBlocks.test.ts @@ -3,7 +3,7 @@ import { CellSelection } from "prosemirror-tables"; import { describe, expect, it } from "vite-plus/test"; import { - getBlockInfoAtNearest, + getBlockInfoNearPos, getBlockInfoFromSelection, getNodeId, } from "../../../getBlockInfoFromPos.js"; @@ -18,12 +18,12 @@ const getEditor = setupTestEnv(); function makeSelectionSpanContent(selectionType: "text" | "node" | "cell") { const blockInfo = getEditor().transact((tr) => getBlockInfoFromSelection(tr)); - if (!blockInfo.isWrappedBlock) { + if (!blockInfo.hasContent) { throw new Error( `Selection points to a ${blockInfo.blockNoteType} node, not a blockContainer node`, ); } - const { blockContent } = blockInfo; + const { content } = blockInfo; const editor = getEditor(); if (selectionType === "cell") { @@ -31,22 +31,22 @@ function makeSelectionSpanContent(selectionType: "text" | "node" | "cell") { tr.setSelection( CellSelection.create( tr.doc, - tr.doc.resolve(blockContent.beforePos + 3).before(), - tr.doc.resolve(blockContent.afterPos - 3).before(), + tr.doc.resolve(content.beforePos + 3).before(), + tr.doc.resolve(content.afterPos - 3).before(), ), ), ); } else if (selectionType === "node") { editor.transact((tr) => - tr.setSelection(NodeSelection.create(tr.doc, blockContent.beforePos)), + tr.setSelection(NodeSelection.create(tr.doc, content.beforePos)), ); } else { editor.transact((tr) => tr.setSelection( TextSelection.create( tr.doc, - blockContent.beforePos + 1, - blockContent.afterPos - 1, + content.beforePos + 1, + content.afterPos - 1, ), ), ); @@ -223,11 +223,11 @@ describe("Test moveBlocksUp", () => { const { anchorBlockId, headBlockId } = getEditor().transact((tr) => ({ anchorBlockId: getNodeId( - getBlockInfoAtNearest(tr, tr.selection.anchor).bnBlock.node, + getBlockInfoNearPos(tr, tr.selection.anchor).block.node, tr.doc, ), headBlockId: getNodeId( - getBlockInfoAtNearest(tr, tr.selection.head).bnBlock.node, + getBlockInfoNearPos(tr, tr.selection.head).block.node, tr.doc, ), })); @@ -347,11 +347,11 @@ describe("Test moveBlocksDown", () => { const { anchorBlockId, headBlockId } = getEditor().transact((tr) => ({ anchorBlockId: getNodeId( - getBlockInfoAtNearest(tr, tr.selection.anchor).bnBlock.node, + getBlockInfoNearPos(tr, tr.selection.anchor).block.node, tr.doc, ), headBlockId: getNodeId( - getBlockInfoAtNearest(tr, tr.selection.head).bnBlock.node, + getBlockInfoNearPos(tr, tr.selection.head).block.node, tr.doc, ), })); diff --git a/packages/core/src/api/blockManipulation/commands/moveBlocks/moveBlocks.ts b/packages/core/src/api/blockManipulation/commands/moveBlocks/moveBlocks.ts index ea97ae8869..267a23b27c 100644 --- a/packages/core/src/api/blockManipulation/commands/moveBlocks/moveBlocks.ts +++ b/packages/core/src/api/blockManipulation/commands/moveBlocks/moveBlocks.ts @@ -1,4 +1,4 @@ -import { NodeType } from "prosemirror-model"; +import type { Node, Schema } from "prosemirror-model"; import { NodeSelection, Selection, @@ -11,14 +11,43 @@ import { Block } from "../../../../blocks/defaultBlocks.js"; import type { BlockNoteEditor } from "../../../../editor/BlockNoteEditor"; import { BlockIdentifier } from "../../../../schema/index.js"; import { - getBlockInfoAtNearest, + isBlockGroupInsertable, + isContainerNode, + isSealed, +} from "../../../../schema/blocks/children.js"; +import { + getBlockInfoNearPos, getNodeId, } from "../../../getBlockInfoFromPos.js"; import { getNodeById } from "../../../nodeUtil.js"; -import { flattenNonInsertableBlocks } from "../../containers/fixContainer.js"; import { getInsertionPos, insertBlocks } from "../insertBlocks/insertBlocks.js"; import { removeAndInsertBlocks } from "../replaceBlocks/replaceBlocks.js"; +/** + * Dissolves `placement: "containerOnly"` blocks into their children. + * + * A `containerOnly` block (a `column`, say) is defined only in terms of the + * container that holds it, so it can't land anywhere a regular block goes — + * moving one out of its container moves its children instead. Every other + * block passes through as itself. + */ +function dissolveContainerOnlyBlocks( + blocks: Block[], + pmSchema: Schema, +): Block[] { + return blocks.flatMap((block) => { + const nodeType = pmSchema.nodes[block.type]; + // A container denied the `blockGroupChild` group is one declared + // `placement: "containerOnly"`. + const isContainerOnly = + isContainerNode(nodeType) && !isBlockGroupInsertable(nodeType); + + return isContainerOnly + ? dissolveContainerOnlyBlocks(block.children, pmSchema) + : [block]; + }); +} + type BlockSelectionData = ( | { type: "text"; @@ -51,18 +80,18 @@ function getBlockSelectionData( editor: BlockNoteEditor, ): BlockSelectionData { return editor.transact((tr) => { - const anchorBlockPosInfo = getBlockInfoAtNearest(tr, tr.selection.anchor); + const anchorBlockPosInfo = getBlockInfoNearPos(tr, tr.selection.anchor); - const anchorBlockId = getNodeId(anchorBlockPosInfo.bnBlock.node, tr.doc); + const anchorBlockId = getNodeId(anchorBlockPosInfo.block.node, tr.doc); if (tr.selection instanceof CellSelection) { return { type: "cell" as const, anchorBlockId, anchorCellOffset: - tr.selection.$anchorCell.pos - anchorBlockPosInfo.bnBlock.beforePos, + tr.selection.$anchorCell.pos - anchorBlockPosInfo.block.beforePos, headCellOffset: - tr.selection.$headCell.pos - anchorBlockPosInfo.bnBlock.beforePos, + tr.selection.$headCell.pos - anchorBlockPosInfo.block.beforePos, }; } else if (tr.selection instanceof NodeSelection) { return { @@ -70,15 +99,14 @@ function getBlockSelectionData( anchorBlockId, }; } else { - const headBlockPosInfo = getBlockInfoAtNearest(tr, tr.selection.head); + const headBlockPosInfo = getBlockInfoNearPos(tr, tr.selection.head); return { type: "text" as const, anchorBlockId, - headBlockId: getNodeId(headBlockPosInfo.bnBlock.node, tr.doc), - anchorOffset: - tr.selection.anchor - anchorBlockPosInfo.bnBlock.beforePos, - headOffset: tr.selection.head - headBlockPosInfo.bnBlock.beforePos, + headBlockId: getNodeId(headBlockPosInfo.block.node, tr.doc), + anchorOffset: tr.selection.anchor - anchorBlockPosInfo.block.beforePos, + headOffset: tr.selection.head - headBlockPosInfo.block.beforePos, }; } }); @@ -164,9 +192,7 @@ export function moveBlocks( removeAndInsertBlocks(tr, blocks, [], { fixContainers: false }); insertBlocks( tr, - // Blocks that can't stand on their own outside their container (e.g. a - // `column` outside its `columnList`) are replaced by their children. - flattenNonInsertableBlocks(blocks, editor.pmSchema), + dissolveContainerOnlyBlocks(blocks, editor.pmSchema), referenceBlock, placement, ); @@ -201,6 +227,21 @@ export function moveSelectedBlocksAndSelection( }); } +// The nearest sealed container a position sits in, or `undefined` if there +// isn't one. +function sealedAncestorId(doc: Node, pos: number): string | undefined { + const $pos = doc.resolve(pos); + + for (let depth = $pos.depth; depth > 0; depth--) { + const ancestor = $pos.node(depth); + if (isSealed(ancestor)) { + return ancestor.attrs.id; + } + } + + return undefined; +} + // Checks if a regular block would be in a valid place after being moved // before/after `referenceBlock`. A regular block nests under any non-container // block (it goes into that block's `blockGroup`), but a container block (e.g. a @@ -213,31 +254,39 @@ function checkPlacementIsValid( editor: BlockNoteEditor, referenceBlock: Block, placement: "before" | "after", - nodeType: NodeType, + movedBlock: Block, ): boolean { + // The PM node type to validate the destination against: the first block + // `moveBlocks` would actually insert, which is `movedBlock` itself unless it + // dissolves. A container (e.g. a `callout`) is inserted as its own node + // type; anything else goes in as a generic `blockContainer` wrapper. + const first = dissolveContainerOnlyBlocks([movedBlock], editor.pmSchema)[0]; + const firstType = first ? editor.pmSchema.nodes[first.type] : undefined; + const nodeType = + firstType && isContainerNode(firstType) + ? firstType + : editor.pmSchema.nodes["blockContainer"]; + return editor.transact((tr) => { const posInfo = getNodeById(referenceBlock.id, tr.doc); - if (!posInfo) { + const movedPosInfo = getNodeById(movedBlock.id, tr.doc); + if (!posInfo || !movedPosInfo) { return false; } - return getInsertionPos(tr.doc, posInfo, placement, nodeType) !== null; - }); -} -// The PM node type `insertBlocks` validates a destination against: the first -// flattened block's own node type when it's a container (e.g. `callout`), -// otherwise the generic `blockContainer` wrapper. Mirrors what `moveBlocks` -// inserts (`flattenNonInsertableBlocks` + `insertBlocks`), so the placement -// pre-check agrees with the insertion instead of always assuming a regular -// block. -function movedNodeType( - editor: BlockNoteEditor, - block: Block, -): NodeType { - const blockContainer = editor.pmSchema.nodes["blockContainer"]; - const first = flattenNonInsertableBlocks([block], editor.pmSchema)[0]; - const nodeType = first?.type ? editor.pmSchema.nodes[first.type] : undefined; - return nodeType?.isInGroup("bnBlock") ? nodeType : blockContainer; + const target = getInsertionPos(tr.doc, posInfo, placement, nodeType); + if (!target) { + return false; + } + + // Moving is a gesture, so it can't take a block across a seal: the block + // and its destination have to sit inside the same sealed container (or + // outside any of them). + return ( + sealedAncestorId(tr.doc, target.pos) === + sealedAncestorId(tr.doc, movedPosInfo.posBeforeNode) + ); + }); } // Gets the placement for moving a block up. This has 3 cases: @@ -252,7 +301,7 @@ function movedNodeType( // the block is already at the top of the document. function getMoveUpPlacement( editor: BlockNoteEditor, - nodeType: NodeType, + movedBlock: Block, prevBlock?: Block, parentBlock?: Block, ): @@ -279,11 +328,11 @@ function getMoveUpPlacement( return undefined; } - if (!checkPlacementIsValid(editor, referenceBlock, placement, nodeType)) { + if (!checkPlacementIsValid(editor, referenceBlock, placement, movedBlock)) { const referenceBlockParent = editor.getParentBlock(referenceBlock); return getMoveUpPlacement( editor, - nodeType, + movedBlock, placement === "after" ? referenceBlock : editor.getPrevBlock(referenceBlock), @@ -306,7 +355,7 @@ function getMoveUpPlacement( // the block is already at the bottom of the document. function getMoveDownPlacement( editor: BlockNoteEditor, - nodeType: NodeType, + movedBlock: Block, nextBlock?: Block, parentBlock?: Block, ): @@ -333,11 +382,11 @@ function getMoveDownPlacement( return undefined; } - if (!checkPlacementIsValid(editor, referenceBlock, placement, nodeType)) { + if (!checkPlacementIsValid(editor, referenceBlock, placement, movedBlock)) { const referenceBlockParent = editor.getParentBlock(referenceBlock); return getMoveDownPlacement( editor, - nodeType, + movedBlock, placement === "before" ? referenceBlock : editor.getNextBlock(referenceBlock), @@ -367,7 +416,7 @@ export function moveBlocksUp( const moveUpPlacement = getMoveUpPlacement( editor, - movedNodeType(editor, sourceBlock), + sourceBlock, editor.getPrevBlock(sourceBlock), editor.getParentBlock(sourceBlock), ); @@ -420,7 +469,7 @@ export function moveBlocksDown( const moveDownPlacement = getMoveDownPlacement( editor, - movedNodeType(editor, firstMovedBlock), + firstMovedBlock, editor.getNextBlock(sourceBlock), editor.getParentBlock(sourceBlock), ); diff --git a/packages/core/src/api/blockManipulation/commands/nestBlock/nestBlock.test.ts b/packages/core/src/api/blockManipulation/commands/nestBlock/nestBlock.test.ts index 8247e9391c..da69161218 100644 --- a/packages/core/src/api/blockManipulation/commands/nestBlock/nestBlock.test.ts +++ b/packages/core/src/api/blockManipulation/commands/nestBlock/nestBlock.test.ts @@ -3,6 +3,7 @@ import { describe, expect, it } from "vite-plus/test"; import { afterAll, beforeAll } from "vite-plus/test"; import { PartialBlock } from "../../../../blocks/defaultBlocks.js"; import { BlockNoteEditor } from "../../../../editor/BlockNoteEditor.js"; +import { containerSchema } from "../../containers/containers.fixture.js"; /** * Custom test setup with a document designed to reproduce nesting/unnesting bugs. @@ -646,6 +647,132 @@ describe("unnestBlock / liftListItem", () => { }); }); +// A second editor, on a schema that has container blocks. `setupNestTestEnv` +// builds a default-schema editor, which can't express any of the cases below. +function setupContainerNestTestEnv() { + let editor: BlockNoteEditor; + const div = document.createElement("div"); + + beforeAll(() => { + editor = BlockNoteEditor.create({ schema: containerSchema }); + editor.mount(div); + }); + + afterAll(() => { + editor._tiptapEditor.destroy(); + editor = undefined as any; + }); + + return (doc: PartialBlock[]) => { + editor.replaceBlocks(editor.document, doc); + return editor; + }; +} + +// `canNestBlock` and `canUnnestBlock` run the real command on a transaction +// that is thrown away, rather than restating its preconditions. The cases here +// are the ones where the old, restated preconditions gave the wrong answer: +// they looked at a previous sibling's mere existence and at the block's depth, +// neither of which knows anything about containers. +describe("canNestBlock / canUnnestBlock around containers", () => { + const withContainerEditor = setupContainerNestTestEnv(); + + it("Reports that a block cannot be nested under a container sibling", () => { + const editor = withContainerEditor([ + { + id: "callout-0", + type: "callout", + children: [ + { id: "callout-child", type: "paragraph", content: "Callout child" }, + ], + }, + { id: "paragraph-0", type: "paragraph", content: "Paragraph 0" }, + ]); + + editor.setTextCursorPosition("paragraph-0", "start"); + + const before = editor.document; + expect(editor.canNestBlock()).toBe(false); + + // And the answer matches what nesting actually does. + editor.nestBlock(); + expect(editor.document).toEqual(before); + }); + + it("Reports that a container's child cannot be unnested out of it", () => { + const editor = withContainerEditor([ + { + id: "callout-0", + type: "callout", + children: [ + { id: "callout-child", type: "paragraph", content: "Callout child" }, + ], + }, + ]); + + editor.setTextCursorPosition("callout-child", "start"); + + const before = editor.document; + expect(editor.canUnnestBlock()).toBe(false); + + editor.unnestBlock(); + expect(editor.document).toEqual(before); + }); + + it("Reports that a block with a plain previous sibling can be nested", () => { + const editor = withContainerEditor([ + { id: "paragraph-0", type: "paragraph", content: "Paragraph 0" }, + { id: "paragraph-1", type: "paragraph", content: "Paragraph 1" }, + ]); + + editor.setTextCursorPosition("paragraph-1", "start"); + + const before = editor.document; + expect(editor.canNestBlock()).toBe(true); + // The probe runs the command on a transaction it never dispatches, so + // answering must not change the document. + expect(editor.document).toEqual(before); + + editor.nestBlock(); + expect(editor.getBlock("paragraph-0")!.children.map((c) => c.id)).toEqual([ + "paragraph-1", + ]); + expect(editor.canUnnestBlock()).toBe(true); + }); + + it("Nests and unnests a block inside a container's children", () => { + const editor = withContainerEditor([ + { + id: "callout-0", + type: "callout", + children: [ + { id: "child-0", type: "paragraph", content: "Child 0" }, + { id: "child-1", type: "paragraph", content: "Child 1" }, + ], + }, + ]); + + const before = editor.document; + + editor.setTextCursorPosition("child-1", "start"); + expect(editor.canNestBlock()).toBe(true); + editor.nestBlock(); + + expect(editor.getBlock("callout-0")!.children.map((c) => c.id)).toEqual([ + "child-0", + ]); + expect(editor.getBlock("child-0")!.children.map((c) => c.id)).toEqual([ + "child-1", + ]); + + editor.setTextCursorPosition("child-1", "start"); + expect(editor.canUnnestBlock()).toBe(true); + editor.unnestBlock(); + + expect(editor.document).toEqual(before); + }); +}); + /** Recursively collects all block IDs from a document */ function flattenBlockIds(blocks: any[]): string[] { const ids: string[] = []; diff --git a/packages/core/src/api/blockManipulation/commands/nestBlock/nestBlock.ts b/packages/core/src/api/blockManipulation/commands/nestBlock/nestBlock.ts index 243e4532dd..b7b091b8f4 100644 --- a/packages/core/src/api/blockManipulation/commands/nestBlock/nestBlock.ts +++ b/packages/core/src/api/blockManipulation/commands/nestBlock/nestBlock.ts @@ -3,7 +3,6 @@ import { Transaction } from "prosemirror-state"; import { canJoin, liftTarget, ReplaceAroundStep } from "prosemirror-transform"; import { BlockNoteEditor } from "../../../../editor/BlockNoteEditor.js"; -import { getBlockInfoFromSelection } from "../../../getBlockInfoFromPos.js"; /** * Modified version of prosemirror-schema-list's sinkItem. @@ -62,14 +61,17 @@ function sinkItem(tr: Transaction, itemType: NodeType, groupType: NodeType) { return true; } -export function nestBlock(editor: BlockNoteEditor) { - return editor.transact((tr) => { - return sinkItem( +function nestCommand(editor: BlockNoteEditor) { + return (tr: Transaction) => + sinkItem( tr, editor.pmSchema.nodes["blockContainer"], editor.pmSchema.nodes["blockGroup"], ); - }); +} + +export function nestBlock(editor: BlockNoteEditor) { + return editor.transact(nestCommand(editor)); } /** @@ -177,50 +179,28 @@ export function liftItem( return false; } -export function unnestBlock(editor: BlockNoteEditor) { - return editor.transact((tr) => +function unnestCommand(editor: BlockNoteEditor) { + return (tr: Transaction) => liftItem( tr, editor.pmSchema.nodes["blockContainer"], editor.pmSchema.nodes["blockGroup"], - ), - ); + ); } +export function unnestBlock(editor: BlockNoteEditor) { + return editor.transact(unnestCommand(editor)); +} + +// `canExec` hands the command a transaction it never dispatches, so "can I +// nest?" is answered by nesting and throwing the result away. A second +// statement of the preconditions would drift from the command it describes — +// and did: it read a previous sibling's mere existence, so a container block +// before the cursor enabled the button while `nestBlock` did nothing. export function canNestBlock(editor: BlockNoteEditor) { - return editor.transact((tr) => { - const { bnBlock: blockContainer } = getBlockInfoFromSelection(tr); - - // Mirrors `sinkItem`'s precondition: nesting is only possible under a - // previous sibling that is itself a `blockContainer`. (A previous sibling - // of another type, e.g. a container block, made this return true while - // `nestBlock` did nothing.) - return ( - tr.doc.resolve(blockContainer.beforePos).nodeBefore?.type === - editor.pmSchema.nodes["blockContainer"] - ); - }); + return editor.canExec((state) => nestCommand(editor)(state.tr)); } export function canUnnestBlock(editor: BlockNoteEditor) { - return editor.transact((tr) => { - const { $from, $to } = tr.selection; - - // Mirrors `liftItem`'s preconditions instead of approximating with depth. - // A block whose depth > 1 because it sits inside a container (e.g. a - // column) is not un-nestable, only a block nested under another - // `blockContainer` is. - const range = $from.blockRange( - $to, - (node) => node.childCount > 0 && node.type.isInGroup("childContainer"), - ); - if (!range) { - return false; - } - - return ( - $from.node(range.depth - 1).type === - editor.pmSchema.nodes["blockContainer"] - ); - }); + return editor.canExec((state) => unnestCommand(editor)(state.tr)); } diff --git a/packages/core/src/api/blockManipulation/commands/replaceBlocks/replaceBlocks.test.ts b/packages/core/src/api/blockManipulation/commands/replaceBlocks/replaceBlocks.test.ts index 5a968c49bf..4951b09fb4 100644 --- a/packages/core/src/api/blockManipulation/commands/replaceBlocks/replaceBlocks.test.ts +++ b/packages/core/src/api/blockManipulation/commands/replaceBlocks/replaceBlocks.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vite-plus/test"; import { setupTestEnv } from "../../setupTestEnv.js"; +import { updateBlock } from "../updateBlock/updateBlock.js"; import { removeAndInsertBlocks } from "./replaceBlocks.js"; import { BlockNoteEditor } from "../../../../editor/BlockNoteEditor.js"; import { PartialBlock } from "../../../../blocks/defaultBlocks.js"; @@ -233,3 +234,73 @@ describe("Test replaceBlocks", () => { expect(getEditor().document).toMatchSnapshot(); }); }); + +// `removeAndInsertBlocks` walks the document while mutating it, so the +// positions it reads go stale as it goes. It corrects for that with +// `tr.mapping.slice(stepsBefore)`, where `stepsBefore` is the step count on +// entry. The slice is what makes the function safe to call on a transaction +// that already carries steps: an unsliced `tr.mapping` would re-apply the +// caller's earlier steps to positions that already account for them, and the +// resulting delete ranges would land on the wrong nodes. +describe("Test replaceBlocks on a transaction that already has steps", () => { + it("Removes the right blocks across two calls in one transaction", () => { + const editor = getEditor(); + const before = editor.document; + + editor.transact((tr) => { + removeAndInsertBlocks(tr, ["paragraph-0"], []); + removeAndInsertBlocks(tr, ["paragraph-2"], []); + }); + + expect(() => editor.prosemirrorState.doc.check()).not.toThrow(); + expect(editor.document).toEqual( + before.filter( + (block) => block.id !== "paragraph-0" && block.id !== "paragraph-2", + ), + ); + }); + + it("Removes the right block after the caller has already updated one", () => { + const editor = getEditor(); + const before = editor.document; + + editor.transact((tr) => { + // Changes the size of a block that sits before the one removed below, + // so the removal's positions are only correct if the earlier step is + // accounted for exactly once. + updateBlock(tr, "paragraph-0", { + type: "heading", + content: "Updated heading", + }); + const inserted: PartialBlock[] = [ + { id: "inserted-paragraph", type: "paragraph", content: "Inserted" }, + ]; + removeAndInsertBlocks(tr, ["paragraph-2"], inserted); + }); + + expect(() => editor.prosemirrorState.doc.check()).not.toThrow(); + + const updated = editor.getBlock("paragraph-0")!; + expect(updated.type).toBe("heading"); + expect(updated.content).toEqual([ + { type: "text", text: "Updated heading", styles: {} }, + ]); + + expect(editor.document.map((block) => block.id)).toEqual( + before.map((block) => + block.id === "paragraph-2" ? "inserted-paragraph" : block.id, + ), + ); + // Every block the two operations didn't target is left exactly as it was. + expect( + editor.document.filter( + (block) => + block.id !== "paragraph-0" && block.id !== "inserted-paragraph", + ), + ).toEqual( + before.filter( + (block) => block.id !== "paragraph-0" && block.id !== "paragraph-2", + ), + ); + }); +}); diff --git a/packages/core/src/api/blockManipulation/commands/replaceBlocks/replaceBlocks.ts b/packages/core/src/api/blockManipulation/commands/replaceBlocks/replaceBlocks.ts index 75305b501d..3b8a364c2c 100644 --- a/packages/core/src/api/blockManipulation/commands/replaceBlocks/replaceBlocks.ts +++ b/packages/core/src/api/blockManipulation/commands/replaceBlocks/replaceBlocks.ts @@ -53,7 +53,12 @@ export function removeAndInsertBlocks< typeof blocksToRemove[0] === "string" ? blocksToRemove[0] : blocksToRemove[0].id; - let removedSize = 0; + + // The walk below reads the document as it is now, but mutates it as it + // goes, so its positions go stale. `tr.mapping` already tracks exactly + // that; sliced from here so it ignores steps the caller added earlier. + const stepsBefore = tr.steps.length; + const mapPos = (pos: number) => tr.mapping.slice(stepsBefore).map(pos); tr.doc.descendants((node, pos) => { // Skips traversing nodes after all target blocks have been removed. @@ -77,16 +82,10 @@ export function removeAndInsertBlocks< idsOfBlocksToRemove.delete(nodeId); if (blocksToInsert.length > 0 && nodeId === idOfFirstBlock) { - const oldDocSize = tr.doc.nodeSize; - tr.insert(pos, nodesToInsert); - const newDocSize = tr.doc.nodeSize; - - removedSize += oldDocSize - newDocSize; + tr.insert(mapPos(pos), nodesToInsert); } - const oldDocSize = tr.doc.nodeSize; - - const $pos = tr.doc.resolve(pos - removedSize); + const $pos = tr.doc.resolve(mapPos(pos)); for (const container of getAncestorContainers($pos.doc, $pos.pos)) { if (!containersToFix.some((c) => c.id === container.id)) { @@ -94,22 +93,24 @@ export function removeAndInsertBlocks< } } + // When the block is the only child of a nested `blockGroup`, delete the + // group with it (`blockGroup` acting as a `min: 1, whenEmptied: "unwrap"` + // container). This can't route through `fixContainer`: repair runs after + // the delete, and by then ProseMirror's replace-fitting has padded the + // `blockGroupChild+` group with a fresh empty `blockContainer` + // indistinguishable from an intentional one. Only here, before the + // delete, is "this was the group's last child" still knowable. + const parent = $pos.node(); if ( - $pos.node().type.name === "blockGroup" && + parent.type.name === "blockGroup" && $pos.node($pos.depth - 1).type.name !== "doc" && - $pos.node().childCount === 1 + parent.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); + tr.delete($pos.pos, $pos.pos + node.nodeSize); } - const newDocSize = tr.doc.nodeSize; - removedSize += oldDocSize - newDocSize; - return false; }); diff --git a/packages/core/src/api/blockManipulation/commands/splitBlock/splitBlock.test.ts b/packages/core/src/api/blockManipulation/commands/splitBlock/splitBlock.test.ts index 9a83857cd1..eb57e39c85 100644 --- a/packages/core/src/api/blockManipulation/commands/splitBlock/splitBlock.test.ts +++ b/packages/core/src/api/blockManipulation/commands/splitBlock/splitBlock.test.ts @@ -3,11 +3,12 @@ import { TextSelection } from "prosemirror-state"; import { describe, expect, it } from "vite-plus/test"; import { - getBlockInfo, + getBlockInfoFromNode, getBlockInfoFromSelection, getNodeId, } from "../../../getBlockInfoFromPos.js"; import { getNodeById } from "../../../nodeUtil.js"; +import { containerSchema } from "../../containers/containers.fixture.js"; import { setupTestEnv } from "../../setupTestEnv.js"; import { splitBlockCommand } from "./splitBlock.js"; @@ -33,15 +34,15 @@ function setSelectionWithOffset( throw new Error(`Block with ID ${targetBlockId} not found`); } - const info = getBlockInfo(posInfo); + const info = getBlockInfoFromNode(posInfo.node, posInfo.posBeforeNode); - if (!info.isWrappedBlock) { + if (!info.hasContent) { throw new Error("Target block is not a block container"); } getEditor().transact((tr) => tr.setSelection( - TextSelection.create(doc, info.blockContent.beforePos + offset + 1), + TextSelection.create(doc, info.content.beforePos + offset + 1), ), ); } @@ -139,7 +140,7 @@ describe("Test splitBlocks", () => { splitBlock(getEditor().transact((tr) => tr.selection.anchor)); const blockId = getEditor().transact((tr) => - getNodeId(getBlockInfoFromSelection(tr).bnBlock.node, tr.doc), + getNodeId(getBlockInfoFromSelection(tr).block.node, tr.doc), ); const anchorIsAtStartOfNewBlock = @@ -149,3 +150,155 @@ describe("Test splitBlocks", () => { expect(anchorIsAtStartOfNewBlock).toBeTruthy(); }); }); + +// `splitBlockTr` splits two levels deep (`blockContent` and its +// `blockContainer`), which assumes the block's parent is a children holder that +// accepts another `blockContainer`. A container's children holder is a +// different node type than `blockGroup`, so these pin that the split lands +// inside the container rather than tearing it open. +describe("Test splitBlocks inside containers", () => { + const getContainerEditor = setupTestEnv({ + schema: containerSchema, + document: [ + { id: "before", type: "paragraph", content: "Before" }, + { + id: "callout-0", + type: "callout", + children: [ + { + id: "callout-child-0", + type: "paragraph", + content: "Callout child", + }, + { + id: "callout-child-1", + type: "heading", + content: "Callout heading", + children: [ + { + id: "nested-child", + type: "paragraph", + content: "Nested child", + }, + ], + }, + ], + }, + { + id: "grid-0", + type: "grid", + children: [ + { + id: "cell-0", + type: "gridCell", + children: [ + { id: "cell-0-p", type: "paragraph", content: "Cell zero" }, + ], + }, + { + id: "cell-1", + type: "gridCell", + children: [ + { id: "cell-1-p", type: "paragraph", content: "Cell one" }, + ], + }, + ], + }, + ], + }); + + function splitContainerBlock(blockId: string, offset: number) { + const editor = getContainerEditor(); + + const posInBlock = editor.transact((tr) => { + const posInfo = getNodeById(blockId, tr.doc); + if (!posInfo) { + throw new Error(`Block with ID ${blockId} not found`); + } + + const info = getBlockInfoFromNode(posInfo.node, posInfo.posBeforeNode); + + // A container has no content to offset into, so we aim at the node + // itself, which is where a `NodeSelection` on it would put the anchor. + return info.hasContent + ? info.content.beforePos + offset + 1 + : info.block.beforePos; + }); + + return editor._tiptapEditor.commands.command( + splitBlockCommand(posInBlock, true), + ); + } + + function textOf(block: { content?: any }) { + return (block.content as { text: string }[]).map((c) => c.text).join(""); + } + + it("Splits a block inside a container in place", () => { + expect(splitContainerBlock("callout-child-0", 7)).toBe(true); + + const document = getContainerEditor().document; + + expect(document.map((block) => block.id)).toEqual([ + "before", + "callout-0", + "grid-0", + ]); + + const callout = document[1]; + expect(callout.type).toBe("callout"); + expect(callout.children.map(textOf)).toEqual([ + "Callout", + " child", + "Callout heading", + ]); + + expect(() => + getContainerEditor().prosemirrorState.doc.check(), + ).not.toThrow(); + }); + + it("Moves the block's children onto the second half of the split", () => { + expect(splitContainerBlock("callout-child-1", 7)).toBe(true); + + const callout = getContainerEditor().document[1]; + + expect(callout.children.map(textOf)).toEqual([ + "Callout child", + "Callout", + " heading", + ]); + // The children follow the trailing half, as they do at the top level. + expect(callout.children[1].children).toEqual([]); + expect(callout.children[2].children.map((child) => child.id)).toEqual([ + "nested-child", + ]); + + expect(() => + getContainerEditor().prosemirrorState.doc.check(), + ).not.toThrow(); + }); + + it("Splits a block inside a nested container", () => { + expect(splitContainerBlock("cell-0-p", 4)).toBe(true); + + const grid = getContainerEditor().document[2]; + + expect(grid.type).toBe("grid"); + expect(grid.children.map((cell) => cell.id)).toEqual(["cell-0", "cell-1"]); + expect(grid.children[0].children.map(textOf)).toEqual(["Cell", " zero"]); + expect(grid.children[1].children.map(textOf)).toEqual(["Cell one"]); + + expect(() => + getContainerEditor().prosemirrorState.doc.check(), + ).not.toThrow(); + }); + + it("Does not split a container block itself", () => { + const before = getContainerEditor().document; + + expect(splitContainerBlock("callout-0", 0)).toBe(false); + + expect(getContainerEditor().document).toEqual(before); + }); +}); diff --git a/packages/core/src/api/blockManipulation/commands/splitBlock/splitBlock.ts b/packages/core/src/api/blockManipulation/commands/splitBlock/splitBlock.ts index ef74f8e898..d5229da6bf 100644 --- a/packages/core/src/api/blockManipulation/commands/splitBlock/splitBlock.ts +++ b/packages/core/src/api/blockManipulation/commands/splitBlock/splitBlock.ts @@ -1,7 +1,7 @@ import { EditorState, Transaction } from "prosemirror-state"; import { - getBlockInfo, + getBlockInfoFromNode, getNearestBlockPos, } from "../../../getBlockInfoFromPos.js"; import { getPmSchema } from "../../../pmUtil.js"; @@ -34,21 +34,24 @@ export const splitBlockTr = ( ): boolean => { const nearestBlockContainerPos = getNearestBlockPos(tr.doc, posInBlock); - const info = getBlockInfo(nearestBlockContainerPos); + const info = getBlockInfoFromNode( + nearestBlockContainerPos.node, + nearestBlockContainerPos.posBeforeNode, + ); - if (!info.isWrappedBlock) { + if (!info.hasContent) { return false; } const schema = getPmSchema(tr); const types = [ { - type: info.bnBlock.node.type, // always keep blockcontainer type - attrs: keepProps ? { ...info.bnBlock.node.attrs, id: undefined } : {}, + type: info.block.node.type, // always keep blockcontainer type + attrs: keepProps ? { ...info.block.node.attrs, id: undefined } : {}, }, { - type: keepType ? info.blockContent.node.type : schema.nodes["paragraph"], - attrs: keepProps ? { ...info.blockContent.node.attrs } : {}, + type: keepType ? info.content.node.type : schema.nodes["paragraph"], + attrs: keepProps ? { ...info.content.node.attrs } : {}, }, ]; diff --git a/packages/core/src/api/blockManipulation/commands/updateBlock/updateBlock.test.ts b/packages/core/src/api/blockManipulation/commands/updateBlock/updateBlock.test.ts index c695de98ae..f64d11d0c9 100644 --- a/packages/core/src/api/blockManipulation/commands/updateBlock/updateBlock.test.ts +++ b/packages/core/src/api/blockManipulation/commands/updateBlock/updateBlock.test.ts @@ -1,8 +1,9 @@ import { describe, expect, it } from "vite-plus/test"; import type { PartialBlock } from "../../../../blocks/defaultBlocks.js"; -import { getBlockInfo } from "../../../getBlockInfoFromPos.js"; +import { getBlockInfoFromNode } from "../../../getBlockInfoFromPos.js"; import { getNodeById } from "../../../nodeUtil.js"; +import { containerSchema } from "../../containers/containers.fixture.js"; import { setupTestEnv } from "../../setupTestEnv.js"; import { updateBlock } from "./updateBlock.js"; @@ -177,11 +178,13 @@ describe("Test updateBlock", () => { }); it("Update partial (offset start)", () => { - const info = getBlockInfo( - getNodeById("heading-with-everything", getEditor().prosemirrorState.doc)!, - ); + const posInfo = getNodeById( + "heading-with-everything", + getEditor().prosemirrorState.doc, + )!; + const info = getBlockInfoFromNode(posInfo.node, posInfo.posBeforeNode); - if (!info.isWrappedBlock) { + if (!info.hasContent) { throw new Error("heading-with-everything is not a block container"); } @@ -198,7 +201,7 @@ describe("Test updateBlock", () => { }, ], }, - info.blockContent.beforePos + 9, + info.content.beforePos + 9, ), ); @@ -206,11 +209,13 @@ describe("Test updateBlock", () => { }); it("Update partial (offset start + end)", () => { - const info = getBlockInfo( - getNodeById("heading-with-everything", getEditor().prosemirrorState.doc)!, - ); + const posInfo = getNodeById( + "heading-with-everything", + getEditor().prosemirrorState.doc, + )!; + const info = getBlockInfoFromNode(posInfo.node, posInfo.posBeforeNode); - if (!info.isWrappedBlock) { + if (!info.hasContent) { throw new Error("heading-with-everything is not a block container"); } @@ -227,8 +232,8 @@ describe("Test updateBlock", () => { }, ], }, - info.blockContent.beforePos + 9, - info.blockContent.beforePos + 9, + info.content.beforePos + 9, + info.content.beforePos + 9, ), ); @@ -236,11 +241,13 @@ describe("Test updateBlock", () => { }); it("Update partial (props + offset end)", () => { - const info = getBlockInfo( - getNodeById("heading-with-everything", getEditor().prosemirrorState.doc)!, - ); + const posInfo = getNodeById( + "heading-with-everything", + getEditor().prosemirrorState.doc, + )!; + const info = getBlockInfoFromNode(posInfo.node, posInfo.posBeforeNode); - if (!info.isWrappedBlock) { + if (!info.hasContent) { throw new Error("heading-with-everything is not a block container"); } @@ -261,7 +268,7 @@ describe("Test updateBlock", () => { ], }, undefined, - info.blockContent.beforePos + 8, + info.content.beforePos + 8, ); }); @@ -269,15 +276,14 @@ describe("Test updateBlock", () => { }); it("Update partial (table cell)", () => { - const info = getBlockInfo( - getNodeById("table-0", getEditor().prosemirrorState.doc)!, - ); + const posInfo = getNodeById("table-0", getEditor().prosemirrorState.doc)!; + const info = getBlockInfoFromNode(posInfo.node, posInfo.posBeforeNode); - if (!info.isWrappedBlock) { + if (!info.hasContent) { throw new Error("table-0 is not a block container"); } - const cell = info.blockContent.node.resolve(2); + const cell = info.content.node.resolve(2); getEditor().transact((tr) => updateBlock( @@ -290,8 +296,8 @@ describe("Test updateBlock", () => { rows: [{ cells: ["updated cell 1"] }], }, }, - info.blockContent.beforePos + 2, - info.blockContent.beforePos + 2 + cell.node().nodeSize, + info.content.beforePos + 2, + info.content.beforePos + 2 + cell.node().nodeSize, ), ); @@ -299,15 +305,14 @@ describe("Test updateBlock", () => { }); it("Update partial (table row)", () => { - const info = getBlockInfo( - getNodeById("table-0", getEditor().prosemirrorState.doc)!, - ); + const posInfo = getNodeById("table-0", getEditor().prosemirrorState.doc)!; + const info = getBlockInfoFromNode(posInfo.node, posInfo.posBeforeNode); - if (!info.isWrappedBlock) { + if (!info.hasContent) { throw new Error("table-0 is not a block container"); } - const cell = info.blockContent.node.resolve(1); + const cell = info.content.node.resolve(1); getEditor().transact((tr) => updateBlock( @@ -324,8 +329,8 @@ describe("Test updateBlock", () => { ], }, }, - info.blockContent.beforePos + 1, - info.blockContent.beforePos + 1 + cell.node().nodeSize, + info.content.beforePos + 1, + info.content.beforePos + 1 + cell.node().nodeSize, ), ); @@ -934,13 +939,12 @@ describe("Test updateBlock minimal steps", () => { it("Type change with offset content replace stays minimal and valid", () => { const editor = getEditor(); - const info = getBlockInfo( - getNodeById( - "paragraph-with-styled-content", - editor.prosemirrorState.doc, - )!, - ); - if (!info.isWrappedBlock) { + const posInfo = getNodeById( + "paragraph-with-styled-content", + editor.prosemirrorState.doc, + )!; + const info = getBlockInfoFromNode(posInfo.node, posInfo.posBeforeNode); + if (!info.hasContent) { throw new Error("paragraph-with-styled-content is not a block container"); } @@ -959,8 +963,8 @@ describe("Test updateBlock minimal steps", () => { props: { level: 3 }, content: [{ type: "text", text: " with NEW ", styles: {} }], }, - info.blockContent.beforePos + 1 + "Paragraph".length, - info.blockContent.beforePos + 1 + "Paragraph with styled ".length, + info.content.beforePos + 1 + "Paragraph".length, + info.content.beforePos + 1 + "Paragraph with styled ".length, ); steps = tr.steps.map((s) => s.toJSON()); }); @@ -976,3 +980,151 @@ describe("Test updateBlock minimal steps", () => { expect(() => editor._tiptapEditor.state.doc.check()).not.toThrow(); }); }); + +// Changing a block's type across the content/container divide can't happen in +// place, so `updateBlock` rebuilds the node and has to decide what to do with +// the content the old shape held and the new one can't. These tests pin that +// decision. Assertions are explicit rather than snapshotted because the point +// is *where* the carried content ends up. +describe("Test updateBlock content carry-over", () => { + const getContainerEditor = setupTestEnv({ + schema: containerSchema, + document: [ + { + id: "paragraph-with-text", + type: "paragraph", + content: "Paragraph with text", + }, + { + id: "empty-paragraph", + type: "paragraph", + }, + { + id: "paragraph-with-text-and-children", + type: "paragraph", + content: "Parent text", + children: [ + { + id: "existing-child", + type: "paragraph", + content: "Existing child", + }, + ], + }, + { + id: "table-0", + type: "table", + content: { + type: "tableContent", + rows: [{ cells: ["Cell 1", "Cell 2"] }], + }, + }, + { + id: "callout-0", + type: "callout", + children: [ + { + id: "callout-child", + type: "paragraph", + content: "Callout child", + }, + ], + }, + ], + }); + + // A block that changes shape is rebuilt rather than updated in place, and the + // rebuilt node is minted a fresh ID. That is long-standing behaviour, not + // something the container work introduced, but converting a paragraph into a + // container is a far more ordinary action than the paragraph/column + // conversions that used to be the only way to reach this path. These tests + // therefore address blocks by position, and the first one pins the ID loss so + // that fixing it shows up as a deliberate change. + it("Moves inline content into a child paragraph when becoming a container", () => { + const editor = getContainerEditor(); + editor.transact((tr) => + updateBlock(tr, "paragraph-with-text", { type: "callout" }), + ); + + const block = editor.document[0] as any; + expect(block.type).toBe("callout"); + expect(block.id).not.toBe("paragraph-with-text"); + expect(block.children).toHaveLength(1); + expect(block.children[0].type).toBe("paragraph"); + expect(block.children[0].content).toEqual([ + { type: "text", text: "Paragraph with text", styles: {} }, + ]); + expect(() => editor.prosemirrorState.doc.check()).not.toThrow(); + }); + + it("Seeds a container's default children when there is no content to carry", () => { + const editor = getContainerEditor(); + editor.transact((tr) => + updateBlock(tr, "empty-paragraph", { type: "seededPair" }), + ); + + // An empty paragraph carries nothing, so the rebuilt node must be passed no + // `children` at all: `blockToNode` seeds from the spec's `default` only + // when `children` is absent, and pads with empty blocks when it is an + // empty array. `seededPair` is used here rather than `callout` because its + // `default` and its padding differ — for `callout` both are one empty + // paragraph, so the distinction is invisible. + const block = editor.document[1] as any; + expect(block.type).toBe("seededPair"); + expect(block.children.map((child: any) => child.content[0]?.text)).toEqual([ + "Seed A", + "Seed B", + ]); + expect(() => editor.prosemirrorState.doc.check()).not.toThrow(); + }); + + it("Puts carried content before existing children", () => { + const editor = getContainerEditor(); + editor.transact((tr) => + updateBlock(tr, "paragraph-with-text-and-children", { type: "callout" }), + ); + + // The paragraph holding the carried text takes the place the text used to + // occupy, i.e. above the children that were already nested under it. + const block = editor.document[2] as any; + expect(block.type).toBe("callout"); + expect(block.children.map((child: any) => child.content[0].text)).toEqual([ + "Parent text", + "Existing child", + ]); + expect(block.children[1].id).toBe("existing-child"); + expect(() => editor.prosemirrorState.doc.check()).not.toThrow(); + }); + + it("Drops table content when becoming a container", () => { + const editor = getContainerEditor(); + // Table content isn't an inline array, so there is no sensible paragraph to + // wrap it in. It's dropped, and the container seeds as if the block had + // been empty. + expect(() => + editor.transact((tr) => updateBlock(tr, "table-0", { type: "callout" })), + ).not.toThrow(); + + const block = editor.document[3] as any; + expect(block.type).toBe("callout"); + expect(block.content).toBeUndefined(); + expect(block.children).toHaveLength(1); + expect(block.children[0].type).toBe("paragraph"); + expect(block.children[0].content).toEqual([]); + expect(() => editor.prosemirrorState.doc.check()).not.toThrow(); + }); + + it("Keeps a container's children and invents no content when becoming a block", () => { + const editor = getContainerEditor(); + editor.transact((tr) => + updateBlock(tr, "callout-0", { type: "paragraph" }), + ); + + const block = editor.document[4] as any; + expect(block.type).toBe("paragraph"); + expect(block.content).toEqual([]); + expect(block.children).toHaveLength(1); + expect(block.children[0].id).toBe("callout-child"); + expect(() => editor.prosemirrorState.doc.check()).not.toThrow(); + }); +}); diff --git a/packages/core/src/api/blockManipulation/commands/updateBlock/updateBlock.ts b/packages/core/src/api/blockManipulation/commands/updateBlock/updateBlock.ts index ad2cc151f3..4833cbdcbe 100644 --- a/packages/core/src/api/blockManipulation/commands/updateBlock/updateBlock.ts +++ b/packages/core/src/api/blockManipulation/commands/updateBlock/updateBlock.ts @@ -19,17 +19,17 @@ import type { StyleSchema } from "../../../../schema/styles/types.js"; import { UnreachableCaseError } from "../../../../util/typescript.js"; import { type BlockInfo, - getBlockInfoFromResolvedPos, + getBlockInfoAt, } from "../../../getBlockInfoFromPos.js"; +import { blockToNode } from "../../../nodeConversions/blockToNode.js"; import { - blockToNode, inlineContentToNodes, tableContentToNodes, } from "../../../nodeConversions/blockToNode.js"; import { nodeToBlock } from "../../../nodeConversions/nodeToBlock.js"; import { getNodeById } from "../../../nodeUtil.js"; import { getBlockSchema, getPmSchema } from "../../../pmUtil.js"; -import { isContainerType } from "../../../../schema/blocks/children.js"; +import { createBlockGroup } from "../../../../schema/blocks/children.js"; // for compatibility with tiptap. TODO: remove as we want to remove dependency on tiptap command interface export const updateBlockCommand = < @@ -65,7 +65,7 @@ export function updateBlockTr< replaceFromPos?: number, replaceToPos?: number, ) { - const blockInfo = getBlockInfoFromResolvedPos(tr.doc.resolve(posBeforeBlock)); + const blockInfo = getBlockInfoAt(tr.doc, posBeforeBlock); let cellAnchor: CellAnchor | null = null; if (blockInfo.blockNoteType === "table") { @@ -91,46 +91,26 @@ export function updateBlockTr< : pmSchema.nodes["blockContainer"]; const replaceFromOffset = - blockInfo.blockContent && + blockInfo.hasContent && replaceFromPos !== undefined && - replaceFromPos > blockInfo.blockContent.beforePos && - replaceFromPos < blockInfo.blockContent.afterPos - ? replaceFromPos - blockInfo.blockContent.beforePos - 1 + replaceFromPos >= blockInfo.contentStart && + replaceFromPos <= blockInfo.contentEnd + ? replaceFromPos - blockInfo.contentStart : undefined; const replaceToOffset = - blockInfo.blockContent && + blockInfo.hasContent && replaceToPos !== undefined && - replaceToPos > blockInfo.blockContent.beforePos && - replaceToPos < blockInfo.blockContent.afterPos - ? replaceToPos - blockInfo.blockContent.beforePos - 1 + replaceToPos >= blockInfo.contentStart && + replaceToPos <= blockInfo.contentEnd + ? replaceToPos - blockInfo.contentStart : undefined; - if ( - blockInfo.isWrappedBlock && - blockInfo.bnBlock.node.type.name === "blockContainer" && - newNodeType.isInGroup("blockContent") - ) { - updateChildren(block, tr, blockInfo); - // The code below determines the new content of the block. - // or "keep" to keep as-is - updateBlockContentNode( - block, - tr, - pmSchema.nodes[blockInfo.blockNoteType], - newNodeType, - blockInfo, - replaceFromOffset, - replaceToOffset, - ); - } else if ( - !blockInfo.isWrappedBlock && - newNodeType.isInGroup("bnBlock") - ) { - updateChildren(block, tr, blockInfo); - // old node was a bnBlock type (like column or columnList) and new block as well - // No op, we just update the bnBlock below (at end of function) and have already updated the children - } else { + // `hasContent` is exactly `blockContainer`-ness, and a block type resolves + // to either a `blockContent` node (a regular block) or a `bnBlock` one (a + // container), so the two together say whether the update keeps the block's + // shape. Only a same-shape update can happen in place. + if (blockInfo.hasContent !== newNodeType.isInGroup("blockContent")) { // switching from blockContainer to non-blockContainer or v.v. // currently breaking for column slash menu items converting empty block // to column. @@ -138,7 +118,7 @@ export function updateBlockTr< // currently, we calculate the new node and replace the entire node with the desired new node. // for this, we do a nodeToBlock on the existing block to get the children. // it would be cleaner to use a ReplaceAroundStep, but this is a bit simpler and it's quite an edge case - const existingBlock = nodeToBlock(blockInfo.bnBlock.node, tr.doc); + const existingBlock = nodeToBlock(blockInfo.block.node, tr.doc); const carried = carryOverContent( existingBlock.content, newBlockType, @@ -160,19 +140,35 @@ export function updateBlockTr< ); replacementNode.check(); // `blockToNode` is lenient; validate before mutating the doc tr.replaceWith( - blockInfo.bnBlock.beforePos, - blockInfo.bnBlock.afterPos, + blockInfo.block.beforePos, + blockInfo.block.afterPos, replacementNode, ); return; } + updateChildren(block, tr, blockInfo); + + if (blockInfo.hasContent) { + // The code below determines the new content of the block. + // or "keep" to keep as-is + updateBlockContentNode( + block, + tr, + pmSchema.nodes[blockInfo.blockNoteType], + newNodeType, + blockInfo, + replaceFromOffset, + replaceToOffset, + ); + } + // Adds all provided props as attributes to the parent blockContainer node too, and also preserves existing // attributes. Uses minimal steps so that an unchanged container (e.g. when // only children or content changed) doesn't emit a step at all. - setNodeMarkupMinimal(tr, blockInfo.bnBlock.beforePos, newBnBlockNodeType, { + setNodeMarkupMinimal(tr, blockInfo.block.beforePos, newBnBlockNodeType, { ...block.props, }); @@ -207,7 +203,7 @@ function carryOverContent( return { content: existingContent, children: [] }; } - if (isContainerType(targetConfig)) { + if (targetConfig.children !== undefined) { return { children: [{ type: "paragraph", content: existingContent } as any], }; @@ -226,10 +222,10 @@ function updateBlockContentNode< oldNodeType: NodeType, newNodeType: NodeType, blockInfo: { - childContainer?: + children?: | { node: PMNode; beforePos: number; afterPos: number } | undefined; - blockContent: { node: PMNode; beforePos: number; afterPos: number }; + content: { node: PMNode; beforePos: number; afterPos: number }; }, replaceFromOffset?: number, replaceToOffset?: number, @@ -259,8 +255,8 @@ function updateBlockContentNode< // no custom content has been provided, use existing content IF possible // Since some block types contain inline content and others don't, // we either need to call setNodeMarkup to just update type & - // attributes, or replaceWith to replace the whole blockContent. - const oldContent = blockInfo.blockContent.node.content; + // attributes, or replaceWith to replace the whole content. + const oldContent = blockInfo.content.node.content; if (oldNodeType.spec.content === "") { // keep old content, because it's empty anyway and should be compatible with // any newContentType @@ -275,7 +271,7 @@ function updateBlockContentNode< // for the new type (e.g. converting styled/complex inline content into a // plain block that disallows formatting marks and inline nodes). Preserve // the text, dropping the styling the new type can't represent. - const text = blockInfo.blockContent.node.textContent; + const text = blockInfo.content.node.textContent; content = text.length > 0 ? [pmSchema.text(text)] : []; } else { // the content type changed and is incompatible, replace the previous content @@ -283,7 +279,7 @@ function updateBlockContentNode< } } - // Now, changes the blockContent node type and adds the provided props + // Now, changes the content node type and adds the provided props // as attributes. Also preserves all existing attributes that are // compatible with the new type. // @@ -291,7 +287,7 @@ function updateBlockContentNode< // content is being replaced or not. if (content === "keep") { // only update the type and attributes, keeping the content as-is - setNodeMarkupMinimal(tr, blockInfo.blockContent.beforePos, newNodeType, { + setNodeMarkupMinimal(tr, blockInfo.content.beforePos, newNodeType, { ...block.props, }); } else if (replaceFromOffset !== undefined || replaceToOffset !== undefined) { @@ -299,7 +295,7 @@ function updateBlockContentNode< // position back. const contentBeforePos = setNodeMarkupMinimalAndRemap( tr, - blockInfo.blockContent.beforePos, + blockInfo.content.beforePos, newNodeType, { ...block.props }, ); @@ -308,7 +304,7 @@ function updateBlockContentNode< const end = contentBeforePos + 1 + - (replaceToOffset ?? blockInfo.blockContent.node.content.size); + (replaceToOffset ?? blockInfo.content.node.content.size); // for content like table cells (where the blockcontent has nested PM nodes), // we need to figure out the correct openStart and openEnd for the slice when replacing @@ -328,7 +324,7 @@ function updateBlockContentNode< ); } else if ( newNodeType === oldNodeType || - newNodeType.validContent(blockInfo.blockContent.node.content) + newNodeType.validContent(blockInfo.content.node.content) ) { // The new type can hold the existing content, so we can update the markup // first and then diff the content. This keeps both steps minimal. @@ -338,7 +334,7 @@ function updateBlockContentNode< // get its (possibly shifted) position back. const contentBeforePos = setNodeMarkupMinimalAndRemap( tr, - blockInfo.blockContent.beforePos, + blockInfo.content.beforePos, newNodeType, { ...block.props }, ); @@ -351,11 +347,11 @@ function updateBlockContentNode< // between inline content, table content, and no content). We can't update // the markup in-place, so replace the whole content node atomically. tr.replaceWith( - blockInfo.blockContent.beforePos, - blockInfo.blockContent.afterPos, + blockInfo.content.beforePos, + blockInfo.content.afterPos, newNodeType.createChecked( { - ...blockInfo.blockContent.node.attrs, + ...blockInfo.content.node.attrs, ...block.props, }, content, @@ -568,24 +564,23 @@ function updateChildren< return node; }); - // Checks if a blockGroup node already exists. - if (blockInfo.childContainer) { - // Replaces the child nodes in the existing blockGroup, only touching the - // range that actually changed (keeping unchanged leading/trailing - // children untouched). + if (blockInfo.children) { + // Replaces the child nodes in the existing children holder, only + // touching the range that actually changed (keeping unchanged + // leading/trailing children untouched). replaceContentMinimal( tr, - blockInfo.childContainer.beforePos, + blockInfo.children.beforePos, Fragment.from(childNodes), ); - } else { - if (!blockInfo.isWrappedBlock) { - throw new Error("impossible"); - } - // Inserts a new blockGroup containing the child nodes created earlier. + } else if (blockInfo.hasContent) { + // A `blockContainer` with no children yet: its `blockGroup` is lazy + // (`blockContent blockGroup?`), so create it around the child nodes and + // insert it after the content node. (Containers always have a children + // holder, so no holder implies a `blockContainer`.) tr.insert( - blockInfo.blockContent.afterPos, - pmSchema.nodes["blockGroup"].createChecked({}, childNodes), + blockInfo.content.afterPos, + createBlockGroup(pmSchema, childNodes), ); } } @@ -617,11 +612,14 @@ export function updateBlock< replaceToPos, ); - const blockContainerNode = tr.doc - .resolve(posInfo.posBeforeNode + 1) // TODO: clean? - .node(); + // `updateBlockTr` may have replaced the node, so re-resolve it at the same + // position (an update never moves the block). + const updatedNode = tr.doc.resolve(posInfo.posBeforeNode).nodeAfter; + if (!updatedNode) { + throw new Error(`Block with ID ${id} not found after update`); + } - return nodeToBlock(blockContainerNode, tr.doc); + return nodeToBlock(updatedNode, tr.doc); } type CellAnchor = { row: number; col: number; offset: number }; @@ -695,12 +693,12 @@ function restoreCellAnchor( // 1) Resolve the table node in the current document let tablePos = -1; - if (blockInfo.isWrappedBlock) { - // Prefer the blockContent position when available (points directly at the PM table node) - tablePos = tr.mapping.map(blockInfo.blockContent.beforePos); + if (blockInfo.hasContent) { + // Prefer the content position when available (points directly at the PM table node) + tablePos = tr.mapping.map(blockInfo.content.beforePos); } else { - // Fallback: scan within the mapped bnBlock range to find the inner table node - const start = tr.mapping.map(blockInfo.bnBlock.beforePos); + // Fallback: scan within the mapped block range to find the inner table node + const start = tr.mapping.map(blockInfo.block.beforePos); const end = start + (tr.doc.nodeAt(start)?.nodeSize || 0); tr.doc.nodesBetween(start, end, (node, pos) => { if (node.type.name === "table") { diff --git a/packages/core/src/api/blockManipulation/containers/containerNav.ts b/packages/core/src/api/blockManipulation/containers/containerNav.ts index 211fee190c..6e5cb87e27 100644 --- a/packages/core/src/api/blockManipulation/containers/containerNav.ts +++ b/packages/core/src/api/blockManipulation/containers/containerNav.ts @@ -1,78 +1,94 @@ import type { Node, NodeType } from "prosemirror-model"; import { isContainerNode, isSealed } from "../../../schema/blocks/children.js"; +import { + type BlockInfo, + getBlockInfoFromNode, +} from "../../getBlockInfoFromPos.js"; /** - * Seal handling for the navigation helpers below. By default the helpers - * ignore seals. The block manipulation API crosses them freely, since an - * explicit placement is an intentional crossing. Gesture code (keyboard - * merges and moves) opts in with `respectSealed`, so content never - * implicitly crosses a sealed boundary. + * The outcome of a descent. `blockedBy` says why no position was found: a + * sealed container on the path ("seal"), or nothing on that edge accepting + * the type ("schema"). Gesture code tells the two apart to select a sealed + * container rather than move content into it. */ -type SealOpts = { respectSealed?: boolean }; +export type InsertionPos = + | { pos: number; blockedBy?: undefined } + | { pos?: undefined; blockedBy: "seal" | "schema" }; -export function descendToLastInsertionPos( - container: Node, - containerBeforePos: number, +/** + * Seal handling for the navigation helpers below. They respect seals, so + * content never implicitly crosses a sealed boundary. The block manipulation + * API opts out with `allowCrossingSeals`, since an explicit placement is an + * intentional crossing. + */ +type SealOpts = { allowCrossingSeals?: boolean }; + +/** + * Walks one edge of a block's children, descending through nested containers, + * to the deepest position where `nodeType` fits. `edge` picks the trailing + * edge (where a new last child goes) or the leading edge. + */ +export function descendToInsertionPos( + info: BlockInfo, nodeType: NodeType, + edge: "first" | "last", opts?: SealOpts, -): number | null { - if (opts?.respectSealed && isSealed(container)) { - return null; - } - const endPos = containerBeforePos + 1 + container.content.size; - if (container.contentMatchAt(container.childCount).matchType(nodeType)) { - return endPos; +): InsertionPos { + const children = info.children; + if (!children) { + return { blockedBy: "schema" }; } - const lastChild = container.lastChild; - if (lastChild && isContainerNode(lastChild.type)) { - return descendToLastInsertionPos( - lastChild, - endPos - lastChild.nodeSize, - nodeType, - opts, - ); + if (!opts?.allowCrossingSeals && isSealed(children.node)) { + return { blockedBy: "seal" }; } - return null; -} -// No seal handling: its only callers are API code, which crosses seals by -// construction. -export function descendToFirstInsertionPos( - container: Node, - containerBeforePos: number, - nodeType: NodeType, -): number | null { - const startPos = containerBeforePos + 1; - if (container.contentMatchAt(0).matchType(nodeType)) { - return startPos; + const last = edge === "last"; + const index = last ? children.node.childCount : 0; + // `canReplaceWith` rather than a bare content match: the children already + // after the position have to still fit once the new node is spliced in. + if (children.node.canReplaceWith(index, index, nodeType)) { + return { pos: last ? children.childrenEnd : children.childrenStart }; } - const firstChild = container.firstChild; - if (firstChild && isContainerNode(firstChild.type)) { - return descendToFirstInsertionPos(firstChild, startPos, nodeType); + + const child = last ? children.node.lastChild : children.node.firstChild; + if (!child || !isContainerNode(child.type)) { + return { blockedBy: "schema" }; } - return null; + return descendToInsertionPos( + getBlockInfoFromNode( + child, + last ? children.childrenEnd - child.nodeSize : children.childrenStart, + ), + nodeType, + edge, + opts, + ); } -export function getFirstLeafBlock( - container: Node, - containerBeforePos: number, - opts?: SealOpts, -): { node: Node; beforePos: number } | null { - // With `respectSealed`, a sealed container's leaf blocks are not reachable - // from outside. - if (opts?.respectSealed && isSealed(container)) { +/** + * Resolves a block to its first leaf block: the block itself when it is not a + * container, otherwise the first leaf of its first child. Returns `null` for + * an empty container, or when reaching the leaf would cross a sealed + * container's boundary. + */ +export function getFirstLeafBlock(info: BlockInfo): BlockInfo | null { + const children = info.children; + if (!children || !isContainerNode(info.block.node.type)) { + // Not a container: the block is its own first leaf. + return info; + } + // A sealed container's leaf blocks are not reachable from outside. + if (isSealed(info.block.node)) { return null; } - const firstChild = container.firstChild; + const firstChild = children.node.firstChild; if (!firstChild) { return null; } - const firstChildBeforePos = containerBeforePos + 1; - if (isContainerNode(firstChild.type)) { - return getFirstLeafBlock(firstChild, firstChildBeforePos, opts); - } - return { node: firstChild, beforePos: firstChildBeforePos }; + return getFirstLeafBlock( + getBlockInfoFromNode(firstChild, children.childrenStart), + ); } /** @@ -80,33 +96,43 @@ export function getFirstLeafBlock( * `side` picks which edge of each climbed container to land on: `"before"` for * moves that put a block above the containers it leaves (Backspace move-out), * `"after"` for moves that put it below them (Enter-exit). + * + * Position-based rather than `BlockInfo`-based (unlike the descend/leaf + * helpers above) because its input is an arbitrary gap position — a point + * between blocks, not a block. */ export function ascendToInsertablePos( doc: Node, pos: number, nodeType: NodeType, - opts?: SealOpts, side: "before" | "after" = "before", -): number | null { +): number | undefined { for (;;) { const $pos = doc.resolve(pos); const parent = $pos.node(); - if (parent.contentMatchAt($pos.index()).matchType(nodeType)) { + if (parent.canReplaceWith($pos.index(), $pos.index(), nodeType)) { return pos; } if ($pos.depth > 0 && isContainerNode(parent.type)) { - // With `respectSealed`, climbing out of a sealed container would move - // content across its boundary. - if (opts?.respectSealed && isSealed(parent)) { - return null; + // Climbing out of a sealed container would move content across its + // boundary. + if (isSealed(parent)) { + return undefined; } pos = side === "before" ? $pos.before() : $pos.after(); continue; } - return null; + return undefined; } } +/** + * The container ancestors of a position, outermost last, each with its block + * id and resolution depth. Used to re-run container repair (`fixContainersById`) + * on every container a mutation may have emptied. Position-based for the same + * reason as `ascendToInsertablePos`: selections and mapped positions are the + * natural inputs. + */ export function getAncestorContainers( doc: Node, pos: number, diff --git a/packages/core/src/api/blockManipulation/containers/containerUI.ts b/packages/core/src/api/blockManipulation/containers/containerUI.ts index b4633d5ead..ad17e8381f 100644 --- a/packages/core/src/api/blockManipulation/containers/containerUI.ts +++ b/packages/core/src/api/blockManipulation/containers/containerUI.ts @@ -1,5 +1,4 @@ import type { BlockNoteEditor } from "../../../editor/BlockNoteEditor.js"; -import { isContainerType } from "../../../schema/blocks/children.js"; export type ContainerUIInfo = { containerTypes: ReadonlySet; @@ -22,9 +21,19 @@ function buildSelector(types: ReadonlySet): string | null { return [...types].map((type) => `[data-node-type="${type}"]`).join(","); } +// The schema never changes over an editor's lifetime, so the info is derived +// once. It's read on every mousemove, which would otherwise walk every block +// spec each time. +const cache = new WeakMap(); + export function getContainerUIInfo( editor: Pick, "schema">, ): ContainerUIInfo { + const cached = cache.get(editor.schema); + if (cached) { + return cached; + } + const containerTypes = new Set(); const draggableContainerTypes = new Set(); const nonDraggableBlockTypes = new Set(); @@ -40,7 +49,7 @@ export function getContainerUIInfo( )) { const draggable = spec.implementation?.meta?.draggable !== false; - if (!isContainerType(spec.config)) { + if (spec.config.children === undefined) { if (!draggable) { nonDraggableBlockTypes.add(type); } @@ -52,10 +61,12 @@ export function getContainerUIInfo( } } - return { + const info: ContainerUIInfo = { containerTypes, draggableContainerTypes, nonDraggableBlockTypes, containerSelector: buildSelector(containerTypes), }; + cache.set(editor.schema, info); + return info; } diff --git a/packages/core/src/api/blockManipulation/containers/containers.fixture.ts b/packages/core/src/api/blockManipulation/containers/containers.fixture.ts index f6149f65b5..934f5a727c 100644 --- a/packages/core/src/api/blockManipulation/containers/containers.fixture.ts +++ b/packages/core/src/api/blockManipulation/containers/containers.fixture.ts @@ -37,8 +37,8 @@ const SealedBox = createBlockSpec( { render: renderDiv }, )(); -// An open container, like a column list. Everything crosses its edge -// (PM `isolating: false`). +// An open container, like a column list: editing gestures cross its edge. +// Same as leaving `boundary` out, spelled explicitly. const OpenBox = createBlockSpec( { type: "openBox" as const, @@ -49,8 +49,8 @@ const OpenBox = createBlockSpec( { render: renderDiv }, )(); -// Same shape as tables: an isolated container (the default) that holds only -// sealed ones, so any descent into it bottoms out at a sealed boundary. +// Same shape as tables: a container that holds only sealed ones, so any +// descent into it bottoms out at a sealed boundary. const SealedGrid = createBlockSpec( { type: "sealedGrid" as const, diff --git a/packages/core/src/api/blockManipulation/containers/containers.test.ts b/packages/core/src/api/blockManipulation/containers/containers.test.ts index 405c22f8a5..b5c06522e8 100644 --- a/packages/core/src/api/blockManipulation/containers/containers.test.ts +++ b/packages/core/src/api/blockManipulation/containers/containers.test.ts @@ -1,4 +1,5 @@ // @vitest-environment node +import { TextSelection } from "prosemirror-state"; import { afterAll, beforeAll, @@ -9,6 +10,8 @@ import { } from "vite-plus/test"; import { BlockNoteEditor } from "../../../editor/BlockNoteEditor.js"; +import { getParentBlockInfo } from "../../getBlockInfoFromPos.js"; +import { getNodeById } from "../../nodeUtil.js"; import { containerSchema } from "./containers.fixture.js"; type PartialBlock = (typeof containerSchema)["PartialBlock"]; @@ -205,14 +208,58 @@ describe("children insertion & seeding", () => { }); describe("boundary", () => { - it("derives ProseMirror `isolating` from `boundary`", () => { + // No container is `isolating`, whatever its boundary. PM only honours that + // flag while no selection spans the edge, and nothing prevents one: given a + // spanning slice, `Fitter` refuses to open into the container and wraps the + // content in a spurious `blockGroup`, corrupting the document. Seals are + // enforced by BlockNote's own `isSealed` guards instead. + it("leaves every container non-isolating, seal or no seal", () => { const nodes = editor.pmSchema.nodes; - expect(nodes["openBox"].spec.isolating).toBe(false); - // "isolated" is the default. - expect(nodes["callout"].spec.isolating).toBe(true); - // "sealed" also isolates. - expect(nodes["sealedBox"].spec.isolating).toBe(true); + for (const type of ["openBox", "callout", "sealedBox", "gridCell"]) { + expect(nodes[type].spec.isolating).toBeFalsy(); + } }); + + // The corruption the line above avoids, pinned end to end: copy a selection + // running from inside a container to after it, paste it back over itself, + // and the document must come back unchanged. Marking the container + // `isolating` instead re-nests the whole fragment a level too deep. + it.each(["openBox", "callout", "sealedBox"])( + "round-trips a paste across a %s's edge", + (type) => { + editor.replaceBlocks(editor.document, [ + { + id: "c", + type, + children: [ + { id: "c1", type: "paragraph", content: "Inner one" }, + { id: "c2", type: "paragraph", content: "Inner two" }, + ], + }, + { id: "a", type: "paragraph", content: "After" }, + ] as PartialBlock[]); + + const before = JSON.stringify(editor.document); + + editor.transact((tr) => { + let from = 0; + let to = 0; + tr.doc.descendants((node, pos) => { + if (node.isText && node.text === "Inner two") { + from = pos; + } + if (node.isText && node.text === "After") { + to = pos + node.nodeSize; + } + }); + + const selection = TextSelection.create(tr.doc, from, to); + tr.setSelection(selection).replace(from, to, selection.content()); + }); + + expect(JSON.stringify(editor.document)).toBe(before); + }, + ); }); // `initialContent` is the only path that builds a document without validating @@ -320,6 +367,96 @@ describe("children repair", () => { "trailing", ]); }); + + // Unlike `refillContainer` (which leaves empty children alone at or above + // `min` — they may be intentional), the unwrap repair drops emptied + // children unconditionally: an emptied third column disappears rather than + // lingering, even though the list stays valid without unwrapping. The + // multicolumn e2e snapshots pin the same behavior from the keyboard side. + it("drops emptied children of an unwrap container even at or above `min`", () => { + editor.replaceBlocks(editor.document, [ + { + type: "grid", + id: "g-0", + children: [ + { + type: "gridCell", + id: "cell-a", + children: [ + { id: "cell-a-p", type: "paragraph", content: "A" }, + { id: "cell-a-extra", type: "paragraph", content: "A2" }, + ], + }, + { + type: "gridCell", + id: "cell-b", + children: [{ id: "cell-b-p", type: "paragraph", content: "B" }], + }, + { + type: "gridCell", + id: "cell-c", + children: [{ id: "cell-c-p", type: "paragraph", content: "" }], + }, + ], + }, + { id: "trailing", type: "paragraph", content: "" }, + ]); + + // Removing a block inside cell A runs repair on the grid; the emptied + // cell C is dropped, and with cells A and B still meeting `min: 2` the + // grid itself survives. + editor.removeBlocks(["cell-a-extra"]); + + const grid = editor.getBlock("g-0")!; + expect(grid.children.map((cell) => cell.id)).toEqual(["cell-a", "cell-b"]); + }); +}); + +describe("parent lookups for container children", () => { + // Regression: `getParentBlockInfo` used to skip the container level for + // container children (returning the grid for a block inside a gridCell), + // which made the Delete-at-end climb run its sealed-container check on the + // wrong node. The parent of a block is the block whose `children` contains + // it: the cell. + it("returns the container as the parent of its direct children", () => { + editor.replaceBlocks(editor.document, [ + { + type: "grid", + id: "g-0", + children: [ + { + type: "gridCell", + id: "cell-a", + children: [{ id: "cell-a-p", type: "paragraph", content: "A" }], + }, + { + type: "gridCell", + id: "cell-b", + children: [{ id: "cell-b-p", type: "paragraph", content: "B" }], + }, + ], + }, + { id: "trailing", type: "paragraph", content: "" }, + ]); + + editor.transact((tr) => { + // The block directly containing a cell's paragraph is the cell. + const cellChild = getNodeById("cell-a-p", tr.doc)!; + expect( + getParentBlockInfo(tr.doc, cellChild.posBeforeNode)?.blockNoteType, + ).toBe("gridCell"); + + // The parent of a cell is the grid; the parent of the grid (a + // top-level block) is undefined. + const cell = getNodeById("cell-a", tr.doc)!; + expect( + getParentBlockInfo(tr.doc, cell.posBeforeNode)?.blockNoteType, + ).toBe("grid"); + + const grid = getNodeById("g-0", tr.doc)!; + expect(getParentBlockInfo(tr.doc, grid.posBeforeNode)).toBeUndefined(); + }); + }); }); describe("children selection", () => { diff --git a/packages/core/src/api/blockManipulation/containers/fixContainer.ts b/packages/core/src/api/blockManipulation/containers/fixContainer.ts index 325e0622d2..b2bffa3848 100644 --- a/packages/core/src/api/blockManipulation/containers/fixContainer.ts +++ b/packages/core/src/api/blockManipulation/containers/fixContainer.ts @@ -1,16 +1,20 @@ -import { Fragment, Slice, type Node } from "prosemirror-model"; +import { Fragment, Slice, type Node, type NodeType } from "prosemirror-model"; import { type Transaction } from "prosemirror-state"; import { ReplaceAroundStep } from "prosemirror-transform"; -import type { Schema } from "prosemirror-model"; import { - BLOCK_GROUP_CHILD_GROUP, - getChildrenConfig, + type BlockInfo, + getBlockInfoFromNode, +} from "../../getBlockInfoFromPos.js"; + +import { + isBlockGroupInsertable, isContainerNode, resolveChildren, } from "../../../schema/blocks/children.js"; import type { ResolvedChildren } from "../../../schema/blocks/children.js"; -import { seedRefillChildren } from "../../nodeConversions/blockToNode.js"; +import type { PartialBlock } from "../../../blocks/defaultBlocks.js"; +import { blockToNode } from "../../nodeConversions/blockToNode.js"; import { getNodeById } from "../../nodeUtil.js"; // Defined in `children.ts` (it answers a schema-level question); re-exported @@ -42,75 +46,56 @@ export function removeEmptyChildren(tr: Transaction, containerPos: number) { ); } - for ( - let childIndex = container.childCount - 1; - childIndex >= 0; - childIndex-- - ) { - const childPos = tr.doc.resolve(containerPos + 1).posAtIndex(childIndex); - const child = tr.doc.resolve(childPos).nodeAfter; - if (!child) { - throw new Error("Invalid childPos: does not point to a child node."); - } - + // Collected before deleting anything, so every position is taken from the + // same (untouched) container, then applied back to front so the earlier + // ones stay valid. + const emptyChildren: { from: number; to: number }[] = []; + container.forEach((child, offset) => { if (isEmptyContainerChild(child)) { - tr.delete(childPos, childPos + child.nodeSize); + const from = containerPos + 1 + offset; + emptyChildren.push({ from, to: from + child.nodeSize }); } - } -} - -function isInsertableChild(node: Node): boolean { - return ( - node.type.name === "blockContainer" || - node.type.isInGroup(BLOCK_GROUP_CHILD_GROUP) - ); -} - -type ContainerRepairTarget = { - blockPos: number; - blockNode: Node; -}; + }); -function getContainerRepairTarget( - doc: Node, - containerPos: number, -): ContainerRepairTarget | undefined { - const node = doc.resolve(containerPos).nodeAfter; - if (!node || !isContainerNode(node.type)) { - return undefined; + for (let i = emptyChildren.length - 1; i >= 0; i--) { + tr.delete(emptyChildren[i].from, emptyChildren[i].to); } - - return { blockPos: containerPos, blockNode: node }; } /** - * The (possibly rebuilt) block at the repair target, with where its children - * now start. Recomputed after each mutation of `tr`. + * The container's BlockInfo at `containerPos` in `tr`'s current doc, or + * `undefined` when the node there is gone or no longer of `type`. */ -function refreshRepairTarget( +function getContainerInfo( tr: Transaction, - target: ContainerRepairTarget, -): { children: Node; childrenStart: number } | undefined { - const refreshedBlock = tr.doc.resolve(target.blockPos).nodeAfter; - if (!refreshedBlock || refreshedBlock.type !== target.blockNode.type) { + containerPos: number, + type: NodeType, +): Extract | undefined { + const node = tr.doc.resolve(containerPos).nodeAfter; + if (!node || node.type !== type) { return undefined; } - - return { children: refreshedBlock, childrenStart: target.blockPos + 1 }; + const info = getBlockInfoFromNode(node, containerPos); + if (info.hasContent) { + // `type` is a container node type, so its BlockInfo always takes the + // no-content arm. + throw new Error( + `Container node "${type.name}" unexpectedly resolved with a content node.`, + ); + } + return info; } export function fixContainer(tr: Transaction, containerPos: number) { - const target = getContainerRepairTarget(tr.doc, containerPos); - if (!target) { + const node = tr.doc.resolve(containerPos).nodeAfter; + if (!node || !isContainerNode(node.type)) { throw new Error( "Invalid containerPos: does not point to a container node.", ); } - const blockConfig = target.blockNode.type.spec.blockConfig; - const childrenConfig = blockConfig - ? getChildrenConfig(blockConfig) - : undefined; + const blockConfig = node.type.spec.blockConfig; + const childrenConfig = blockConfig?.children; const config = childrenConfig ? resolveChildren(childrenConfig) : undefined; if (!config) { @@ -118,28 +103,33 @@ export function fixContainer(tr: Transaction, containerPos: number) { } if (config.whenEmptied === "unwrap") { - unwrapContainer(tr, target, config); + unwrapContainer(tr, containerPos, node.type, config); } else { - // `blockConfig` is set whenever `config` is. - refillContainer(tr, target, config, blockConfig!.type); + refillContainer(tr, containerPos, node.type, config); } } function unwrapContainer( tr: Transaction, - target: ContainerRepairTarget, + containerPos: number, + type: NodeType, config: ResolvedChildren, ) { - removeEmptyChildren(tr, target.blockPos); - - const refreshed = refreshRepairTarget(tr, target); - if (!refreshed) { + // Emptied children are dropped unconditionally, even when the container + // sits at or above `min` afterwards: for an unwrap container (a + // columnList), a child the user emptied is done for — an emptied third + // column disappears rather than lingering. This deliberately differs from + // `refillContainer`, which leaves empty children alone at or above `min`. + removeEmptyChildren(tr, containerPos); + + const info = getContainerInfo(tr, containerPos, type); + if (!info) { return; } - const { children: refreshedChildren, childrenStart } = refreshed; + const { childrenStart } = info.children; const nonEmptyChildren: { child: Node; offset: number }[] = []; - refreshedChildren.forEach((child, offset) => { + info.children.node.forEach((child, offset) => { if (!isEmptyContainerChild(child)) { nonEmptyChildren.push({ child, offset }); } @@ -149,11 +139,10 @@ function unwrapContainer( return; } - const refreshedBlock = tr.doc.resolve(target.blockPos).nodeAfter!; - const blockEnd = target.blockPos + refreshedBlock.nodeSize; + const blockEnd = info.block.afterPos; if (nonEmptyChildren.length === 0) { - tr.delete(target.blockPos, blockEnd); + tr.delete(info.block.beforePos, blockEnd); return; } @@ -162,13 +151,13 @@ function unwrapContainer( const { child, offset } = nonEmptyChildren[0]; const childStart = childrenStart + offset; - const [gapFrom, gapTo] = isInsertableChild(child) + const [gapFrom, gapTo] = isBlockGroupInsertable(child.type) ? [childStart, childStart + child.nodeSize] : [childStart + 1, childStart + child.nodeSize - 1]; tr.step( new ReplaceAroundStep( - target.blockPos, + info.block.beforePos, blockEnd, gapFrom, gapTo, @@ -183,13 +172,13 @@ function unwrapContainer( // Several survivors but still below `min`: rebuild replacement content. const replacement: Node[] = []; for (const { child } of nonEmptyChildren) { - if (isInsertableChild(child)) { + if (isBlockGroupInsertable(child.type)) { replacement.push(child); } else { child.forEach((grandChild) => replacement.push(grandChild)); } } - tr.replaceWith(target.blockPos, blockEnd, Fragment.from(replacement)); + tr.replaceWith(info.block.beforePos, blockEnd, Fragment.from(replacement)); } /** @@ -205,15 +194,15 @@ function unwrapContainer( */ function refillContainer( tr: Transaction, - target: ContainerRepairTarget, + containerPos: number, + type: NodeType, config: ResolvedChildren, - blockType: string, ) { - const current = refreshRepairTarget(tr, target); - if (!current) { + const info = getContainerInfo(tr, containerPos, type); + if (!info) { return; } - const { children, childrenStart } = current; + const { node: children, childrenStart, childrenEnd } = info.children; const survivors: Node[] = []; children.forEach((child) => { @@ -227,12 +216,15 @@ function refillContainer( return; } - const seeds = seedRefillChildren( - blockType, - tr.doc.type.schema, - survivors.length, - config.min, - ); + // The refill seeds are the unconsumed tail of the container's `default` + // (`default[survivors.length..min-1]`), each converted exactly like an + // inserted block. Empty when the container has no `default`; the remainder + // is padded with empty fill below. + const seeds = (config.default ?? []) + .slice(survivors.length, config.min) + .map((child) => + blockToNode(child as PartialBlock, tr.doc.type.schema), + ); if (seeds.length === 0) { // No `default` to seed from, so empty children are the right fill, and @@ -241,7 +233,7 @@ function refillContainer( const match = children.type.contentMatch.matchFragment(children.content); const fill = match?.fillBefore(Fragment.empty, true); if (fill && fill.size > 0) { - tr.insert(childrenStart + children.content.size, fill); + tr.insert(childrenEnd, fill); } return; } @@ -255,9 +247,17 @@ function refillContainer( content = content.append(fill); } - tr.replaceWith(childrenStart, childrenStart + children.content.size, content); + tr.replaceWith(childrenStart, childrenEnd, content); } +/** + * Runs `fixContainer` on each of the given containers, looked up by ID in + * `tr`'s current doc. Containers are repaired deepest-first so that an inner + * repair (e.g. a column emptying out) is observed by the outer container's + * repair (e.g. its columnList unwrapping) in the same pass. Containers that + * no longer exist by the time their turn comes are skipped — an earlier + * repair may have removed them. + */ export function fixContainersById( tr: Transaction, containers: { id: string; depth: number }[], @@ -272,28 +272,3 @@ export function fixContainersById( fixContainer(tr, target.posBeforeNode); }); } - -export function flattenNonInsertableBlocks< - T extends { type?: string; content?: unknown; children?: T[] }, ->(blocks: T[], pmSchema: Schema): T[] { - return blocks.flatMap((block) => { - const nodeType = block.type ? pmSchema.nodes[block.type] : undefined; - if ( - nodeType && - nodeType.isInGroup("bnBlock") && - !nodeType.isInGroup(BLOCK_GROUP_CHILD_GROUP) - ) { - const children = flattenNonInsertableBlocks( - block.children ?? [], - pmSchema, - ); - return Array.isArray(block.content) && block.content.length > 0 - ? [ - { type: "paragraph", content: block.content } as unknown as T, - ...children, - ] - : children; - } - return [block]; - }); -} diff --git a/packages/core/src/api/blockManipulation/getBlock/getBlock.ts b/packages/core/src/api/blockManipulation/getBlock/getBlock.ts index 9982402a4e..98fc3b7eef 100644 --- a/packages/core/src/api/blockManipulation/getBlock/getBlock.ts +++ b/packages/core/src/api/blockManipulation/getBlock/getBlock.ts @@ -6,6 +6,7 @@ import type { InlineContentSchema, StyleSchema, } from "../../../schema/index.js"; +import { getParentBlockInfo } from "../../getBlockInfoFromPos.js"; import { nodeToBlock } from "../../nodeConversions/nodeToBlock.js"; import { getNodeById } from "../../nodeUtil.js"; @@ -94,21 +95,10 @@ export function getParentBlock< return undefined; } - const $posBeforeNode = doc.resolve(posInfo.posBeforeNode); - const parentNode = $posBeforeNode.node(); - const grandparentNode = $posBeforeNode.node(-1); - // A block's children live in its parent's `blockGroup` (regular nesting), - // in which case the actual parent block is the grandparent. A container - // holds its children directly, so its own node is the parent. - const nodeToConvert = - grandparentNode.type.name !== "doc" - ? parentNode.type.name === "blockGroup" - ? grandparentNode - : parentNode - : undefined; - if (!nodeToConvert) { + const parentInfo = getParentBlockInfo(doc, posInfo.posBeforeNode); + if (!parentInfo) { return undefined; } - return nodeToBlock(nodeToConvert, doc); + return nodeToBlock(parentInfo.block.node, doc); } diff --git a/packages/core/src/api/blockManipulation/selections/selection.ts b/packages/core/src/api/blockManipulation/selections/selection.ts index 466845d94a..34591c8c8d 100644 --- a/packages/core/src/api/blockManipulation/selections/selection.ts +++ b/packages/core/src/api/blockManipulation/selections/selection.ts @@ -1,5 +1,4 @@ import { TextSelection, type Transaction } from "prosemirror-state"; -import { TableMap } from "prosemirror-tables"; import { Block } from "../../../blocks/defaultBlocks.js"; import { Selection } from "../../../editor/selectionTypes.js"; import { @@ -9,13 +8,16 @@ import { StyleSchema, } from "../../../schema/index.js"; import { expandPMRangeToWords } from "../../../util/expandToWords.js"; -import { getBlockInfo, getNearestBlockPos } from "../../getBlockInfoFromPos.js"; +import { + blockEdgePos, + getBlockInfoFromNode, + getNearestBlockPos, +} from "../../getBlockInfoFromPos.js"; import { nodeToBlock, prosemirrorSliceToSlicedBlocks, } from "../../nodeConversions/nodeToBlock.js"; import { getNodeById } from "../../nodeUtil.js"; -import { getBlockNoteSchema, getPmSchema } from "../../pmUtil.js"; export function getSelection< BSchema extends BlockSchema, @@ -140,8 +142,6 @@ export function setSelection( const startBlockId = typeof startBlock === "string" ? startBlock : startBlock.id; const endBlockId = typeof endBlock === "string" ? endBlock : endBlock.id; - const pmSchema = getPmSchema(tr); - const schema = getBlockNoteSchema(pmSchema); if (startBlockId === endBlockId) { throw new Error( @@ -157,59 +157,28 @@ export function setSelection( throw new Error(`Block with ID ${endBlockId} not found`); } - const anchorBlockInfo = getBlockInfo(anchorPosInfo); - const headBlockInfo = getBlockInfo(headPosInfo); - - const anchorBlockConfig = - schema.blockSchema[ - anchorBlockInfo.blockNoteType as keyof typeof schema.blockSchema - ]; - const headBlockConfig = - schema.blockSchema[ - headBlockInfo.blockNoteType as keyof typeof schema.blockSchema - ]; + const anchorBlockInfo = getBlockInfoFromNode( + anchorPosInfo.node, + anchorPosInfo.posBeforeNode, + ); + const headBlockInfo = getBlockInfoFromNode( + headPosInfo.node, + headPosInfo.posBeforeNode, + ); - if (!anchorBlockInfo.isWrappedBlock || anchorBlockConfig.content === "none") { + const startPos = blockEdgePos(anchorBlockInfo, "start"); + if (startPos === null) { throw new Error( `Attempting to set selection anchor in block without content (id ${startBlockId})`, ); } - if (!headBlockInfo.isWrappedBlock || headBlockConfig.content === "none") { + const endPos = blockEdgePos(headBlockInfo, "end"); + if (endPos === null) { throw new Error( - `Attempting to set selection anchor in block without content (id ${endBlockId})`, + `Attempting to set selection head in block without content (id ${endBlockId})`, ); } - let startPos: number; - let endPos: number; - - if (anchorBlockConfig.content === "table") { - const tableMap = TableMap.get(anchorBlockInfo.blockContent.node); - const firstCellPos = - anchorBlockInfo.blockContent.beforePos + - tableMap.positionAt(0, 0, anchorBlockInfo.blockContent.node) + - 1; - startPos = firstCellPos + 2; - } else { - startPos = anchorBlockInfo.blockContent.beforePos + 1; - } - - if (headBlockConfig.content === "table") { - const tableMap = TableMap.get(headBlockInfo.blockContent.node); - const lastCellPos = - headBlockInfo.blockContent.beforePos + - tableMap.positionAt( - tableMap.height - 1, - tableMap.width - 1, - headBlockInfo.blockContent.node, - ) + - 1; - const lastCellNodeSize = tr.doc.resolve(lastCellPos).nodeAfter!.nodeSize; - endPos = lastCellPos + lastCellNodeSize - 2; - } else { - endPos = headBlockInfo.blockContent.afterPos - 1; - } - // TODO: We should polish up the `MultipleNodeSelection` and use that instead. // Right now it's missing a few things like a jsonID and styling to show // which nodes are selected. `TextSelection` is ok for now, but has the diff --git a/packages/core/src/api/blockManipulation/selections/textCursorPosition.ts b/packages/core/src/api/blockManipulation/selections/textCursorPosition.ts index 38ad256457..0130af1992 100644 --- a/packages/core/src/api/blockManipulation/selections/textCursorPosition.ts +++ b/packages/core/src/api/blockManipulation/selections/textCursorPosition.ts @@ -1,9 +1,4 @@ -import type { Node } from "prosemirror-model"; -import { - NodeSelection, - TextSelection, - type Transaction, -} from "prosemirror-state"; +import { type Transaction } from "prosemirror-state"; import type { TextCursorPosition } from "../../../editor/cursorPositionTypes.js"; import type { BlockIdentifier, @@ -11,43 +6,34 @@ import type { InlineContentSchema, StyleSchema, } from "../../../schema/index.js"; -import { UnreachableCaseError } from "../../../util/typescript.js"; import { - getBlockInfo, + blockEdgeSelection, + getBlockInfoFromNode, getBlockInfoFromSelection, - getNodeId, + getParentBlockInfo, } from "../../getBlockInfoFromPos.js"; import { nodeToBlock } from "../../nodeConversions/nodeToBlock.js"; import { getNodeById } from "../../nodeUtil.js"; -import { getBlockNoteSchema, getPmSchema } from "../../pmUtil.js"; export function getTextCursorPosition< BSchema extends BlockSchema, I extends InlineContentSchema, S extends StyleSchema, >(tr: Transaction): TextCursorPosition { - const { bnBlock } = getBlockInfoFromSelection(tr); + const { block } = getBlockInfoFromSelection(tr); - const resolvedPos = tr.doc.resolve(bnBlock.beforePos); + const resolvedPos = tr.doc.resolve(block.beforePos); // Gets previous blockContainer node at the same nesting level, if the current node isn't the first child. const prevNode = resolvedPos.nodeBefore; // Gets next blockContainer node at the same nesting level, if the current node isn't the last child. - const nextNode = tr.doc.resolve(bnBlock.afterPos).nodeAfter; + const nextNode = tr.doc.resolve(block.afterPos).nodeAfter; - // Gets parent blockContainer node, if the current node is nested. - let parentNode: Node | undefined = undefined; - if (resolvedPos.depth > 1) { - // for nodes nested in bnBlocks - parentNode = resolvedPos.node(); - if (!parentNode.type.isInGroup("bnBlock")) { - // for blockGroups, we need to go one level up - parentNode = resolvedPos.node(resolvedPos.depth - 1); - } - } + // Gets the parent block's node, if the current block is nested. + const parentNode = getParentBlockInfo(tr.doc, block.beforePos)?.block.node; return { - block: nodeToBlock(bnBlock.node, tr.doc), + block: nodeToBlock(block.node, tr.doc), prevBlock: prevNode === null ? undefined : nodeToBlock(prevNode, tr.doc), nextBlock: nextNode === null ? undefined : nodeToBlock(nextNode, tr.doc), parentBlock: @@ -61,65 +47,17 @@ export function setTextCursorPosition( placement: "start" | "end" = "start", ) { const id = typeof targetBlock === "string" ? targetBlock : targetBlock.id; - const pmSchema = getPmSchema(tr.doc); - const schema = getBlockNoteSchema(pmSchema); const posInfo = getNodeById(id, tr.doc); if (!posInfo) { throw new Error(`Block with ID ${id} not found`); } - const info = getBlockInfo(posInfo); - - const contentType: "none" | "inline" | "table" | "plain" = - schema.blockSchema[info.blockNoteType]!.content; - - if (info.isWrappedBlock) { - const blockContent = info.blockContent; - if (contentType === "none") { - tr.setSelection(NodeSelection.create(tr.doc, blockContent.beforePos)); - return; - } - - if (contentType === "inline" || contentType === "plain") { - if (placement === "start") { - tr.setSelection( - TextSelection.create(tr.doc, blockContent.beforePos + 1), - ); - } else { - tr.setSelection( - TextSelection.create(tr.doc, blockContent.afterPos - 1), - ); - } - } else if (contentType === "table") { - if (placement === "start") { - // Need to offset the position as we have to get through the `tableRow` - // and `tableCell` nodes to get to the `tableParagraph` node we want to - // set the selection in. - tr.setSelection( - TextSelection.create(tr.doc, blockContent.beforePos + 4), - ); - } else { - tr.setSelection( - TextSelection.create(tr.doc, blockContent.afterPos - 4), - ); - } - } else { - throw new UnreachableCaseError(contentType); - } - } else { - const child = - placement === "start" - ? info.childContainer.node.firstChild - : info.childContainer.node.lastChild; - - if (!child) { - // A container allowed to hold no children has no text to put a cursor - // in, so the container itself is selected instead. - tr.setSelection(NodeSelection.create(tr.doc, info.bnBlock.beforePos)); - return; - } - - setTextCursorPosition(tr, getNodeId(child, tr.doc), placement); - } + tr.setSelection( + blockEdgeSelection( + tr.doc, + getBlockInfoFromNode(posInfo.node, posInfo.posBeforeNode), + placement, + ), + ); } diff --git a/packages/core/src/api/blockManipulation/setupTestEnv.ts b/packages/core/src/api/blockManipulation/setupTestEnv.ts index c1da2be25e..c54fea1f09 100644 --- a/packages/core/src/api/blockManipulation/setupTestEnv.ts +++ b/packages/core/src/api/blockManipulation/setupTestEnv.ts @@ -1,14 +1,43 @@ import { afterAll, beforeAll, beforeEach } from "vite-plus/test"; -import { PartialBlock } from "../../blocks/defaultBlocks.js"; +import { BlockNoteSchema } from "../../blocks/BlockNoteSchema.js"; +import { + DefaultBlockSchema, + DefaultInlineContentSchema, + DefaultStyleSchema, + PartialBlock, +} from "../../blocks/defaultBlocks.js"; import { BlockNoteEditor } from "../../editor/BlockNoteEditor.js"; +import { + BlockSchema, + InlineContentSchema, + StyleSchema, +} from "../../schema/index.js"; -export function setupTestEnv() { - let editor: BlockNoteEditor; +/** + * Mounts an editor for a test file and resets its document before each test. + * + * Called without arguments it uses the default schema and {@link testDocument}. + * A custom schema and document have to be passed together, since a document is + * only valid against the schema it was written for. + */ +export function setupTestEnv< + B extends BlockSchema = DefaultBlockSchema, + I extends InlineContentSchema = DefaultInlineContentSchema, + S extends StyleSchema = DefaultStyleSchema, +>(options?: { + schema: BlockNoteSchema; + document: PartialBlock[]; +}): () => BlockNoteEditor { + let editor: BlockNoteEditor; const div = document.createElement("div"); beforeAll(() => { - editor = BlockNoteEditor.create(); + // `B`/`I`/`S` fall back to the default schema's types, but TS can't see + // that from inside the body, so the no-options branch needs a cast. + editor = options + ? BlockNoteEditor.create({ schema: options.schema }) + : (BlockNoteEditor.create() as unknown as BlockNoteEditor); editor.mount(div); }); @@ -18,13 +47,16 @@ export function setupTestEnv() { }); beforeEach(() => { - editor.replaceBlocks(editor.document, testDocument); + editor.replaceBlocks( + editor.document, + options?.document ?? (testDocument as unknown as PartialBlock[]), + ); }); return () => editor; } -const testDocument: PartialBlock[] = [ +export const testDocument: PartialBlock[] = [ { id: "paragraph-0", type: "paragraph", diff --git a/packages/core/src/api/clipboard/fromClipboard/handleFileInsertion.ts b/packages/core/src/api/clipboard/fromClipboard/handleFileInsertion.ts index aa422bb23a..74af5ba031 100644 --- a/packages/core/src/api/clipboard/fromClipboard/handleFileInsertion.ts +++ b/packages/core/src/api/clipboard/fromClipboard/handleFileInsertion.ts @@ -5,7 +5,7 @@ import { InlineContentSchema, StyleSchema, } from "../../../schema/index.js"; -import { getBlockInfoAtNearest, getNodeId } from "../../getBlockInfoFromPos.js"; +import { getBlockInfoNearPos, getNodeId } from "../../getBlockInfoFromPos.js"; import { acceptedMIMETypes } from "./acceptedMIMETypes.js"; function checkFileExtensionsMatch( @@ -159,8 +159,8 @@ export async function handleFileInsertion< } insertedBlockId = editor.transact((tr) => { - const blockInfo = getBlockInfoAtNearest(tr, pos.pos); - const id = getNodeId(blockInfo.bnBlock.node, tr.doc); + const blockInfo = getBlockInfoNearPos(tr, pos.pos); + const id = getNodeId(blockInfo.block.node, tr.doc); // TODO technically data-id will always be the non-rewritten id, so there might be multiple in the document. // getNodeId might find the wrong one (aka point to a deleted node when it should be a non-deleted on) // This is acceptable right now, given that we don't expect edits on the document content diff --git a/packages/core/src/api/exporters/html/util/serializeBlocksExternalHTML.ts b/packages/core/src/api/exporters/html/util/serializeBlocksExternalHTML.ts index e9942518b5..36acaed470 100644 --- a/packages/core/src/api/exporters/html/util/serializeBlocksExternalHTML.ts +++ b/packages/core/src/api/exporters/html/util/serializeBlocksExternalHTML.ts @@ -6,7 +6,6 @@ import { BlockImplementation, BlockSchema, InlineContentSchema, - isContainerType, StyleSchema, } from "../../../../schema/index.js"; import { fillContainerAttributes } from "../../../../schema/blocks/containerAttributes.js"; @@ -284,7 +283,7 @@ function serializeBlock< } else { // Asked of the block config rather than of its ProseMirror node. See the // same check in `serializeBlocksInternalHTML`. - if (isContainerType(editor.schema.blockSchema[block.type as any])) { + if (editor.schema.blockSchema[block.type as any].children !== undefined) { // Container blocks own their outer DOM. Make sure the attributes // needed to parse the HTML back (the type marker and non-default // props, in the same `data-*` convention `propsToAttributes` reads) diff --git a/packages/core/src/api/exporters/html/util/serializeBlocksInternalHTML.ts b/packages/core/src/api/exporters/html/util/serializeBlocksInternalHTML.ts index 9bc719c41c..319cafd3fc 100644 --- a/packages/core/src/api/exporters/html/util/serializeBlocksInternalHTML.ts +++ b/packages/core/src/api/exporters/html/util/serializeBlocksInternalHTML.ts @@ -5,7 +5,6 @@ import type { BlockNoteEditor } from "../../../../editor/BlockNoteEditor.js"; import { BlockSchema, InlineContentSchema, - isContainerType, StyleSchema, } from "../../../../schema/index.js"; import { fillContainerAttributes } from "../../../../schema/blocks/containerAttributes.js"; @@ -163,7 +162,7 @@ function serializeBlock< ); const blockConfig = editor.schema.blockSchema[block.type as any]; - const isContainer = isContainerType(blockConfig); + const isContainer = blockConfig.children !== undefined; if (ret.contentDOM && block.content) { const ic = serializeInlineContentInternalHTML( diff --git a/packages/core/src/api/getBlockInfoFromPos.test.ts b/packages/core/src/api/getBlockInfoFromPos.test.ts index 6af6e7b6d8..3787b64879 100644 --- a/packages/core/src/api/getBlockInfoFromPos.test.ts +++ b/packages/core/src/api/getBlockInfoFromPos.test.ts @@ -1,9 +1,17 @@ -import { Schema } from "prosemirror-model"; +import { Node, Schema } from "prosemirror-model"; import { describe, expect, it } from "vite-plus/test"; import { BlockNoteEditor } from "../editor/BlockNoteEditor.js"; +import { blockToNode } from "./nodeConversions/blockToNode.js"; import { docToBlocks } from "./nodeConversions/nodeToBlock.js"; -import { getNodeId } from "./getBlockInfoFromPos.js"; +import { + getBlockInfoFromNode, + getLastDescendantBlockInfo, + getNextBlockInfo, + getNodeId, + getParentBlockInfo, + getPrevBlockInfo, +} from "./getBlockInfoFromPos.js"; import { YAttributionMarksExtension } from "../y/extensions/YAttributionMarks.js"; /** @@ -168,6 +176,227 @@ describe("getNodeId", () => { }); }); +describe("derived position and content fields", () => { + let editor: BlockNoteEditor; + + // Only the schema is needed to construct nodes; a single non-mounted editor + // instance is enough for all cases here. + function getSchema() { + if (!editor) { + editor = BlockNoteEditor.create(); + } + return editor.pmSchema; + } + + it("precomputes content bounds for an inline-content block", () => { + const schema = getSchema(); + const node = blockToNode( + { id: "0", type: "paragraph", content: "Hello" } as any, + schema, + ); + + // A non-zero offset, so the derived positions provably include it. + const info = getBlockInfoFromNode(node, 10); + + expect(info.hasContent).toBe(true); + expect(info.contentStart).toBe(info.content!.beforePos + 1); + expect(info.contentEnd).toBe(info.content!.afterPos - 1); + expect(info.contentKind).toBe("inline"); + expect(info.isContentEmpty).toBe(false); + expect(info.children).toBeUndefined(); + }); + + it("flags an empty inline-content block", () => { + const schema = getSchema(); + const node = blockToNode( + { id: "0", type: "paragraph", content: "" } as any, + schema, + ); + + const info = getBlockInfoFromNode(node, 0); + + expect(info.contentKind).toBe("inline"); + expect(info.isContentEmpty).toBe(true); + // An empty content node still has an inside: start and end coincide. + expect(info.contentStart).toBe(info.contentEnd); + }); + + it("precomputes children bounds when a block has children", () => { + const schema = getSchema(); + const node = blockToNode( + { + id: "0", + type: "paragraph", + content: "Parent", + children: [{ id: "1", type: "paragraph", content: "Child" }], + } as any, + schema, + ); + + const info = getBlockInfoFromNode(node, 0); + + expect(info.children).toBeDefined(); + expect(info.children!.childrenStart).toBe(info.children!.beforePos + 1); + expect(info.children!.childrenEnd).toBe(info.children!.afterPos - 1); + }); + + it("classifies a table's content", () => { + const schema = getSchema(); + const node = blockToNode( + { + id: "0", + type: "table", + content: { type: "tableContent", rows: [{ cells: ["A"] }] }, + } as any, + schema, + ); + + const info = getBlockInfoFromNode(node, 0); + + expect(info.contentKind).toBe("table"); + expect(info.isContentEmpty).toBe(false); + }); + + it("classifies a content-less block", () => { + const schema = getSchema(); + const node = blockToNode({ id: "0", type: "image" } as any, schema); + + const info = getBlockInfoFromNode(node, 0); + + // The block HAS a content node; that node just accepts no content. + expect(info.hasContent).toBe(true); + expect(info.contentKind).toBe("none"); + expect(info.isContentEmpty).toBe(true); + }); + + it("classifies plain-text content as plain", () => { + const schema = getSchema(); + const node = blockToNode( + { id: "0", type: "codeBlock", content: "let x;" } as any, + schema, + ); + + const info = getBlockInfoFromNode(node, 0); + + expect(info.contentKind).toBe("plain"); + }); +}); + +describe("navigation helpers on plain nested blocks", () => { + let editor: BlockNoteEditor; + + function getSchema() { + if (!editor) { + editor = BlockNoteEditor.create(); + } + return editor.pmSchema; + } + + // doc + // └ blockGroup + // ├ A + // │ ├ B + // │ └ C + // │ └ D + // └ E + function buildDoc() { + const schema = getSchema(); + const nodeA = blockToNode( + { + id: "A", + type: "paragraph", + content: "A", + children: [ + { id: "B", type: "paragraph", content: "B" }, + { + id: "C", + type: "paragraph", + content: "C", + children: [{ id: "D", type: "paragraph", content: "D" }], + }, + ], + } as any, + schema, + ); + const nodeE = blockToNode( + { id: "E", type: "paragraph", content: "E" } as any, + schema, + ); + return schema.nodes["doc"].createChecked( + {}, + schema.nodes["blockGroup"].createChecked({}, [nodeA, nodeE]), + ); + } + + function posOf(doc: Node, id: string): number { + let found: number | undefined; + doc.descendants((node, pos) => { + if (node.attrs.id === id) { + found = pos; + return false; + } + return true; + }); + if (found === undefined) { + throw new Error(`Block ${id} not found`); + } + return found; + } + + it("finds the parent block, or undefined at the top level", () => { + const doc = buildDoc(); + expect(getParentBlockInfo(doc, posOf(doc, "B"))?.block.node.attrs.id).toBe( + "A", + ); + expect(getParentBlockInfo(doc, posOf(doc, "D"))?.block.node.attrs.id).toBe( + "C", + ); + expect(getParentBlockInfo(doc, posOf(doc, "A"))).toBeUndefined(); + }); + + it("finds the previous sibling, or undefined for a first child", () => { + const doc = buildDoc(); + expect(getPrevBlockInfo(doc, posOf(doc, "C"))?.block.node.attrs.id).toBe( + "B", + ); + expect(getPrevBlockInfo(doc, posOf(doc, "E"))?.block.node.attrs.id).toBe( + "A", + ); + expect(getPrevBlockInfo(doc, posOf(doc, "B"))).toBeUndefined(); + }); + + it("finds the next sibling, or undefined for a last child", () => { + const doc = buildDoc(); + expect(getNextBlockInfo(doc, posOf(doc, "B"))?.block.node.attrs.id).toBe( + "C", + ); + expect(getNextBlockInfo(doc, posOf(doc, "A"))?.block.node.attrs.id).toBe( + "E", + ); + expect(getNextBlockInfo(doc, posOf(doc, "C"))).toBeUndefined(); + }); + + it("descends to the deepest last block", () => { + const doc = buildDoc(); + const infoA = getBlockInfoFromNode( + doc.nodeAt(posOf(doc, "A"))!, + posOf(doc, "A"), + ); + expect(getLastDescendantBlockInfo(doc, infoA).block.node.attrs.id).toBe( + "D", + ); + + const infoE = getBlockInfoFromNode( + doc.nodeAt(posOf(doc, "E"))!, + posOf(doc, "E"), + ); + // No children: the block itself is the bottom one. + expect(getLastDescendantBlockInfo(doc, infoE).block.node.attrs.id).toBe( + "E", + ); + }); +}); + describe("docToBlocks round trip with suggested deletions", () => { let editor: BlockNoteEditor; diff --git a/packages/core/src/api/getBlockInfoFromPos.ts b/packages/core/src/api/getBlockInfoFromPos.ts index f09627a1d0..4ab02d85e2 100644 --- a/packages/core/src/api/getBlockInfoFromPos.ts +++ b/packages/core/src/api/getBlockInfoFromPos.ts @@ -1,7 +1,30 @@ -import { Node, ResolvedPos } from "prosemirror-model"; -import { EditorState, Transaction } from "prosemirror-state"; +import { Node } from "prosemirror-model"; +import { + EditorState, + NodeSelection, + Selection, + TextSelection, + Transaction, +} from "prosemirror-state"; -import { CHILD_CONTAINER_GROUP } from "../schema/blocks/children.js"; +import { + CHILD_CONTAINER_GROUP, + getBlockRegions, + isSealed, +} from "../schema/blocks/children.js"; + +/** + * Producers for {@link BlockInfo}, named by the input you already have: + * + * - `getBlockInfoFromNode(node, beforePos)` — you hold the block's ProseMirror + * node and the position just before it. + * - `getBlockInfoAt(doc, posBeforeBlock)` — you know the exact position just + * before a block node (throws if no node starts there). + * - `getBlockInfoNearPos(source, pos)` — you have an arbitrary position; walks + * up/over to the nearest block. + * - `getBlockInfoFromSelection(source)` — you want the block containing the + * current selection anchor. + */ type SingleBlockInfo = { node: Node; @@ -9,52 +32,169 @@ type SingleBlockInfo = { afterPos: number; }; +/** + * The node holding a block's children, plus the bounds of the child range. + */ +export type ChildrenInfo = SingleBlockInfo & { + /** + * `beforePos + 1`: the position of the first child; also the insertion + * position for a new first child. + */ + childrenStart: number; + /** `afterPos - 1`: the position just after the last child. */ + childrenEnd: number; +}; + +/** + * What a block's content node holds, derived from its ProseMirror content + * expression. + */ +export type BlockContentKind = "inline" | "plain" | "none" | "table" | "other"; + +function getContentKind(contentNode: Node): BlockContentKind { + const content = contentNode.type.spec.content; + return content === "inline*" + ? "inline" + : content === "text*" + ? "plain" + : content === "" + ? "none" + : content === "tableRow+" + ? "table" + : "other"; +} + +function toChildrenInfo(info: SingleBlockInfo): ChildrenInfo { + return { + ...info, + childrenStart: info.beforePos + 1, + childrenEnd: info.afterPos - 1, + }; +} + export type BlockInfo = { /** * The outer node that represents a BlockNote block. This is the node that has the ID. * Most of the time, this will be a blockContainer node, but it could also be a Column or ColumnList */ - bnBlock: SingleBlockInfo; + block: SingleBlockInfo; /** * The type of BlockNote block that this node represents. - * When dealing with a blockContainer, this is retrieved from the blockContent node, otherwise it's retrieved from the bnBlock node. + * When dealing with a blockContainer, this is retrieved from the content node, otherwise it's retrieved from the block node. */ blockNoteType: string; } & ( | { // A container block (Column, ColumnList, a custom container): its own - // node holds its children directly, and it has no `blockContent` of + // node holds its children directly, and it has no content node of // its own. /** * The Prosemirror node that holds block.children. For a container block, - * this node is the same as bnBlock. + * this node is the same as `block`. */ - childContainer: SingleBlockInfo; - blockContent?: undefined; - isWrappedBlock: false; + children: ChildrenInfo; + content?: undefined; + hasContent: false; + contentStart?: undefined; + contentEnd?: undefined; + contentKind?: undefined; + isContentEmpty?: undefined; } | { /** * The Prosemirror node that holds block.children. For blockContainers, this is the blockGroup node, if it exists. */ - childContainer?: SingleBlockInfo; + children?: ChildrenInfo; /** * The Prosemirror node that wraps block.content and has most of the props */ - blockContent: SingleBlockInfo; + content: SingleBlockInfo; + /** `content.beforePos + 1`: the first position inside the content. */ + contentStart: number; + /** `content.afterPos - 1`: the last position inside the content. */ + contentEnd: number; + /** What the content node holds, from its ProseMirror content expression. */ + contentKind: BlockContentKind; + /** `content.node.childCount === 0`. */ + isContentEmpty: boolean; /** - * Whether `bnBlock` wraps the block's content in a node of its own: a - * `blockContainer` (an ordinary block wrapped for nesting), shaped as a - * content node followed by an optional child container. + * Whether the block has a content node: a `blockContainer` (an + * ordinary block wrapped for nesting), shaped as a content node + * followed by an optional child container. * * Note this is the opposite of "is a container block": a column has - * `isWrappedBlock: false`. + * `hasContent: false`. */ - isWrappedBlock: true; + hasContent: true; } ); +/** + * The caret position at an edge of a table content region: 4 levels in + * (`table` → `tableRow` → `tableCell` → `tableParagraph`) from the region's + * boundary — the first cell's paragraph start, or the last cell's paragraph + * end. + */ +export function tableContentCaretPos( + content: { beforePos: number; afterPos: number }, + edge: "start" | "end", +): number { + return edge === "start" ? content.beforePos + 4 : content.afterPos - 4; +} + +/** + * The caret position at an edge of a block's content, or `null` when the block + * has none there: a container block, or content that holds no text (an image). + */ +export function blockEdgePos( + info: BlockInfo, + edge: "start" | "end", +): number | null { + if (!info.hasContent || info.contentKind === "none") { + return null; + } + return info.contentKind === "table" + ? tableContentCaretPos(info.content, edge) + : edge === "start" + ? info.contentStart + : info.contentEnd; +} + +/** + * A selection at an edge of a block. A container resolves to the same edge of + * its first/last child, recursively. Where there is no caret position the + * nearest node is selected instead: the content node of a block holding no + * text, or the block itself for a container holding no children. + */ +export function blockEdgeSelection( + doc: Node, + info: BlockInfo, + edge: "start" | "end", +): Selection { + const pos = blockEdgePos(info, edge); + if (pos !== null) { + return TextSelection.create(doc, pos); + } + if (info.hasContent) { + return NodeSelection.create(doc, info.content.beforePos); + } + + const { node, childrenStart, childrenEnd } = info.children; + const child = edge === "start" ? node.firstChild : node.lastChild; + if (!child) { + return NodeSelection.create(doc, info.block.beforePos); + } + return blockEdgeSelection( + doc, + getBlockInfoFromNode( + child, + edge === "start" ? childrenStart : childrenEnd - child.nodeSize, + ), + edge, + ); +} + export function isSuggestedDeletionNode(node: Node): boolean { return node.marks.some((m) => ["y-attributed-delete"].includes(m.type.name)); } @@ -165,126 +305,208 @@ export function getNearestBlockPos(doc: Node, pos: number) { /** * Gets information regarding the ProseMirror nodes that make up a block in a - * BlockNote document. This includes the main `blockContainer` node, the - * `blockContent` node with the block's main body, and the optional `blockGroup` - * node which contains the block's children. As well as the nodes, also returns - * the ProseMirror positions just before & after each node. - * @param node The main `blockContainer` node that the block information should - * be retrieved from, - * @param bnBlockBeforePosOffset the position just before the - * `blockContainer` node in the document. + * BlockNote document, given the block's outer node and the position just + * before it. This includes the outer node with the block's ID, the content + * node with the block's main body, and the optional node which contains the + * block's children. As well as the nodes, also returns the ProseMirror + * positions just before & after each node. + * @param node The outer node that the block information should be retrieved + * from. + * @param beforePos The position just before the outer node in the document. */ -export function getBlockInfoWithManualOffset( - node: Node, - bnBlockBeforePosOffset: number, -): BlockInfo { +export function getBlockInfoFromNode(node: Node, beforePos: number): BlockInfo { if (!node.type.isInGroup("bnBlock")) { throw new Error( - `Attempted to get bnBlock node at position but found node of different type ${node.type.name}`, + `Attempted to get block node at position but found node of different type ${node.type.name}`, ); } - const bnBlockNode = node; - const bnBlockBeforePos = bnBlockBeforePosOffset; - const bnBlockAfterPos = bnBlockBeforePos + bnBlockNode.nodeSize; + // The one place block shape is resolved; everything below is position + // annotation over the regions. + const regions = getBlockRegions(node); - const bnBlock: SingleBlockInfo = { - node: bnBlockNode, - beforePos: bnBlockBeforePos, - afterPos: bnBlockAfterPos, + const block: SingleBlockInfo = { + node, + beforePos, + afterPos: beforePos + node.nodeSize, }; - if (bnBlockNode.type.name === "blockContainer") { - let blockContent: SingleBlockInfo | undefined; - let childContainer: SingleBlockInfo | undefined; - - bnBlockNode.forEach((node, offset) => { - const beforePos = bnBlockBeforePos + offset + 1; - const afterPos = beforePos + node.nodeSize; - - if (node.type.spec.group === "blockContent") { - blockContent = { node, beforePos, afterPos }; - } else if (node.type.isInGroup(CHILD_CONTAINER_GROUP)) { - childContainer = { node, beforePos, afterPos }; - } - }); - - if (!blockContent) { - throw new Error( - // eslint-disable-next-line @typescript-eslint/restrict-template-expressions - `${bnBlockNode.type.name} node does not contain a content node in its children: ${bnBlockNode}`, - ); - } - + if (regions.content) { + const content: SingleBlockInfo = { + node: regions.content.node, + beforePos: beforePos + regions.content.offset, + afterPos: + beforePos + regions.content.offset + regions.content.node.nodeSize, + }; + const holder = regions.childrenHolder; return { - isWrappedBlock: true, - bnBlock, - blockContent, - childContainer, + hasContent: true, + block, + content, + children: holder + ? toChildrenInfo({ + node: holder.node, + beforePos: beforePos + holder.offset, + afterPos: beforePos + holder.offset + holder.node.nodeSize, + }) + : undefined, + contentStart: content.beforePos + 1, + contentEnd: content.afterPos - 1, + contentKind: getContentKind(content.node), + isContentEmpty: content.node.childCount === 0, // A `blockContainer` is a generic wrapper, so its type comes from the // content node inside it. - blockNoteType: blockContent.node.type.name, + blockNoteType: content.node.type.name, }; - } else { - if (!bnBlock.node.type.isInGroup("childContainer")) { - throw new Error( - // eslint-disable-next-line @typescript-eslint/restrict-template-expressions - `bnBlock node is not in the childContainer group: ${bnBlock.node}`, - ); - } + } - return { - isWrappedBlock: false, - bnBlock: bnBlock, - childContainer: bnBlock, - blockNoteType: bnBlock.node.type.name, - }; + return { + hasContent: false, + block, + // A container holds its children directly, so the holder is the block + // node itself. + children: toChildrenInfo(block), + blockNoteType: node.type.name, + }; +} + +/** + * Gets information regarding the ProseMirror nodes that make up a block, given + * a position known to be just before a block node. Throws if no node starts at + * that position. + * @param doc The ProseMirror doc. + * @param posBeforeBlock The position just before the block's outer node. + */ +export function getBlockInfoAt(doc: Node, posBeforeBlock: number): BlockInfo { + const $pos = doc.resolve(posBeforeBlock); + if (!$pos.nodeAfter) { + throw new Error( + `Attempted to get block node at position ${posBeforeBlock} but a node at this position does not exist`, + ); } + return getBlockInfoFromNode($pos.nodeAfter, $pos.pos); } /** - * Gets information regarding the ProseMirror nodes that make up a block in a - * BlockNote document. This includes the main `blockContainer` node, the - * `blockContent` node with the block's main body, and the optional `blockGroup` - * node which contains the block's children. As well as the nodes, also returns - * the ProseMirror positions just before & after each node. - * @param posInfo An object with the main `blockContainer` node that the block - * information should be retrieved from, and the position just before it in the - * document. + * Gets information regarding the ProseMirror nodes that make up the block + * nearest to an arbitrary position (see {@link getNearestBlockPos}). + * @param source The ProseMirror editor state or transaction. + * @param pos An integer position in the document. */ -export function getBlockInfo(posInfo: { posBeforeNode: number; node: Node }) { - return getBlockInfoWithManualOffset(posInfo.node, posInfo.posBeforeNode); +export function getBlockInfoNearPos( + source: EditorState | Transaction, + pos: number, +): BlockInfo { + const posInfo = getNearestBlockPos(source.doc, pos); + return getBlockInfoFromNode(posInfo.node, posInfo.posBeforeNode); } /** - * Gets information regarding the ProseMirror nodes that make up a block from a - * resolved position just before the `blockContainer` node in the document that - * corresponds to it. - * @param resolvedPos The resolved position just before the `blockContainer` - * node. + * Gets information regarding the ProseMirror nodes that make up the block + * containing the current ProseMirror selection anchor. + * @param source The ProseMirror editor state or transaction. */ -export function getBlockInfoFromResolvedPos(resolvedPos: ResolvedPos) { - if (!resolvedPos.nodeAfter) { - throw new Error( - `Attempted to get blockContainer node at position ${resolvedPos.pos} but a node at this position does not exist`, - ); +export function getBlockInfoFromSelection(source: EditorState | Transaction) { + return getBlockInfoNearPos(source, source.selection.anchor); +} + +/** + * The parent block's info: the block whose `children` contains the block at + * `posBeforeBlock`, or `undefined` for a top-level block. A container is the + * parent of its direct children (a block inside a column → the column, not + * the columnList); a regular block's children live in its `blockGroup`, so + * the parent is the group's own parent. + */ +export function getParentBlockInfo( + doc: Node, + posBeforeBlock: number, +): BlockInfo | undefined { + const $pos = doc.resolve(posBeforeBlock); + const parent = $pos.node(); + + if (parent.type.isInGroup("bnBlock")) { + return getBlockInfoAt(doc, $pos.before($pos.depth)); } - return getBlockInfoWithManualOffset(resolvedPos.nodeAfter, resolvedPos.pos); + // A `blockGroup`: its own parent block is the real parent, unless it's the + // document root group. + if (parent.type.isInGroup(CHILD_CONTAINER_GROUP) && $pos.depth > 1) { + return getBlockInfoAt(doc, $pos.before($pos.depth - 1)); + } + return undefined; } /** - * Gets information regarding the ProseMirror nodes that make up a block. The - * block chosen is the one currently containing the current ProseMirror - * selection. - * @param source The ProseMirror editor state. + * Returns the block info from the sibling block before (above) the given block, + * or undefined if the given block is the first sibling. */ -export function getBlockInfoFromSelection(source: EditorState | Transaction) { - return getBlockInfoAtNearest(source, source.selection.anchor); +export function getPrevBlockInfo( + doc: Node, + beforePos: number, +): BlockInfo | undefined { + const $pos = doc.resolve(beforePos); + + const indexInParent = $pos.index(); + + if (indexInParent === 0) { + return undefined; + } + + const prevBlockBeforePos = $pos.posAtIndex(indexInParent - 1); + + return getBlockInfoAt(doc, prevBlockBeforePos); } -export function getBlockInfoAtNearest( - source: EditorState | Transaction, - pos: number, -) { - return getBlockInfo(getNearestBlockPos(source.doc, pos)); +/** + * Returns the block info from the sibling block after (below) the given block, + * or undefined if the given block is the last sibling. + */ +export function getNextBlockInfo( + doc: Node, + beforePos: number, +): BlockInfo | undefined { + const $pos = doc.resolve(beforePos); + + const indexInParent = $pos.index(); + + if (indexInParent === $pos.node().childCount - 1) { + return undefined; + } + + const nextBlockBeforePos = $pos.posAtIndex(indexInParent + 1); + + return getBlockInfoAt(doc, nextBlockBeforePos); +} + +/** + * If a block has children like this: + * A + * - B + * - C + * -- D + * + * Then the last descendant block returned is D. + * + * The descent stops at a sealed container, returning the container itself + * rather than a block inside it. Every caller is a keyboard gesture, and + * seals bind gestures. + */ +export function getLastDescendantBlockInfo( + doc: Node, + blockInfo: BlockInfo, +): BlockInfo { + // A container that allows zero children can have an empty child container, + // in which case the block itself is the bottom one. + while (blockInfo.children && blockInfo.children.node.childCount) { + if (isSealed(blockInfo.children.node)) { + break; + } + const group = blockInfo.children.node; + + const newPos = doc + .resolve(blockInfo.children.beforePos + 1) + .posAtIndex(group.childCount - 1); + blockInfo = getBlockInfoAt(doc, newPos); + } + + return blockInfo; } diff --git a/packages/core/src/api/getBlocksChangedByTransaction.test.ts b/packages/core/src/api/getBlocksChangedByTransaction.test.ts index 2186fefe7d..b2853b9181 100644 --- a/packages/core/src/api/getBlocksChangedByTransaction.test.ts +++ b/packages/core/src/api/getBlocksChangedByTransaction.test.ts @@ -2,7 +2,7 @@ import { describe, expect, it, beforeEach } from "vite-plus/test"; import { setupTestEnv } from "./blockManipulation/setupTestEnv.js"; import { getBlocksChangedByTransaction } from "./getBlocksChangedByTransaction.js"; -import { getBlockInfo } from "./getBlockInfoFromPos.js"; +import { getBlockInfoFromNode } from "./getBlockInfoFromPos.js"; import { getNodeById } from "./nodeUtil.js"; import { BlockNoteEditor } from "../editor/BlockNoteEditor.js"; import { PartialBlock } from "../blocks/defaultBlocks.js"; @@ -651,15 +651,15 @@ describe("getBlocksChangedByTransaction - ranged optimization", () => { if (!posInfo) { throw new Error("block not found"); } - const info = getBlockInfo(posInfo); - if (!info.isWrappedBlock) { + const info = getBlockInfoFromNode(posInfo.node, posInfo.posBeforeNode); + if (!info.hasContent) { throw new Error("expected a wrapped block"); } // Adding a mark produces an AddMarkStep, whose StepMap is empty — the case // getChangedRange has to recover from the step's own from/to. tr.addMark( - info.blockContent.beforePos + 1, - info.blockContent.afterPos - 1, + info.content.beforePos + 1, + info.content.afterPos - 1, editor.pmSchema.marks.bold.create(), ); return getBlocksChangedByTransaction(tr); diff --git a/packages/core/src/api/nodeConversions/blockToNode.ts b/packages/core/src/api/nodeConversions/blockToNode.ts index 81f6937040..77d5585d4c 100644 --- a/packages/core/src/api/nodeConversions/blockToNode.ts +++ b/packages/core/src/api/nodeConversions/blockToNode.ts @@ -1,11 +1,4 @@ -import { - Attrs, - Fragment, - Mark, - Node, - NodeType, - Schema, -} from "@tiptap/pm/model"; +import { Attrs, Fragment, Mark, Node, Schema } from "@tiptap/pm/model"; import UniqueID from "../../extensions/tiptap-extensions/UniqueID/UniqueID.js"; import type { @@ -24,10 +17,10 @@ import { isStyledTextInlineContent, } from "../../schema/inlineContent/types.js"; // `isContainerNode` comes from `children.js` directly (rather than via its -// `fixContainer.js` re-export) because `fixContainer.js` imports the seeding -// machinery below; going through it would create an import cycle. +// `fixContainer.js` re-export) because `fixContainer.js` imports `blockToNode` +// below; going through it would create an import cycle. import { - getChildrenConfig, + createBlockGroup, isContainerNode, resolveChildren, } from "../../schema/blocks/children.js"; @@ -307,13 +300,17 @@ export function tableContentToNodes< return rowNodes; } +/** + * Converts a block's (or custom inline content element's) `content` field to a + * `blockContent` (or custom inline content) prosemirror node. + */ function blockOrInlineContentToContentNode( block: | PartialBlock | PartialCustomInlineContentFromConfig, schema: Schema, styleSchema: StyleSchema, -) { +): Node { let contentNode: Node; let type = block.type; @@ -356,160 +353,29 @@ function blockOrInlineContentToContentNode( const EMPTY_SEEDING: ReadonlySet = new Set(); function unwrapsWhenEmptied(blockType: string, schema: Schema): boolean { - const blockConfig = getBlockSchema(schema)[blockType]; - const children = blockConfig ? getChildrenConfig(blockConfig) : undefined; + const children = getBlockSchema(schema)[blockType]?.children; return !!children && resolveChildren(children).whenEmptied === "unwrap"; } -// `createAndFill` produces nodes with `id: null`; patch them before use. +// `createAndFill` fills with schema defaults, which leaves `id: null`. Always +// rebuilds: the only inputs are freshly created nodes, so there is no shared +// structure worth preserving. function withGeneratedIds(node: Node): Node { if (node.isText) { return node; } const children: Node[] = []; - let childChanged = false; - node.forEach((child) => { - const next = withGeneratedIds(child); - childChanged ||= next !== child; - children.push(next); - }); + node.forEach((child) => children.push(withGeneratedIds(child))); const needsId = node.type.isInGroup("bnBlock") && node.attrs.id === null; - if (!needsId && !childChanged) { - return node; - } - return node.type.create( needsId ? { ...node.attrs, id: UniqueID.options.generateID() } : node.attrs, - childChanged ? Fragment.from(children) : node.content, + Fragment.from(children), node.marks, ); } -function seedDefaultChildren( - blockType: string, - schema: Schema, - styleSchema: StyleSchema, - seedingTypes: ReadonlySet, -): Node[] | undefined { - const blockSchemaConfig = getBlockSchema(schema)[blockType]; - const childrenConfig = blockSchemaConfig - ? getChildrenConfig(blockSchemaConfig) - : undefined; - - if (!childrenConfig) { - return undefined; - } - - const defaultChildren = resolveChildren(childrenConfig).default; - if (!defaultChildren || defaultChildren.length === 0) { - return undefined; - } - - if (seedingTypes.has(blockType)) { - throw new Error( - `Seeding "${blockType}" ends up seeding it again (${[...seedingTypes, blockType].join(" -> ")}). ` + - "Give the cyclic default explicit children, or remove the self-reference.", - ); - } - - const nextSeeding = new Set(seedingTypes).add(blockType); - return defaultChildren.map((child) => - blockToNode( - child as PartialBlock, - schema, - styleSchema, - nextSeeding, - ), - ); -} - -/** - * The nodes `whenEmptied: "refill"` appends when a container's non-empty - * children drop below `min`: the unconsumed tail of its `default` - * (`default[from..min-1]`), each converted exactly like an inserted block. - * Empty when the container has no `default`; the caller pads any remainder - * with empty fill. - */ -export function seedRefillChildren( - blockType: string, - schema: Schema, - from: number, - min: number, -): Node[] { - const blockConfig = getBlockSchema(schema)[blockType]; - const children = blockConfig ? getChildrenConfig(blockConfig) : undefined; - const defaultChildren = children - ? resolveChildren(children).default - : undefined; - if (!defaultChildren) { - return []; - } - - return defaultChildren - .slice(from, min) - .map((child) => blockToNode(child as PartialBlock, schema)); -} - -function createContainerChildrenNode( - blockType: string, - type: NodeType, - schema: Schema, - styleSchema: StyleSchema, - seedingTypes: ReadonlySet, - attrs: Attrs | null = null, -): Node { - const seeded = seedDefaultChildren( - blockType, - schema, - styleSchema, - seedingTypes, - ); - - if (!seeded && unwrapsWhenEmptied(blockType, schema)) { - // Fill so the node satisfies its own content expression for the - // `node.check()` that runs before the repair pass (e.g. in - // `removeAndInsertBlocks`); that pass then unwraps the still-empty - // container. Without the fill, a `min >= 1` unwrap container with no - // `default` produces a schema-invalid node and `check()` throws. - return type.createAndFill(attrs) ?? type.create(attrs); - } - - const node = type.createAndFill(attrs, seeded); - if (!node) { - throw new Error( - `Cannot create block "${blockType}": its \`default\` children don't fit its \`children\` config ` + - `(it accepts \`${type.spec.content}\`).`, - ); - } - - return node; -} - -// Passes explicit children straight through for unwrap-on-empty containers -// (fill would be undone by the next repair pass) and for unfittable content -// (let `node.check()` report it). An empty child list is the exception: it -// still needs filling to survive the pre-repair `node.check()`. -function createExplicitChildrenNode( - blockType: string, - type: NodeType, - schema: Schema, - children: Node[], - attrs: Attrs | null = null, -): Node { - if (unwrapsWhenEmptied(blockType, schema)) { - // An empty explicit `children: []` would leave a `min >= 1` container - // schema-invalid and fail the pre-repair `node.check()`, so fill it (the - // repair pass unwraps it). Non-empty explicit children are left as given. - return children.length === 0 - ? (type.createAndFill(attrs) ?? type.create(attrs)) - : type.create(attrs, children); - } - - return type.createAndFill(attrs, children) ?? type.create(attrs, children); -} - /** * Converts a BlockNote block to a Prosemirror node. */ @@ -545,9 +411,7 @@ export function blockToNode( ); const groupNode = - children.length > 0 - ? schema.nodes["blockGroup"].createChecked({}, children) - : undefined; + children.length > 0 ? createBlockGroup(schema, children) : undefined; return schema.nodes["blockContainer"].createChecked( { @@ -560,22 +424,60 @@ export function blockToNode( const type = schema.nodes[block.type]; const attrs = { id: id, ...block.props }; + // Explicit children are padded up to the container's `min` so that even a + // `children: []` satisfies the content expression, and so survives the + // `node.check()` callers run before touching the doc. The exception is a + // non-empty child list on a container that unwraps as it empties: padding + // it would invent content the next repair pass deletes anyway, so it is + // passed through as given and `node.check()` reports the shortfall. if (block.children !== undefined) { - return withGeneratedIds( - createExplicitChildrenNode(block.type, type, schema, children, attrs), + const padded = + children.length === 0 || !unwrapsWhenEmptied(block.type, schema) + ? type.createAndFill(attrs, children) + : null; + + return withGeneratedIds(padded ?? type.create(attrs, children)); + } + + // No explicit `children`: seed the container from its `children` config's + // `default`, converting each default child exactly like an inserted block. + // `seedingTypes` tracks the container types currently being seeded so a + // cyclic `default` (a container whose default children seed it again) + // fails loudly instead of recursing forever. + const childrenConfig = getBlockSchema(schema)[block.type]?.children; + const defaultChildren = childrenConfig + ? resolveChildren(childrenConfig).default + : undefined; + + let seeded: Node[] | undefined; + if (defaultChildren && defaultChildren.length > 0) { + if (seedingTypes.has(block.type)) { + throw new Error( + `Seeding "${block.type}" ends up seeding it again (${[...seedingTypes, block.type].join(" -> ")}). ` + + "Give the cyclic default explicit children, or remove the self-reference.", + ); + } + + const nextSeeding = new Set(seedingTypes).add(block.type); + seeded = defaultChildren.map((child) => + blockToNode( + child as PartialBlock, + schema, + styleSchema, + nextSeeding, + ), ); } - return withGeneratedIds( - createContainerChildrenNode( - block.type, - type, - schema, - styleSchema, - seedingTypes, - attrs, - ), - ); + const node = type.createAndFill(attrs, seeded); + if (!node) { + throw new Error( + `Cannot create block "${block.type}": its \`default\` children don't fit its \`children\` config ` + + `(it accepts \`${type.spec.content}\`).`, + ); + } + + return withGeneratedIds(node); } else { throw new Error( `block type ${block.type} doesn't match blockContent or bnBlock group`, diff --git a/packages/core/src/api/nodeConversions/fragmentToBlocks.ts b/packages/core/src/api/nodeConversions/fragmentToBlocks.ts index ddd3de46f2..5b67cdb3bc 100644 --- a/packages/core/src/api/nodeConversions/fragmentToBlocks.ts +++ b/packages/core/src/api/nodeConversions/fragmentToBlocks.ts @@ -6,9 +6,7 @@ import { StyleSchema, } from "../../schema/index.js"; import { - getChildrenConfig, isContainerNode, - isPlaceableAnywhere, resolveChildren, } from "../../schema/blocks/children.js"; import { getBlockSchema } from "../pmUtil.js"; @@ -18,13 +16,13 @@ function isSelfContainedContainer(node: Node): boolean { if (!isContainerNode(node.type)) { return false; } - const blockConfig = getBlockSchema(node.type.schema)[node.type.name] ?? {}; - const childrenConfig = getChildrenConfig(blockConfig); - if (!childrenConfig) { + const blockConfig = getBlockSchema(node.type.schema)[node.type.name]; + const childrenConfig = blockConfig?.children; + if (!blockConfig || !childrenConfig) { return false; } return ( - isPlaceableAnywhere(blockConfig) && + blockConfig.placement !== "containerOnly" && node.childCount >= resolveChildren(childrenConfig).min ); } diff --git a/packages/core/src/api/nodeConversions/nodeToBlock.ts b/packages/core/src/api/nodeConversions/nodeToBlock.ts index 0037759daa..43b5b0780e 100644 --- a/packages/core/src/api/nodeConversions/nodeToBlock.ts +++ b/packages/core/src/api/nodeConversions/nodeToBlock.ts @@ -19,10 +19,8 @@ import { isStyledTextInlineContent, } from "../../schema/inlineContent/types.js"; import { UnreachableCaseError } from "../../util/typescript.js"; -import { - getBlockInfoWithManualOffset, - getNodeId, -} from "../getBlockInfoFromPos.js"; +import { getBlockInfoFromNode, getNodeId } from "../getBlockInfoFromPos.js"; +import type { BlockInfo } from "../getBlockInfoFromPos.js"; import { getBlockCache, getBlockSchema, @@ -389,6 +387,17 @@ export function nodeToCustomInlineContent< return ic; } +// A config declaring content and a node shaped to hold it state the same +// fact, but nothing ties them together in the type system. +function contentNode(blockInfo: BlockInfo, declared: string): Node { + if (!blockInfo.hasContent) { + throw new Error( + `Block "${blockInfo.blockNoteType}" declares ${declared} content but holds children.`, + ); + } + return blockInfo.content.node; +} + /** * Convert a Prosemirror node to a BlockNote block. */ @@ -403,7 +412,7 @@ export function nodeToBlock< const styleSchema = getStyleSchema(schema) as S; const blockCache = getBlockCache(schema); if (!node.type.isInGroup("bnBlock")) { - throw Error("Node should be a bnBlock, but is instead: " + node.type.name); + throw Error("Node should be a block, but is instead: " + node.type.name); } const cachedBlock = blockCache?.get(node); @@ -412,28 +421,28 @@ export function nodeToBlock< return cachedBlock; } - const blockInfo = getBlockInfoWithManualOffset(node, 0); + const blockInfo = getBlockInfoFromNode(node, 0); let id: string; try { - id = getNodeId(blockInfo.bnBlock.node, doc); + id = getNodeId(blockInfo.block.node, doc); } catch { // Only used for blocks converted from other formats. id = UniqueID.options.generateID(); } - const blockSpec = blockSchema[blockInfo.blockNoteType]; + const blockConfig = blockSchema[blockInfo.blockNoteType]; - if (!blockSpec) { + if (!blockConfig) { throw Error("Block is of an unrecognized type: " + blockInfo.blockNoteType); } const props: any = {}; for (const [attr, value] of Object.entries({ ...node.attrs, - ...(blockInfo.isWrappedBlock ? blockInfo.blockContent.node.attrs : {}), + ...(blockInfo.hasContent ? blockInfo.content.node.attrs : {}), })) { - const propSchema = blockSpec.propSchema; + const propSchema = blockConfig.propSchema; if ( attr in propSchema && @@ -443,40 +452,29 @@ export function nodeToBlock< } } - const blockConfig = blockSchema[blockInfo.blockNoteType]; - const children: Block[] = []; - blockInfo.childContainer?.node.forEach((child) => { + blockInfo.children?.node.forEach((child) => { children.push(nodeToBlock(child, doc)); }); let content: Block["content"]; if (blockConfig.content === "inline") { - if (!blockInfo.isWrappedBlock) { - throw new Error("impossible"); - } content = contentNodeToInlineContent( - blockInfo.blockContent.node, + contentNode(blockInfo, blockConfig.content), inlineContentSchema, styleSchema, ); } else if (blockConfig.content === "table") { - if (!blockInfo.isWrappedBlock) { - throw new Error("impossible"); - } content = contentNodeToTableContent( - blockInfo.blockContent.node, + contentNode(blockInfo, blockConfig.content), inlineContentSchema, styleSchema, ); } else if (blockConfig.content === "plain") { - if (!blockInfo.isWrappedBlock) { - throw new Error("impossible"); - } // Plain content is a single unstyled text item; an empty block is an // empty array, matching inline content. - const text = blockInfo.blockContent.node.textContent; + const text = contentNode(blockInfo, blockConfig.content).textContent; content = text.length > 0 ? [{ type: "text", text, styles: {} }] : []; } else if (blockConfig.content === "none") { content = undefined; @@ -565,7 +563,7 @@ export function prosemirrorSliceToSlicedBlocks< blockCutAtEnd: string | undefined; } { // Both `blockGroup` and container nodes (columnList, column, callout, - // ...) hold bnBlock children directly, so both can be processed here. + // ...) hold block children directly, so both can be processed here. if (node.type.name !== "blockGroup" && !isContainerNode(node.type)) { throw new Error("unexpected"); } @@ -573,32 +571,43 @@ export function prosemirrorSliceToSlicedBlocks< let blockCutAtStart: string | undefined; let blockCutAtEnd: string | undefined; + // Descends into a child-holding node the slice boundary is open inside + // of: the holder wrapper is skipped and the included children are spliced + // in, propagating cut ids from whichever ends are open. Shared by open + // container children and the degenerate `blockContainer`-around- + // `blockGroup` wrapper — regular nesting's version of the same shape. + function descendOpenHolder( + holder: Node, + openAtStart: boolean, + openAtEnd: boolean, + ) { + const ret = processNode( + holder, + openAtStart ? Math.max(0, openStart - 1) : 0, + openAtEnd ? Math.max(0, openEnd - 1) : 0, + ); + if (openAtStart) { + blockCutAtStart = ret.blockCutAtStart; + } + if (openAtEnd) { + blockCutAtEnd = ret.blockCutAtEnd; + } + blocks.push(...ret.blocks); + } + node.forEach((blockContainer, _offset, index) => { const isFirstBlock = index === 0; const isLastBlock = index === node.childCount - 1; if (isContainerNode(blockContainer.type)) { // A container child. When the slice boundary is open inside it, the - // selection covers part of its children, so skip the container - // wrapper and splice in the included children (mirroring the - // nested-blockGroup descent below). When fully enclosed, convert it - // wholesale. + // selection covers part of its children; when fully enclosed, convert + // it wholesale. const openAtStart = isFirstBlock && openStart > 0; const openAtEnd = isLastBlock && openEnd > 0; if (openAtStart || openAtEnd) { - const ret = processNode( - blockContainer, - openAtStart ? Math.max(0, openStart - 1) : 0, - openAtEnd ? Math.max(0, openEnd - 1) : 0, - ); - if (openAtStart) { - blockCutAtStart = ret.blockCutAtStart; - } - if (openAtEnd) { - blockCutAtEnd = ret.blockCutAtEnd; - } - blocks.push(...ret.blocks); + descendOpenHolder(blockContainer, openAtStart, openAtEnd); return; } @@ -634,16 +643,11 @@ export function prosemirrorSliceToSlicedBlocks< if (!isFirstBlock) { throw new Error("unexpected"); } - const ret = processNode( - blockContainer.firstChild!, - Math.max(0, openStart - 1), - isLastBlock ? Math.max(0, openEnd - 1) : 0, - ); - blockCutAtStart = ret.blockCutAtStart; - if (isLastBlock) { - blockCutAtEnd = ret.blockCutAtEnd; - } - blocks.push(...ret.blocks); + // Open at the start by construction (a `blockContainer` can only lead + // with its `blockGroup` when the slice cut its content node away); + // open at the end whenever it is also the last block, matching the + // pre-refactor cut propagation. + descendOpenHolder(blockContainer.firstChild!, true, isLastBlock); return; } diff --git a/packages/core/src/api/nodeUtil.ts b/packages/core/src/api/nodeUtil.ts index 9214a9ad42..efb41ba1c2 100644 --- a/packages/core/src/api/nodeUtil.ts +++ b/packages/core/src/api/nodeUtil.ts @@ -18,7 +18,7 @@ export function getNodeById( } // Keeps traversing nodes if block with target ID has not been found. Some - // bnBlock nodes we merely pass over (e.g. `column`/`columnList`) may not + // block nodes we merely pass over (e.g. `column`/`columnList`) may not // carry an id — skip them without calling the throwing `getNodeId`, which // errors on id-less nodes. Only nodes that actually have an id are compared. if (!isNodeBlock(node) || !node.attrs.id || getNodeId(node, doc) !== id) { diff --git a/packages/core/src/blocks/ListItem/ListItemKeyboardShortcuts.ts b/packages/core/src/blocks/ListItem/ListItemKeyboardShortcuts.ts index 71f3ecaf35..218618ca1f 100644 --- a/packages/core/src/blocks/ListItem/ListItemKeyboardShortcuts.ts +++ b/packages/core/src/blocks/ListItem/ListItemKeyboardShortcuts.ts @@ -11,17 +11,17 @@ export const handleEnter = (editor: BlockNoteEditor) => { }; }); - if (!blockInfo.isWrappedBlock) { + if (!blockInfo.hasContent) { return false; } - const { bnBlock: blockContainer, blockContent } = blockInfo; + const { block: blockContainer, content } = blockInfo; if ( !( - blockContent.node.type.name === "toggleListItem" || - blockContent.node.type.name === "bulletListItem" || - blockContent.node.type.name === "numberedListItem" || - blockContent.node.type.name === "checkListItem" + content.node.type.name === "toggleListItem" || + content.node.type.name === "bulletListItem" || + content.node.type.name === "numberedListItem" || + content.node.type.name === "checkListItem" ) || !selectionEmpty ) { @@ -32,7 +32,7 @@ export const handleEnter = (editor: BlockNoteEditor) => { () => // Changes list item block to a paragraph block if the content is empty. commands.command(() => { - if (blockContent.node.childCount === 0) { + if (blockInfo.isContentEmpty) { return commands.command( updateBlockCommand(blockContainer.beforePos, { type: "paragraph", @@ -48,7 +48,7 @@ export const handleEnter = (editor: BlockNoteEditor) => { // Splits the current block, moving content inside that's after the cursor // to a new block of the same type below. commands.command(() => { - if (blockContent.node.childCount > 0) { + if (content.node.childCount > 0) { chain() .deleteSelection() .command(splitBlockCommand(state.selection.from, true)) diff --git a/packages/core/src/blocks/ListItem/NumberedListItem/IndexingPlugin.ts b/packages/core/src/blocks/ListItem/NumberedListItem/IndexingPlugin.ts index 5e52c8c76f..0222fff46a 100644 --- a/packages/core/src/blocks/ListItem/NumberedListItem/IndexingPlugin.ts +++ b/packages/core/src/blocks/ListItem/NumberedListItem/IndexingPlugin.ts @@ -3,7 +3,7 @@ import type { Transaction } from "@tiptap/pm/state"; import { Plugin, PluginKey } from "@tiptap/pm/state"; import { Decoration, DecorationSet } from "@tiptap/pm/view"; -import { getBlockInfo } from "../../../api/getBlockInfoFromPos.js"; +import { getBlockInfoFromNode } from "../../../api/getBlockInfoFromPos.js"; // Loosely based on https://github.com/ueberdosis/tiptap/blob/7ac01ef0b816a535e903b5ca92492bff110a71ae/packages/extension-mathematics/src/MathematicsPlugin.ts (MIT) @@ -31,11 +31,11 @@ function calculateListItemIndex( const hasStart = !!node.firstChild!.attrs["start"]; // Fast path: previous sibling already in cache - const blockInfo = getBlockInfo({ posBeforeNode: pos, node }); - if (!blockInfo.isWrappedBlock) { + const blockInfo = getBlockInfoFromNode(node, pos); + if (!blockInfo.hasContent) { throw new Error("impossible"); } - const prevBlock = tr.doc.resolve(blockInfo.bnBlock.beforePos).nodeBefore; + const prevBlock = tr.doc.resolve(blockInfo.block.beforePos).nodeBefore; const prevBlockIndex = prevBlock ? map.get(prevBlock) : undefined; if (prevBlockIndex !== undefined) { const index = prevBlockIndex + 1; @@ -48,7 +48,7 @@ function calculateListItemIndex( // or the start of the parent. const chain: { node: Node; pos: number }[] = [{ node, pos }]; let curNode = prevBlock; - let curBeforePos = blockInfo.bnBlock.beforePos; + let curBeforePos = blockInfo.block.beforePos; while (curNode) { const cachedIndex = map.get(curNode); @@ -56,16 +56,16 @@ function calculateListItemIndex( // Found a cached predecessor — start counting from here break; } - const curInfo = getBlockInfo({ - posBeforeNode: curBeforePos - curNode.nodeSize, - node: curNode, - }); + const curInfo = getBlockInfoFromNode( + curNode, + curBeforePos - curNode.nodeSize, + ); if (curInfo.blockNoteType !== "numberedListItem") { break; } chain.push({ node: curNode, pos: curBeforePos - curNode.nodeSize }); - const nextPrev = tr.doc.resolve(curInfo.bnBlock.beforePos).nodeBefore; - curBeforePos = curInfo.bnBlock.beforePos; + const nextPrev = tr.doc.resolve(curInfo.block.beforePos).nodeBefore; + curBeforePos = curInfo.block.beforePos; curNode = nextPrev; } @@ -76,14 +76,11 @@ function calculateListItemIndex( // Determine starting index from the block just before the chain const lastInChain = chain[chain.length - 1]; - const lastInfo = getBlockInfo({ - posBeforeNode: lastInChain.pos, - node: lastInChain.node, - }); - if (!lastInfo.isWrappedBlock) { + const lastInfo = getBlockInfoFromNode(lastInChain.node, lastInChain.pos); + if (!lastInfo.hasContent) { throw new Error("impossible"); } - const predecessorNode = tr.doc.resolve(lastInfo.bnBlock.beforePos).nodeBefore; + const predecessorNode = tr.doc.resolve(lastInfo.block.beforePos).nodeBefore; const predecessorIndex = predecessorNode ? map.get(predecessorNode) : undefined; diff --git a/packages/core/src/blocks/utils/listItemEnterHandler.ts b/packages/core/src/blocks/utils/listItemEnterHandler.ts index 578d3aae8b..6008c1a023 100644 --- a/packages/core/src/blocks/utils/listItemEnterHandler.ts +++ b/packages/core/src/blocks/utils/listItemEnterHandler.ts @@ -14,16 +14,16 @@ export const handleEnter = ( }; }); - if (!blockInfo.isWrappedBlock) { + if (!blockInfo.hasContent) { return false; } - const { bnBlock: blockContainer, blockContent } = blockInfo; + const { block: blockContainer, content } = blockInfo; - if (!(blockContent.node.type.name === listItemType) || !selectionEmpty) { + if (!(content.node.type.name === listItemType) || !selectionEmpty) { return false; } - if (blockContent.node.childCount === 0) { + if (blockInfo.isContentEmpty) { editor.transact((tr) => { updateBlockTr(tr, blockContainer.beforePos, { type: "paragraph", @@ -31,7 +31,7 @@ export const handleEnter = ( }); }); return true; - } else if (blockContent.node.childCount > 0) { + } else if (content.node.childCount > 0) { return editor.transact((tr) => { tr.deleteSelection(); tr.scrollIntoView(); diff --git a/packages/core/src/editor/BlockNoteEditor.test.ts b/packages/core/src/editor/BlockNoteEditor.test.ts index bf4253711e..680a663b98 100644 --- a/packages/core/src/editor/BlockNoteEditor.test.ts +++ b/packages/core/src/editor/BlockNoteEditor.test.ts @@ -2,7 +2,7 @@ import { afterEach, expect, it } from "vite-plus/test"; import * as Y from "yjs"; import { - getBlockInfo, + getBlockInfoFromNode, getNearestBlockPos, } from "../api/getBlockInfoFromPos.js"; import { BlockNoteEditor } from "./BlockNoteEditor.js"; @@ -26,7 +26,7 @@ it("creates an editor", () => { const editor = BlockNoteEditor.create(); editorsToCleanup.push(editor); const posInfo = editor.transact((tr) => getNearestBlockPos(tr.doc, 2)); - const info = getBlockInfo(posInfo); + const info = getBlockInfoFromNode(posInfo.node, posInfo.posBeforeNode); expect(info.blockNoteType).toEqual("paragraph"); }); diff --git a/packages/core/src/editor/BlockNoteEditor.ts b/packages/core/src/editor/BlockNoteEditor.ts index 5f0f34a746..d685993714 100644 --- a/packages/core/src/editor/BlockNoteEditor.ts +++ b/packages/core/src/editor/BlockNoteEditor.ts @@ -38,7 +38,6 @@ import type { StyleSchema, StyleSpecs, } from "../schema/index.js"; -import { assertContainerSchemaInvariants } from "../schema/blocks/assertSchemaInvariants.js"; import "../style.css"; import { mergeCSSClasses } from "../util/browser.js"; import { EventEmitter } from "../util/EventEmitter.js"; @@ -581,8 +580,6 @@ export class BlockNoteEditor< this.pmSchema.cached.blockNoteEditor = this; - assertContainerSchemaInvariants(this.pmSchema); - this._tiptapEditor.on("mount", () => { this.headless = false; }); @@ -1063,8 +1060,8 @@ export class BlockNoteEditor< * @param blocksToInsert An array of partial blocks that should be inserted. * @param referenceBlock An identifier for an existing block, at which the new blocks should be inserted. * @param placement Where the blocks go relative to the `referenceBlock`: as its previous (`"before"`) or next - * (`"after"`) sibling, or nested inside it as its first (`"start"`) or last (`"end"`) children. Throws an error if - * the `referenceBlock` (or its parent, for `"before"`/`"after"`) doesn't accept the blocks there. + * (`"after"`) sibling, or nested inside it as its first (`"first-child"`) or last (`"last-child"`) children. Throws + * an error if the `referenceBlock` (or its parent, for `"before"`/`"after"`) doesn't accept the blocks there. */ public insertBlocks( blocksToInsert: PartialBlock[], diff --git a/packages/core/src/editor/managers/BlockManager.ts b/packages/core/src/editor/managers/BlockManager.ts index a33bfcab4b..ca4c62555e 100644 --- a/packages/core/src/editor/managers/BlockManager.ts +++ b/packages/core/src/editor/managers/BlockManager.ts @@ -154,7 +154,7 @@ export class BlockManager< * @param blocksToInsert An array of partial blocks that should be inserted. * @param referenceBlock An identifier for an existing block, at which the new blocks should be inserted. * @param placement Where the blocks go relative to the `referenceBlock`: as its previous (`"before"`) or next - * (`"after"`) sibling, or nested inside it as its first (`"start"`) or last (`"end"`) children. + * (`"after"`) sibling, or nested inside it as its first (`"first-child"`) or last (`"last-child"`) children. */ public insertBlocks( blocksToInsert: PartialBlock[], diff --git a/packages/core/src/editor/managers/ExtensionManager/extensions.ts b/packages/core/src/editor/managers/ExtensionManager/extensions.ts index 90cb91e432..3d2a3e5bda 100644 --- a/packages/core/src/editor/managers/ExtensionManager/extensions.ts +++ b/packages/core/src/editor/managers/ExtensionManager/extensions.ts @@ -39,7 +39,6 @@ import { UniqueID, } from "../../../extensions/tiptap-extensions/index.js"; import { BlockContainer, BlockGroup, Doc } from "../../../pm-nodes/index.js"; -import { isContainerType } from "../../../schema/blocks/children.js"; import type { BlockNoteEditor, BlockNoteEditorOptions, @@ -70,7 +69,7 @@ export function getDefaultTiptapExtensions( // block itself, so the id lives on its attrs rather than on a // wrapping blockContainer. ...Object.entries(editor.schema.blockSpecs) - .filter(([, spec]) => isContainerType((spec as any).config)) + .filter(([, spec]) => (spec as any).config.children !== undefined) .map(([type]) => type), ], setIdAttribute: options.setIdAttribute, diff --git a/packages/core/src/editor/managers/ExtensionManager/index.ts b/packages/core/src/editor/managers/ExtensionManager/index.ts index 71167e8f5a..b8973e3394 100644 --- a/packages/core/src/editor/managers/ExtensionManager/index.ts +++ b/packages/core/src/editor/managers/ExtensionManager/index.ts @@ -563,7 +563,7 @@ export class ExtensionManager { const blockInfo = getBlockInfoFromSelection(tr); if ( - !blockInfo.isWrappedBlock || + !blockInfo.hasContent || this.editor.schema.blockSchema[blockInfo.blockNoteType] ?.content !== "inline" ) { @@ -571,14 +571,14 @@ export class ExtensionManager { } tr.deleteRange(start, end); - updateBlockTr(tr, blockInfo.bnBlock.beforePos, replaceWith); + updateBlockTr(tr, blockInfo.block.beforePos, replaceWith); // updateBlockTr's replaceWith path leaves the selection after // the new block when the content is replaced wholesale (e.g. // when the rule returns content: []). Move the cursor back // inside the new block so the user can keep typing. setTextCursorPosition( tr, - getNodeId(blockInfo.bnBlock.node, tr.doc), + getNodeId(blockInfo.block.node, tr.doc), "start", ); return tr; diff --git a/packages/core/src/editor/transformPasted.ts b/packages/core/src/editor/transformPasted.ts index 033df48484..935e2b5bd2 100644 --- a/packages/core/src/editor/transformPasted.ts +++ b/packages/core/src/editor/transformPasted.ts @@ -66,7 +66,7 @@ function removeChild(node: Fragment, n: number) { * Wrap adjacent tableRow items in a table. * * This makes sure the content that we paste is always a table (and not a tableRow) - * A table works better for the remaing paste handling logic, as it's actually a blockContent node + * A table works better for the remaing paste handling logic, as it's actually a content node */ export function wrapTableRows(f: Fragment, schema: Schema) { const newItems: any[] = []; @@ -217,15 +217,15 @@ function retypeLeadingParagraphForEmptyTarget( } const blockInfo = getBlockInfoFromSelection(view.state); - const target = blockInfo.isWrappedBlock ? blockInfo.blockContent.node : null; if ( - !target || - target.type.name === "paragraph" || - target.type.spec.content !== "inline*" || - target.childCount > 0 + !blockInfo.hasContent || + blockInfo.content.node.type.name === "paragraph" || + blockInfo.contentKind !== "inline" || + !blockInfo.isContentEmpty ) { return null; } + const target = blockInfo.content.node; const blockGroup = fragment.firstChild; const blockContainer = blockGroup?.firstChild; @@ -277,9 +277,8 @@ function shouldApplyFix(fragment: Fragment, view: EditorView) { // for both paste and drop events. Drop events can potentially cause // issues as they don't always happen at the current selection. const blockInfo = getBlockInfoFromSelection(view.state); - if (blockInfo.isWrappedBlock) { - const selectedBlockHasTableContent = - blockInfo.blockContent.node.type.spec.content === "tableRow+"; + if (blockInfo.hasContent) { + const selectedBlockHasTableContent = blockInfo.contentKind === "table"; // Case for when we paste a single node with table content, i.e. a // table. Normally, we return true as we want to ensure the table is diff --git a/packages/core/src/exporter/Exporter.ts b/packages/core/src/exporter/Exporter.ts index de4bdeece5..af918fd669 100644 --- a/packages/core/src/exporter/Exporter.ts +++ b/packages/core/src/exporter/Exporter.ts @@ -11,7 +11,6 @@ import { StyledText, Styles, } from "../schema/index.js"; -import { isContainerType } from "../schema/blocks/children.js"; import type { BlockMapping, @@ -88,7 +87,7 @@ export abstract class Exporter< const spec = (this.blockNoteSchema.blockSpecs as Record)[ blockType ]; - return !!spec && isContainerType(spec.config); + return !!spec && spec.config.children !== undefined; } /** diff --git a/packages/core/src/extensions/SideMenu/SideMenu.ts b/packages/core/src/extensions/SideMenu/SideMenu.ts index 94c8dfd1c4..a6d3de046b 100644 --- a/packages/core/src/extensions/SideMenu/SideMenu.ts +++ b/packages/core/src/extensions/SideMenu/SideMenu.ts @@ -28,6 +28,7 @@ import { getDraggableBlockFromElement } from "../getDraggableBlockFromElement.js import { dragStart, unsetDragImage } from "./dragging.js"; import { getContainerChildAtCursor, + getDirectChildBlocks, hasHorizontalContainerAncestor, } from "./sideMenuContainerGeometry.js"; @@ -59,22 +60,22 @@ function getBlockFromCoords( adjustForHorizontalContainers && containerUIInfo.containerSelector && // Inside a container with side-by-side children (e.g. a columnList), - // the x position must be offset. The hovered coordinates land in the - // side menu's own gutter, which belongs to a different child. The + // the cursor is in the side menu's own gutter, so it lands on the + // container itself rather than on the block it lines up with. The // horizontal container can be any ancestor (the element may sit inside // a vertical child of it, like a block inside a column). hasHorizontalContainerAncestor(element, containerUIInfo) ) { - return getBlockFromCoords( - view, - { - // TODO can we do better than this? - left: coords.left + 50, // bit hacky, but if we're inside a column, offset x position to right to account for the width of sidemenu itself - top: coords.top, - }, - containerUIInfo, - false, - ); + // Walk down through the containers by their children's measured rects + // instead, which finds that block without guessing an x offset. + const cursor = { x: coords.left, y: coords.top }; + let target = element; + let child = getContainerChildAtCursor(target, cursor, containerUIInfo); + while (child) { + target = child; + child = getContainerChildAtCursor(target, cursor, containerUIInfo); + } + return getDraggableBlockFromElement(target, view, containerUIInfo); } return getDraggableBlockFromElement(element, view, containerUIInfo); } @@ -296,13 +297,13 @@ export class SideMenuView< show: true, referencePos: new DOMRect( container - ? // We anchor to the container's first block element (rather - // than the container itself, which may have padding or its own + ? // We anchor to the container's first child block (rather than + // the container itself, which may have padding or its own // chrome around the block area). This is a little weird since // this element is the first block, but since it's always // non-nested and we only take the x coordinate, it's ok. ( - container.querySelector('[data-node-type="blockOuter"]') ?? + getDirectChildBlocks(container, containerUIInfo)[0] ?? container.firstElementChild! ).getBoundingClientRect().x : ( diff --git a/packages/core/src/extensions/tiptap-extensions/KeyboardShortcuts/KeyboardShortcutsExtension.test.ts b/packages/core/src/extensions/tiptap-extensions/KeyboardShortcuts/KeyboardShortcutsExtension.test.ts index 2f1e601a35..2f3464a2db 100644 --- a/packages/core/src/extensions/tiptap-extensions/KeyboardShortcuts/KeyboardShortcutsExtension.test.ts +++ b/packages/core/src/extensions/tiptap-extensions/KeyboardShortcuts/KeyboardShortcutsExtension.test.ts @@ -110,6 +110,352 @@ function getTextContent(editor: BlockNoteEditor) { return text; } +/** + * Characterization tests for the Backspace/Delete/Enter/Tab handlers: they pin + * the current document transformations so the BlockInfo migration inside the + * handlers is provably behavior-preserving. + */ +function createEditorWithBlocks( + initialContent: any[], + cursor: { id: string; placement: "start" | "end" }, +) { + const editor = BlockNoteEditor.create({ schema, initialContent }); + editor.mount(document.createElement("div")); + editor.setTextCursorPosition(cursor.id, cursor.placement); + return editor; +} + +/** Compact structural view of the document for snapshotting. */ +function outline(blocks: any[]): any[] { + return blocks.map((b) => ({ + type: b.type, + text: Array.isArray(b.content) + ? b.content.map((c: any) => c.text ?? "").join("") + : undefined, + ...(b.children.length > 0 ? { children: outline(b.children) } : {}), + })); +} + +describe("KeyboardShortcutsExtension Backspace", () => { + it("merges a block into the previous one at block start", () => { + const editor = createEditorWithBlocks( + [ + { id: "a", type: "paragraph", content: "Hello" }, + { id: "b", type: "paragraph", content: "World" }, + ], + { id: "b", placement: "start" }, + ); + + pressKeys(editor, "Backspace"); + + expect(outline(editor.document)).toMatchInlineSnapshot(` + [ + { + "text": "HelloWorld", + "type": "paragraph", + }, + ] + `); + editor._tiptapEditor.destroy(); + }); + + it("merges into the previous block's deepest descendant", () => { + const editor = createEditorWithBlocks( + [ + { + id: "a", + type: "paragraph", + content: "Parent", + children: [{ id: "a1", type: "paragraph", content: "Nested" }], + }, + { id: "b", type: "paragraph", content: "World" }, + ], + { id: "b", placement: "start" }, + ); + + pressKeys(editor, "Backspace"); + + expect(outline(editor.document)).toMatchInlineSnapshot(` + [ + { + "children": [ + { + "text": "NestedWorld", + "type": "paragraph", + }, + ], + "text": "Parent", + "type": "paragraph", + }, + ] + `); + editor._tiptapEditor.destroy(); + }); + + it("lifts a nested first child at block start", () => { + const editor = createEditorWithBlocks( + [ + { + id: "a", + type: "paragraph", + content: "Parent", + children: [{ id: "a1", type: "paragraph", content: "Nested" }], + }, + ], + { id: "a1", placement: "start" }, + ); + + pressKeys(editor, "Backspace"); + + expect(outline(editor.document)).toMatchInlineSnapshot(` + [ + { + "text": "Parent", + "type": "paragraph", + }, + { + "text": "Nested", + "type": "paragraph", + }, + ] + `); + editor._tiptapEditor.destroy(); + }); + + it("deletes an empty block, moving its children out", () => { + const editor = createEditorWithBlocks( + [ + { id: "a", type: "paragraph", content: "Before" }, + { + id: "b", + type: "paragraph", + content: "", + children: [{ id: "b1", type: "paragraph", content: "Child" }], + }, + ], + { id: "b", placement: "start" }, + ); + + pressKeys(editor, "Backspace"); + + expect(outline(editor.document)).toMatchInlineSnapshot(` + [ + { + "text": "Before", + "type": "paragraph", + }, + { + "text": "Child", + "type": "paragraph", + }, + ] + `); + editor._tiptapEditor.destroy(); + }); +}); + +describe("KeyboardShortcutsExtension Delete", () => { + it("merges the next block in at block end", () => { + const editor = createEditorWithBlocks( + [ + { id: "a", type: "paragraph", content: "Hello" }, + { id: "b", type: "paragraph", content: "World" }, + ], + { id: "a", placement: "end" }, + ); + + pressKeys(editor, "Delete"); + + expect(outline(editor.document)).toMatchInlineSnapshot(` + [ + { + "text": "HelloWorld", + "type": "paragraph", + }, + ] + `); + editor._tiptapEditor.destroy(); + }); + + it("merges a next block that has children, un-nesting them", () => { + const editor = createEditorWithBlocks( + [ + { id: "a", type: "paragraph", content: "Hello" }, + { + id: "b", + type: "paragraph", + content: "World", + children: [ + { id: "b1", type: "paragraph", content: "Child 1" }, + { id: "b2", type: "paragraph", content: "Child 2" }, + ], + }, + ], + { id: "a", placement: "end" }, + ); + + pressKeys(editor, "Delete"); + + expect(outline(editor.document)).toMatchInlineSnapshot(` + [ + { + "text": "HelloWorld", + "type": "paragraph", + }, + { + "text": "Child 1", + "type": "paragraph", + }, + { + "text": "Child 2", + "type": "paragraph", + }, + ] + `); + editor._tiptapEditor.destroy(); + }); + + it("removes an empty next block, adopting its children", () => { + const editor = createEditorWithBlocks( + [ + { id: "a", type: "paragraph", content: "Hello" }, + { + id: "b", + type: "paragraph", + content: "", + children: [{ id: "b1", type: "paragraph", content: "Child" }], + }, + ], + { id: "a", placement: "end" }, + ); + + pressKeys(editor, "Delete"); + + expect(outline(editor.document)).toMatchInlineSnapshot(` + [ + { + "text": "Hello", + "type": "paragraph", + }, + { + "text": "Child", + "type": "paragraph", + }, + ] + `); + editor._tiptapEditor.destroy(); + }); + + it("removes an empty current block on Delete", () => { + const editor = createEditorWithBlocks( + [ + { id: "a", type: "paragraph", content: "" }, + { id: "b", type: "paragraph", content: "After" }, + ], + { id: "a", placement: "start" }, + ); + + pressKeys(editor, "Delete"); + + expect(outline(editor.document)).toMatchInlineSnapshot(` + [ + { + "text": "After", + "type": "paragraph", + }, + ] + `); + editor._tiptapEditor.destroy(); + }); +}); + +describe("KeyboardShortcutsExtension Enter", () => { + it("inserts an empty block above when Enter is pressed at the start", () => { + const editor = createEditorWithBlocks( + [{ id: "a", type: "paragraph", content: "Hello" }], + { id: "a", placement: "start" }, + ); + + pressKeys(editor, "Enter"); + + expect(outline(editor.document)).toMatchInlineSnapshot(` + [ + { + "text": "", + "type": "paragraph", + }, + { + "text": "Hello", + "type": "paragraph", + }, + ] + `); + editor._tiptapEditor.destroy(); + }); + + it("lifts an empty nested block on Enter", () => { + const editor = createEditorWithBlocks( + [ + { + id: "a", + type: "paragraph", + content: "Parent", + children: [{ id: "a1", type: "paragraph", content: "" }], + }, + ], + { id: "a1", placement: "start" }, + ); + + pressKeys(editor, "Enter"); + + expect(outline(editor.document)).toMatchInlineSnapshot(` + [ + { + "text": "Parent", + "type": "paragraph", + }, + { + "text": "", + "type": "paragraph", + }, + ] + `); + editor._tiptapEditor.destroy(); + }); +}); + +describe("KeyboardShortcutsExtension Shift-Tab", () => { + it("un-nests a nested block", () => { + const editor = createEditorWithBlocks( + [ + { + id: "a", + type: "paragraph", + content: "Parent", + children: [{ id: "a1", type: "paragraph", content: "Nested" }], + }, + ], + { id: "a1", placement: "start" }, + ); + + pressKeys(editor, "Shift-Tab"); + + expect(outline(editor.document)).toMatchInlineSnapshot(` + [ + { + "text": "Parent", + "type": "paragraph", + }, + { + "text": "Nested", + "type": "paragraph", + }, + ] + `); + editor._tiptapEditor.destroy(); + }); +}); + describe("KeyboardShortcutsExtension hardBreakShortcut", () => { it("inserts a hard break on Shift-Enter by default", () => { const editor = createEditor("paragraph"); diff --git a/packages/core/src/extensions/tiptap-extensions/KeyboardShortcuts/KeyboardShortcutsExtension.ts b/packages/core/src/extensions/tiptap-extensions/KeyboardShortcuts/KeyboardShortcutsExtension.ts index 31734d0096..db6ef22305 100644 --- a/packages/core/src/extensions/tiptap-extensions/KeyboardShortcuts/KeyboardShortcutsExtension.ts +++ b/packages/core/src/extensions/tiptap-extensions/KeyboardShortcuts/KeyboardShortcutsExtension.ts @@ -2,13 +2,7 @@ import { CommandProps, Extension } from "@tiptap/core"; import { Fragment, Node } from "prosemirror-model"; import { NodeSelection, TextSelection, Transaction } from "prosemirror-state"; -import { - getBottomNestedBlockInfo, - getNextBlockInfo, - getParentBlockInfo, - getPrevBlockInfo, - mergeBlocksCommand, -} from "../../../api/blockManipulation/commands/mergeBlocks/mergeBlocks.js"; +import { mergeBlocksCommand } from "../../../api/blockManipulation/commands/mergeBlocks/mergeBlocks.js"; import { liftItem, nestBlock, @@ -20,7 +14,7 @@ import { } from "../../../api/blockManipulation/containers/fixContainer.js"; import { ascendToInsertablePos, - descendToLastInsertionPos, + descendToInsertionPos, getAncestorContainers, getFirstLeafBlock, } from "../../../api/blockManipulation/containers/containerNav.js"; @@ -28,8 +22,14 @@ import { isSealed } from "../../../schema/blocks/children.js"; import { splitBlockCommand } from "../../../api/blockManipulation/commands/splitBlock/splitBlock.js"; import { updateBlockCommand } from "../../../api/blockManipulation/commands/updateBlock/updateBlock.js"; import { - getBlockInfoFromResolvedPos, + getBlockInfoAt, + getBlockInfoFromNode, getBlockInfoFromSelection, + getLastDescendantBlockInfo, + getNextBlockInfo, + getParentBlockInfo, + getPrevBlockInfo, + tableContentCaretPos, } from "../../../api/getBlockInfoFromPos.js"; import { BlockNoteEditor } from "../../../editor/BlockNoteEditor.js"; import { FilePanelExtension } from "../../FilePanel/FilePanel.js"; @@ -71,28 +71,28 @@ function moveBlockOutAndPlaceCaret( function selectSealedSiblingCommand(direction: "prev" | "next") { return ({ state, tr, dispatch }: CommandProps) => { const blockInfo = getBlockInfoFromSelection(state); - if (!blockInfo.isWrappedBlock) { + if (!blockInfo.hasContent) { return false; } const atEdge = direction === "prev" - ? state.selection.from === blockInfo.blockContent.beforePos + 1 - : state.selection.from === blockInfo.blockContent.afterPos - 1; + ? state.selection.from === blockInfo.contentStart + : state.selection.from === blockInfo.contentEnd; if (!atEdge || !state.selection.empty) { return false; } const sibling = ( direction === "prev" ? getPrevBlockInfo : getNextBlockInfo - )(state.doc, blockInfo.bnBlock.beforePos); - if (!sibling || !isSealed(sibling.bnBlock.node)) { + )(state.doc, blockInfo.block.beforePos); + if (!sibling || !isSealed(sibling.block.node)) { return false; } - if (dispatch && NodeSelection.isSelectable(sibling.bnBlock.node)) { + if (dispatch && NodeSelection.isSelectable(sibling.block.node)) { tr.setSelection( - NodeSelection.create(tr.doc, sibling.bnBlock.beforePos), + NodeSelection.create(tr.doc, sibling.block.beforePos), ).scrollIntoView(); } return true; @@ -119,18 +119,18 @@ export const KeyboardShortcutsExtension = Extension.create<{ () => commands.command(({ state }) => { const blockInfo = getBlockInfoFromSelection(state); - if (!blockInfo.isWrappedBlock) { + if (!blockInfo.hasContent) { return false; } const selectionAtBlockStart = - state.selection.from === blockInfo.blockContent.beforePos + 1; + state.selection.from === blockInfo.contentStart; const isParagraph = - blockInfo.blockContent.node.type.name === "paragraph"; + blockInfo.content.node.type.name === "paragraph"; if (selectionAtBlockStart && !isParagraph) { return commands.command( - updateBlockCommand(blockInfo.bnBlock.beforePos, { + updateBlockCommand(blockInfo.block.beforePos, { type: "paragraph", props: {}, }), @@ -143,13 +143,12 @@ export const KeyboardShortcutsExtension = Extension.create<{ () => commands.command(({ state, tr }) => { const blockInfo = getBlockInfoFromSelection(state); - if (!blockInfo.isWrappedBlock) { + if (!blockInfo.hasContent) { return false; } - const { blockContent } = blockInfo; const selectionAtBlockStart = - state.selection.from === blockContent.beforePos + 1; + state.selection.from === blockInfo.contentStart; if (selectionAtBlockStart) { return liftItem( @@ -169,28 +168,28 @@ export const KeyboardShortcutsExtension = Extension.create<{ () => commands.command(({ state }) => { const blockInfo = getBlockInfoFromSelection(state); - if (!blockInfo.isWrappedBlock) { + if (!blockInfo.hasContent) { return false; } - const { bnBlock: blockContainer, blockContent } = blockInfo; + const { block: blockContainer } = blockInfo; const prevBlockInfo = getPrevBlockInfo( state.doc, - blockInfo.bnBlock.beforePos, + blockInfo.block.beforePos, ); // If the previous block has no inline content, it can't be merged. // It's instead deleted, which is done later in the chan, so we // return early here. if ( !prevBlockInfo || - !prevBlockInfo.isWrappedBlock || - prevBlockInfo.blockContent.node.type.spec.content !== "inline*" + !prevBlockInfo.hasContent || + prevBlockInfo.contentKind !== "inline" ) { return false; } const selectionAtBlockStart = - state.selection.from === blockContent.beforePos + 1; + state.selection.from === blockInfo.contentStart; const selectionEmpty = state.selection.empty; const posBetweenBlocks = blockContainer.beforePos; @@ -211,53 +210,41 @@ export const KeyboardShortcutsExtension = Extension.create<{ () => commands.command(({ state, tr, dispatch }) => { const blockInfo = getBlockInfoFromSelection(state); - if (!blockInfo.isWrappedBlock) { + if (!blockInfo.hasContent) { return false; } const selectionAtBlockStart = - state.selection.from === blockInfo.blockContent.beforePos + 1; + state.selection.from === blockInfo.contentStart; if (!selectionAtBlockStart) { return false; } const prevBlockInfo = getPrevBlockInfo( state.doc, - blockInfo.bnBlock.beforePos, + blockInfo.block.beforePos, ); - if (!prevBlockInfo || prevBlockInfo.isWrappedBlock) { + if (!prevBlockInfo || prevBlockInfo.hasContent) { return false; } - const insertionPos = descendToLastInsertionPos( - prevBlockInfo.bnBlock.node, - prevBlockInfo.bnBlock.beforePos, - state.schema.nodes["blockContainer"], - { respectSealed: true }, + const blockContainerType = state.schema.nodes["blockContainer"]; + const insertion = descendToInsertionPos( + prevBlockInfo, + blockContainerType, + "last", ); - if (insertionPos === null) { - // When only a sealed boundary blocked the descent, the - // container can't be entered, so it's selected instead, and a - // second Backspace deletes it explicitly. A container with + if (insertion.pos === undefined) { + // A sealed container can't be entered, so it's selected instead, + // and a second Backspace deletes it explicitly. A container with // nowhere a `blockContainer` can land falls through as before. - // (The probe descends without `respectSealed`, i.e. through - // seals.) - const blockedBySeal = - descendToLastInsertionPos( - prevBlockInfo.bnBlock.node, - prevBlockInfo.bnBlock.beforePos, - state.schema.nodes["blockContainer"], - ) !== null; if ( - blockedBySeal && - NodeSelection.isSelectable(prevBlockInfo.bnBlock.node) + insertion.blockedBy === "seal" && + NodeSelection.isSelectable(prevBlockInfo.block.node) ) { if (dispatch) { tr.setSelection( - NodeSelection.create( - tr.doc, - prevBlockInfo.bnBlock.beforePos, - ), + NodeSelection.create(tr.doc, prevBlockInfo.block.beforePos), ).scrollIntoView(); } return true; @@ -266,15 +253,12 @@ export const KeyboardShortcutsExtension = Extension.create<{ } if (dispatch) { - tr.delete( - blockInfo.bnBlock.beforePos, - blockInfo.bnBlock.afterPos, - ); - tr.insert(insertionPos, blockInfo.bnBlock.node); - tr.setSelection( - TextSelection.near(tr.doc.resolve(insertionPos + 1)), - ); - + moveBlockOutAndPlaceCaret(tr, { + from: blockInfo.block.beforePos, + to: blockInfo.block.afterPos, + node: blockInfo.block.node, + insertAt: insertion.pos, + }); return true; } @@ -288,17 +272,17 @@ export const KeyboardShortcutsExtension = Extension.create<{ () => commands.command(({ state, tr, dispatch }) => { const blockInfo = getBlockInfoFromSelection(state); - if (!blockInfo.isWrappedBlock) { + if (!blockInfo.hasContent) { return false; } const selectionAtBlockStart = - tr.selection.from === blockInfo.blockContent.beforePos + 1; + tr.selection.from === blockInfo.contentStart; if (!selectionAtBlockStart) { return false; } - const $pos = tr.doc.resolve(blockInfo.bnBlock.beforePos); + const $pos = tr.doc.resolve(blockInfo.block.beforePos); const prevBlock = $pos.nodeBefore; if (prevBlock) { @@ -332,27 +316,28 @@ export const KeyboardShortcutsExtension = Extension.create<{ : null; const insertionPos = prevSibling - ? descendToLastInsertionPos( - prevSibling, - containerBeforePos - prevSibling.nodeSize, + ? descendToInsertionPos( + getBlockInfoFromNode( + prevSibling, + containerBeforePos - prevSibling.nodeSize, + ), blockContainerType, - { respectSealed: true }, - ) + "last", + ).pos : ascendToInsertablePos( tr.doc, containerBeforePos, blockContainerType, - { respectSealed: true }, ); - if (insertionPos === null) { + if (insertionPos === undefined) { return false; } if (dispatch) { moveBlockOutAndPlaceCaret(tr, { - from: blockInfo.bnBlock.beforePos, - to: blockInfo.bnBlock.afterPos, - node: blockInfo.bnBlock.node, + from: blockInfo.block.beforePos, + to: blockInfo.block.afterPos, + node: blockInfo.block.node, insertAt: insertionPos, }); } @@ -364,63 +349,57 @@ export const KeyboardShortcutsExtension = Extension.create<{ () => commands.command(({ state }) => { const blockInfo = getBlockInfoFromSelection(state); - if (!blockInfo.isWrappedBlock) { + if (!blockInfo.hasContent) { return false; } const blockEmpty = - blockInfo.blockContent.node.childCount === 0 && - blockInfo.blockContent.node.type.spec.content === "inline*"; + blockInfo.isContentEmpty && blockInfo.contentKind === "inline"; if (blockEmpty) { const prevBlockInfo = getPrevBlockInfo( state.doc, - blockInfo.bnBlock.beforePos, + blockInfo.block.beforePos, ); if (!prevBlockInfo) { return false; } - const bottomNestedPrevBlockInfo = getBottomNestedBlockInfo( + const bottomNestedPrevBlockInfo = getLastDescendantBlockInfo( state.doc, prevBlockInfo, ); - if (!bottomNestedPrevBlockInfo.isWrappedBlock) { + if (!bottomNestedPrevBlockInfo.hasContent) { return false; } let chainedCommands = chain(); // Moves the children the current block. - if (blockInfo.childContainer) { + if (blockInfo.children) { chainedCommands.insertContentAt( - blockInfo.bnBlock.afterPos, - blockInfo.childContainer?.node.content, + blockInfo.block.afterPos, + blockInfo.children?.node.content, ); } if ( - bottomNestedPrevBlockInfo.blockContent.node.type.spec - .content === "tableRow+" + bottomNestedPrevBlockInfo.content.node.type.spec.content === + "tableRow+" ) { - const tableBlockEndPos = blockInfo.bnBlock.beforePos - 1; - const tableBlockContentEndPos = tableBlockEndPos - 1; - const lastRowEndPos = tableBlockContentEndPos - 1; - const lastCellEndPos = lastRowEndPos - 1; - const lastCellParagraphEndPos = lastCellEndPos - 1; - chainedCommands = chainedCommands.setTextSelection( - lastCellParagraphEndPos, + tableContentCaretPos( + bottomNestedPrevBlockInfo.content, + "end", + ), ); } else if ( - bottomNestedPrevBlockInfo.blockContent.node.type.spec - .content === "" + bottomNestedPrevBlockInfo.content.node.type.spec.content === "" ) { chainedCommands = chainedCommands.setNodeSelection( - bottomNestedPrevBlockInfo.blockContent.beforePos, + bottomNestedPrevBlockInfo.content.beforePos, ); } else { - const blockContentEndPos = - bottomNestedPrevBlockInfo.blockContent.afterPos - 1; + const blockContentEndPos = bottomNestedPrevBlockInfo.contentEnd; chainedCommands = chainedCommands.setTextSelection(blockContentEndPos); @@ -428,8 +407,8 @@ export const KeyboardShortcutsExtension = Extension.create<{ return chainedCommands .deleteRange({ - from: blockInfo.bnBlock.beforePos, - to: blockInfo.bnBlock.afterPos, + from: blockInfo.block.beforePos, + to: blockInfo.block.afterPos, }) .scrollIntoView() .run(); @@ -444,56 +423,50 @@ export const KeyboardShortcutsExtension = Extension.create<{ commands.command(({ state }) => { const blockInfo = getBlockInfoFromSelection(state); - if (!blockInfo.isWrappedBlock) { + if (!blockInfo.hasContent) { return false; } const selectionAtBlockStart = - state.selection.from === blockInfo.blockContent.beforePos + 1; + state.selection.from === blockInfo.contentStart; const selectionEmpty = state.selection.empty; const prevBlockInfo = getPrevBlockInfo( state.doc, - blockInfo.bnBlock.beforePos, + blockInfo.block.beforePos, ); if (prevBlockInfo && selectionAtBlockStart && selectionEmpty) { - // The sealed-aware descent stops at a sealed container instead - // of finding an (empty) block inside it, so the current block - // is never cut in across the boundary. - const bottomBlock = getBottomNestedBlockInfo( + // The descent stops at a sealed container instead of finding an + // (empty) block inside it, so the current block is never cut in + // across the boundary. A container has no content, so the guard + // below rejects it. + const bottomBlock = getLastDescendantBlockInfo( state.doc, prevBlockInfo, - { stopAtSealed: true }, ); - if (!bottomBlock.isWrappedBlock) { - return false; - } - // A sealed content container also stops the descent; deleting - // it here would take its children with it. - if (isSealed(bottomBlock.bnBlock.node)) { + if (!bottomBlock.hasContent) { return false; } const prevBlockNotTableAndNoContent = - bottomBlock.blockContent.node.type.spec.content === "" || - (bottomBlock.blockContent.node.type.spec.content === - "inline*" && - bottomBlock.blockContent.node.childCount === 0); + bottomBlock.contentKind === "none" || + (bottomBlock.contentKind === "inline" && + bottomBlock.isContentEmpty); if (prevBlockNotTableAndNoContent) { return chain() .cut( { - from: blockInfo.bnBlock.beforePos, - to: blockInfo.bnBlock.afterPos, + from: blockInfo.block.beforePos, + to: blockInfo.block.afterPos, }, - bottomBlock.bnBlock.afterPos, + bottomBlock.block.afterPos, ) .deleteRange({ - from: bottomBlock.bnBlock.beforePos, - to: bottomBlock.bnBlock.afterPos, + from: bottomBlock.block.beforePos, + to: bottomBlock.block.afterPos, }) .run(); } @@ -515,55 +488,54 @@ export const KeyboardShortcutsExtension = Extension.create<{ () => commands.command(({ state }) => { const blockInfo = getBlockInfoFromSelection(state); - if (!blockInfo.isWrappedBlock || !blockInfo.childContainer) { + if (!blockInfo.hasContent || !blockInfo.children) { return false; } - const { blockContent, childContainer } = blockInfo; + const { children } = blockInfo; // A container allowed to hold no children still has a child // container node, but no first child to pull anything out of. - if (childContainer.node.childCount === 0) { + if (children.node.childCount === 0) { return false; } const selectionAtBlockEnd = - state.selection.from === blockContent.afterPos - 1; + state.selection.from === blockInfo.contentEnd; const selectionEmpty = state.selection.empty; - const firstChildBlockInfo = getBlockInfoFromResolvedPos( - state.doc.resolve(childContainer.beforePos + 1), + const firstChildBlockInfo = getBlockInfoAt( + state.doc, + children.childrenStart, ); - if (!firstChildBlockInfo.isWrappedBlock) { + if (!firstChildBlockInfo.hasContent) { return false; } if (selectionAtBlockEnd && selectionEmpty) { - const firstChildBlockContent = - firstChildBlockInfo.blockContent.node; + const firstChildBlockContent = firstChildBlockInfo.content.node; const firstChildBlockHasInlineContent = - firstChildBlockContent.type.spec.content === "inline*"; - const blockHasInlineContent = - blockContent.node.type.spec.content === "inline*"; + firstChildBlockInfo.contentKind === "inline"; + const blockHasInlineContent = blockInfo.contentKind === "inline"; return ( chain() // Un-nests child block's children if necessary. .insertContentAt( - firstChildBlockInfo.bnBlock.afterPos, - firstChildBlockInfo.childContainer?.node.content || + firstChildBlockInfo.block.afterPos, + firstChildBlockInfo.children?.node.content || Fragment.empty, ) .deleteRange( // Deletes whole child container if there's only one // child. - childContainer.node.childCount === 1 + children.node.childCount === 1 ? { - from: childContainer.beforePos, - to: childContainer.afterPos, + from: children.beforePos, + to: children.afterPos, } : { - from: firstChildBlockInfo.bnBlock.beforePos, - to: firstChildBlockInfo.bnBlock.afterPos, + from: firstChildBlockInfo.block.beforePos, + to: firstChildBlockInfo.block.afterPos, }, ) // Appends inline content from child block if possible. @@ -591,21 +563,21 @@ export const KeyboardShortcutsExtension = Extension.create<{ () => commands.command(({ state }) => { const blockInfo = getBlockInfoFromSelection(state); - if (!blockInfo.isWrappedBlock) { + if (!blockInfo.hasContent) { return false; } - const { bnBlock: blockContainer, blockContent } = blockInfo; + const { block: blockContainer } = blockInfo; const nextBlockInfo = getNextBlockInfo( state.doc, - blockInfo.bnBlock.beforePos, + blockInfo.block.beforePos, ); - if (!nextBlockInfo || !nextBlockInfo.isWrappedBlock) { + if (!nextBlockInfo || !nextBlockInfo.hasContent) { return false; } const selectionAtBlockEnd = - state.selection.from === blockContent.afterPos - 1; + state.selection.from === blockInfo.contentEnd; const selectionEmpty = state.selection.empty; const posBetweenBlocks = blockContainer.afterPos; @@ -624,39 +596,35 @@ export const KeyboardShortcutsExtension = Extension.create<{ () => commands.command(({ state, tr, dispatch }) => { const blockInfo = getBlockInfoFromSelection(state); - if (!blockInfo.isWrappedBlock) { + if (!blockInfo.hasContent) { return false; } const selectionAtBlockEnd = - state.selection.from === blockInfo.blockContent.afterPos - 1; + state.selection.from === blockInfo.contentEnd; if (!selectionAtBlockEnd) { return false; } const nextBlockInfo = getNextBlockInfo( state.doc, - blockInfo.bnBlock.beforePos, + blockInfo.block.beforePos, ); - if (!nextBlockInfo || nextBlockInfo.isWrappedBlock) { + if (!nextBlockInfo || nextBlockInfo.hasContent) { return false; } - const firstLeaf = getFirstLeafBlock( - nextBlockInfo.bnBlock.node, - nextBlockInfo.bnBlock.beforePos, - { respectSealed: true }, - ); + const firstLeaf = getFirstLeafBlock(nextBlockInfo); if (!firstLeaf) { return false; } if (dispatch) { moveBlockOutAndPlaceCaret(tr, { - from: firstLeaf.beforePos, - to: firstLeaf.beforePos + firstLeaf.node.nodeSize, - node: firstLeaf.node, - insertAt: blockInfo.bnBlock.afterPos, + from: firstLeaf.block.beforePos, + to: firstLeaf.block.afterPos, + node: firstLeaf.block.node, + insertAt: blockInfo.block.afterPos, }); return true; @@ -671,17 +639,17 @@ export const KeyboardShortcutsExtension = Extension.create<{ () => commands.command(({ state, tr, dispatch }) => { const blockInfo = getBlockInfoFromSelection(state); - if (!blockInfo.isWrappedBlock) { + if (!blockInfo.hasContent) { return false; } const selectionAtBlockEnd = - tr.selection.from === blockInfo.blockContent.afterPos - 1; + tr.selection.from === blockInfo.contentEnd; if (!selectionAtBlockEnd) { return false; } - const $pos = tr.doc.resolve(blockInfo.bnBlock.afterPos); + const $pos = tr.doc.resolve(blockInfo.block.afterPos); const nextBlock = $pos.nodeAfter; if (nextBlock) { @@ -716,21 +684,19 @@ export const KeyboardShortcutsExtension = Extension.create<{ // The block to pull in: the next node itself, or its first leaf // block when it's a container. - const target = isContainerNode(nextNode.type) - ? getFirstLeafBlock(nextNode, $boundary.pos, { - respectSealed: true, - }) - : { node: nextNode, beforePos: $boundary.pos }; + const target = getFirstLeafBlock( + getBlockInfoFromNode(nextNode, $boundary.pos), + ); if (!target) { return false; } if (dispatch) { moveBlockOutAndPlaceCaret(tr, { - from: target.beforePos, - to: target.beforePos + target.node.nodeSize, - node: target.node, - insertAt: blockInfo.bnBlock.afterPos, + from: target.block.beforePos, + to: target.block.afterPos, + node: target.block.node, + insertAt: blockInfo.block.afterPos, }); } @@ -744,13 +710,12 @@ export const KeyboardShortcutsExtension = Extension.create<{ () => commands.command(({ state }) => { const blockInfo = getBlockInfoFromSelection(state); - if (!blockInfo.isWrappedBlock) { + if (!blockInfo.hasContent) { return false; } - const { blockContent } = blockInfo; const selectionAtBlockEnd = - state.selection.from === blockContent.afterPos - 1; + state.selection.from === blockInfo.contentEnd; const selectionEmpty = state.selection.empty; if (selectionAtBlockEnd && selectionEmpty) { @@ -768,42 +733,40 @@ export const KeyboardShortcutsExtension = Extension.create<{ !parentBlockInfo || // Never climbs past a sealed boundary. A block found // there would be pulled in across it. - isSealed(parentBlockInfo.bnBlock.node) + isSealed(parentBlockInfo.block.node) ) { return undefined; } return getNextBlockInfoAtAnyLevel( doc, - parentBlockInfo.bnBlock.beforePos, + parentBlockInfo.block.beforePos, ); }; const nextBlockInfo = getNextBlockInfoAtAnyLevel( state.doc, - blockInfo.bnBlock.beforePos, + blockInfo.block.beforePos, ); - if (!nextBlockInfo || !nextBlockInfo.isWrappedBlock) { + if (!nextBlockInfo || !nextBlockInfo.hasContent) { return false; } - const nextBlockContent = nextBlockInfo.blockContent.node; + const nextBlockContent = nextBlockInfo.content.node; const nextBlockHasInlineContent = - nextBlockContent.type.spec.content === "inline*"; - const blockHasInlineContent = - blockContent.node.type.spec.content === "inline*"; + nextBlockInfo.contentKind === "inline"; + const blockHasInlineContent = blockInfo.contentKind === "inline"; return ( chain() // Un-nests next block's children if necessary. .insertContentAt( - nextBlockInfo.bnBlock.afterPos, - nextBlockInfo.childContainer?.node.content || - Fragment.empty, + nextBlockInfo.block.afterPos, + nextBlockInfo.children?.node.content || Fragment.empty, ) .deleteRange({ - from: nextBlockInfo.bnBlock.beforePos, - to: nextBlockInfo.bnBlock.afterPos, + from: nextBlockInfo.block.beforePos, + to: nextBlockInfo.block.afterPos, }) // Appends inline content from child block if possible. .insertContentAt( @@ -825,54 +788,42 @@ export const KeyboardShortcutsExtension = Extension.create<{ () => commands.command(({ state }) => { const blockInfo = getBlockInfoFromSelection(state); - if (!blockInfo.isWrappedBlock) { + if (!blockInfo.hasContent) { return false; } const blockEmpty = - blockInfo.blockContent.node.childCount === 0 && - blockInfo.blockContent.node.type.spec.content === "inline*"; + blockInfo.isContentEmpty && blockInfo.contentKind === "inline"; if (blockEmpty) { const nextBlockInfo = getNextBlockInfo( state.doc, - blockInfo.bnBlock.beforePos, + blockInfo.block.beforePos, ); - if (!nextBlockInfo || !nextBlockInfo.isWrappedBlock) { + if (!nextBlockInfo || !nextBlockInfo.hasContent) { return false; } let chainedCommands = chain(); - if ( - nextBlockInfo.blockContent.node.type.spec.content === - "tableRow+" - ) { - const tableBlockStartPos = blockInfo.bnBlock.afterPos + 1; - const tableBlockContentStartPos = tableBlockStartPos + 1; - const firstRowStartPos = tableBlockContentStartPos + 1; - const firstCellStartPos = firstRowStartPos + 1; - const firstCellParagraphStartPos = firstCellStartPos + 1; - + if (nextBlockInfo.contentKind === "table") { chainedCommands = chainedCommands.setTextSelection( - firstCellParagraphStartPos, + tableContentCaretPos(nextBlockInfo.content, "start"), ); - } else if ( - nextBlockInfo.blockContent.node.type.spec.content === "" - ) { + } else if (nextBlockInfo.contentKind === "none") { chainedCommands = chainedCommands.setNodeSelection( - nextBlockInfo.blockContent.beforePos, + nextBlockInfo.content.beforePos, ); } else { chainedCommands = chainedCommands.setTextSelection( - nextBlockInfo.blockContent.beforePos + 1, + nextBlockInfo.contentStart, ); } return chainedCommands .deleteRange({ - from: blockInfo.bnBlock.beforePos, - to: blockInfo.bnBlock.afterPos, + from: blockInfo.block.beforePos, + to: blockInfo.block.afterPos, }) .scrollIntoView() .run(); @@ -887,45 +838,40 @@ export const KeyboardShortcutsExtension = Extension.create<{ commands.command(({ state }) => { const blockInfo = getBlockInfoFromSelection(state); - if (!blockInfo.isWrappedBlock) { + if (!blockInfo.hasContent) { return false; } const selectionAtBlockEnd = - state.selection.from === blockInfo.blockContent.afterPos - 1; + state.selection.from === blockInfo.contentEnd; const selectionEmpty = state.selection.empty; const nextBlockInfo = getNextBlockInfo( state.doc, - blockInfo.bnBlock.beforePos, + blockInfo.block.beforePos, ); if (!nextBlockInfo) { return false; } - if (!nextBlockInfo.isWrappedBlock) { + if (!nextBlockInfo.hasContent) { return false; } if (nextBlockInfo && selectionAtBlockEnd && selectionEmpty) { const nextBlockNotTableAndNoContent = - nextBlockInfo.blockContent.node.type.spec.content === "" || - (nextBlockInfo.blockContent.node.type.spec.content === - "inline*" && - nextBlockInfo.blockContent.node.childCount === 0); + nextBlockInfo.contentKind === "none" || + (nextBlockInfo.contentKind === "inline" && + nextBlockInfo.isContentEmpty); if (nextBlockNotTableAndNoContent) { - const childBlocks = - nextBlockInfo.bnBlock.node.lastChild!.content; return chain() .deleteRange({ - from: nextBlockInfo.bnBlock.beforePos, - to: nextBlockInfo.bnBlock.afterPos, + from: nextBlockInfo.block.beforePos, + to: nextBlockInfo.block.afterPos, }) .insertContentAt( - blockInfo.bnBlock.afterPos, - nextBlockInfo.bnBlock.node.childCount === 2 - ? childBlocks - : null, + blockInfo.block.afterPos, + nextBlockInfo.children?.node.content ?? null, ) .run(); } @@ -942,10 +888,10 @@ export const KeyboardShortcutsExtension = Extension.create<{ () => commands.command(({ state, tr }) => { const blockInfo = getBlockInfoFromSelection(state); - if (!blockInfo.isWrappedBlock) { + if (!blockInfo.hasContent) { return false; } - const { bnBlock: blockContainer, blockContent } = blockInfo; + const { block: blockContainer } = blockInfo; const { depth } = state.doc.resolve(blockContainer.beforePos); @@ -953,7 +899,7 @@ export const KeyboardShortcutsExtension = Extension.create<{ state.selection.$anchor.parentOffset === 0; const selectionEmpty = state.selection.anchor === state.selection.head; - const blockEmpty = blockContent.node.childCount === 0; + const blockEmpty = blockInfo.isContentEmpty; const blockIndented = depth > 1; if ( @@ -1041,25 +987,25 @@ export const KeyboardShortcutsExtension = Extension.create<{ () => commands.command(({ state, tr, dispatch }) => { const blockInfo = getBlockInfoFromSelection(state); - if (!blockInfo.isWrappedBlock) { + if (!blockInfo.hasContent) { return false; } const selectionEmpty = state.selection.anchor === state.selection.head; - const blockEmpty = blockInfo.blockContent.node.childCount === 0; + const blockEmpty = blockInfo.isContentEmpty; if (!selectionEmpty || !blockEmpty) { return false; } - const $pos = tr.doc.resolve(blockInfo.bnBlock.beforePos); + const $pos = tr.doc.resolve(blockInfo.block.beforePos); const parentBlock = $pos.node(); if (!isContainerNode(parentBlock.type)) { return false; } // Only fires on the container's last child. - if (tr.doc.resolve(blockInfo.bnBlock.afterPos).nodeAfter !== null) { + if (tr.doc.resolve(blockInfo.block.afterPos).nodeAfter !== null) { return false; } @@ -1072,18 +1018,17 @@ export const KeyboardShortcutsExtension = Extension.create<{ tr.doc, $pos.after(), state.schema.nodes["blockContainer"], - { respectSealed: true }, "after", ); - if (containerAfterPos === null) { + if (containerAfterPos === undefined) { return false; } if (dispatch) { moveBlockOutAndPlaceCaret(tr, { - from: blockInfo.bnBlock.beforePos, - to: blockInfo.bnBlock.afterPos, - node: blockInfo.bnBlock.node, + from: blockInfo.block.beforePos, + to: blockInfo.block.afterPos, + node: blockInfo.block.node, insertAt: containerAfterPos, }); tr.scrollIntoView(); @@ -1096,16 +1041,16 @@ export const KeyboardShortcutsExtension = Extension.create<{ () => commands.command(({ state, dispatch, tr }) => { const blockInfo = getBlockInfoFromSelection(state); - if (!blockInfo.isWrappedBlock) { + if (!blockInfo.hasContent) { return false; } - const { bnBlock: blockContainer, blockContent } = blockInfo; + const { block: blockContainer } = blockInfo; const selectionAtBlockStart = state.selection.$anchor.parentOffset === 0; const selectionEmpty = state.selection.anchor === state.selection.head; - const blockEmpty = blockContent.node.childCount === 0; + const blockEmpty = blockInfo.isContentEmpty; if (selectionAtBlockStart && selectionEmpty && blockEmpty) { const newBlockInsertionPos = blockContainer.afterPos; @@ -1121,7 +1066,7 @@ export const KeyboardShortcutsExtension = Extension.create<{ [ state.schema.nodes["paragraph"].createAndFill() || undefined, - blockInfo.childContainer?.node, + blockInfo.children?.node, ].filter((node) => node !== undefined), )!; @@ -1134,10 +1079,10 @@ export const KeyboardShortcutsExtension = Extension.create<{ // Deletes old block's children, as they have been moved to // the new one. - if (blockInfo.childContainer) { + if (blockInfo.children) { tr.delete( - blockInfo.childContainer.beforePos, - blockInfo.childContainer.afterPos, + blockInfo.children.beforePos, + blockInfo.children.afterPos, ); } } @@ -1152,14 +1097,13 @@ export const KeyboardShortcutsExtension = Extension.create<{ () => commands.command(({ state, chain }) => { const blockInfo = getBlockInfoFromSelection(state); - if (!blockInfo.isWrappedBlock) { + if (!blockInfo.hasContent) { return false; } - const { blockContent } = blockInfo; const selectionAtBlockStart = state.selection.$anchor.parentOffset === 0; - const blockEmpty = blockContent.node.childCount === 0; + const blockEmpty = blockInfo.isContentEmpty; if (!blockEmpty) { chain() diff --git a/packages/core/src/internal.ts b/packages/core/src/internal.ts index 8a50e9f2f3..5fe72ad8ff 100644 --- a/packages/core/src/internal.ts +++ b/packages/core/src/internal.ts @@ -6,9 +6,8 @@ * can use it. Not part of the public API: anything here may change in any * release, without a major version bump or a deprecation. * - * The public counterparts stay on the root entrypoint: `isContainerType`, - * `isContainerNode`, and the `children` config types (`ChildrenConfig`, - * `ChildrenAllow`). + * The public counterparts stay on the root entrypoint: `isContainerNode` and + * the `children` config types (`ChildrenConfig`, `ChildrenAllow`). */ // How a `children` config compiles to a ProseMirror content expression, and @@ -20,19 +19,9 @@ export { CONTAINER_NODE_PRIORITY, childrenContentExpression, containerNodePriority, - getChildrenConfig, - isPlaceableAnywhere, resolveChildren, } from "./schema/blocks/children.js"; -// Validation of `children` configs, run when a schema is built. -export { - validateChildrenConfigs, - validateContainerRunsBefore, -} from "./schema/blocks/validateChildren.js"; - -export { assertContainerSchemaInvariants } from "./schema/blocks/assertSchemaInvariants.js"; - // The attributes a container block's root element carries, and the three ways // they get there (node view, HTML serialization, framework render). export { @@ -44,7 +33,6 @@ export { export { fixContainer, fixContainersById, - flattenNonInsertableBlocks, isEmptyContainerChild, removeEmptyChildren, } from "./api/blockManipulation/containers/fixContainer.js"; @@ -52,8 +40,7 @@ export { // Position-based navigation through arbitrarily nested containers. export { ascendToInsertablePos, - descendToFirstInsertionPos, - descendToLastInsertionPos, + descendToInsertionPos, getAncestorContainers, getFirstLeafBlock, } from "./api/blockManipulation/containers/containerNav.js"; diff --git a/packages/core/src/pm-nodes/README.md b/packages/core/src/pm-nodes/README.md index be57ead212..4577caf841 100644 --- a/packages/core/src/pm-nodes/README.md +++ b/packages/core/src/pm-nodes/README.md @@ -99,6 +99,10 @@ We use Prosemirror "groups" to help organize this schema. Here is a list of the _Note that the last two groups, `bnBlock` and `childContainer`, are not used anywhere in the schema. They are however helpful while programming. For example, we can check whether a node is a `bnBlock`, and then we know it corresponds to a BlockNote Block. Or, we can check whether a node is a `childContainer`, and then we know it's a container of a BlockNote Block's `children`. See `getBlockInfoFromPos` for an example of how this is used._ +## Relation to container blocks + +The `blockContainer` + `blockGroup` pair predates the container-blocks API, but it behaves exactly like a container block configured `children: { allow: "any", min: 1, whenEmptied: "unwrap", boundary: "open" }`: any block may nest, a nested `blockGroup` disappears when its last child does, and nothing blocks selections at its edge. The code shares what it can (reading children through `BlockInfo.children`, writing them through `childrenHolder.ts`, the group-membership predicates in `children.ts`), but the node pair itself stays: collapsing regular blocks onto the container mechanism would change the ProseMirror tree of every existing document, which is a document-format migration (collaboration data, HTML round-trip, and snapshots all pin the current shape), not a refactor. + ## Example document ```xml diff --git a/packages/core/src/schema/blocks/assertSchemaInvariants.ts b/packages/core/src/schema/blocks/assertSchemaInvariants.ts deleted file mode 100644 index 3fc2263159..0000000000 --- a/packages/core/src/schema/blocks/assertSchemaInvariants.ts +++ /dev/null @@ -1,98 +0,0 @@ -import { Fragment, type Schema } from "prosemirror-model"; - -import { - ANY_CONTAINER_GROUP, - getChildrenConfig, - isContainerNode, - isPlaceableAnywhere, -} from "./children.js"; - -/** - * Checks the structural properties the rest of the container machinery - * assumes, once, when the ProseMirror schema is built. - * - * Each property is otherwise guaranteed only by a chain of implicit reasoning - * spread across several files. Asserting them here turns silent breakage into - * a startup error naming the cause. - */ -export function assertContainerSchemaInvariants(pmSchema: Schema) { - assertBlockGroupFillsWithBlockContainer(pmSchema); - assertContainersAreFillable(pmSchema); - assertAnyContainerGroupMatchesConfigs(pmSchema); -} - -/** - * `blockGroup` must auto-fill with `blockContainer` rather than with some - * container block type. - * - * Today this holds because container nodes register below `blockContainer`'s - * priority, which drives TipTap's registration order, which drives the order - * ProseMirror resolves a group into types, which drives what `fillBefore` - * picks. Every link in that chain is implicit, and Yjs document - * initialization depends on the result (see `FixUpSchema`, which reads the - * first auto-filled child expecting it to be the id-carrying - * `blockContainer`). - */ -function assertBlockGroupFillsWithBlockContainer(pmSchema: Schema) { - const defaultType = pmSchema.nodes["blockGroup"]?.contentMatch.defaultType; - - if (defaultType?.name !== "blockContainer") { - throw new Error( - `BlockNote schema invariant broken: \`blockGroup\` auto-fills with "${defaultType?.name}" instead of "blockContainer". ` + - "Container block nodes must register at a lower priority than `blockContainer` (see CONTAINER_NODE_PRIORITY). " + - "Yjs document initialization depends on this (see FixUpSchema).", - ); - } -} - -/** - * The `anyContainer` group must contain exactly the container blocks - * placeable anywhere. It is what the `allow` container wildcards (`"any"`, - * `"containers"`) compile to. Generated nodes always get this right; a - * hand-written container node that forgets the group would silently drop out - * of every wildcard `allow`, so the mismatch is reported here instead. - */ -function assertAnyContainerGroupMatchesConfigs(pmSchema: Schema) { - for (const type of Object.values(pmSchema.nodes)) { - const blockConfig = type.spec.blockConfig; - if (!blockConfig || blockConfig.type !== type.name) { - continue; - } - - const shouldBeInGroup = - getChildrenConfig(blockConfig) !== undefined && - isPlaceableAnywhere(blockConfig); - if (shouldBeInGroup !== type.isInGroup(ANY_CONTAINER_GROUP)) { - throw new Error( - shouldBeInGroup - ? `BlockNote schema invariant broken: container block "${type.name}" is placeable anywhere but its node is not in the "${ANY_CONTAINER_GROUP}" group, ` + - `so wildcard \`allow\` containers would not accept it. A hand-written container node must include the group itself.` - : `BlockNote schema invariant broken: node "${type.name}" is in the "${ANY_CONTAINER_GROUP}" group but its block config does not make it a container placeable anywhere.`, - ); - } - } -} - -/** - * Every container must be creatable empty, or inserting one throws a raw - * ProseMirror error at the call site instead of here. - * - * This asks ProseMirror directly rather than re-deriving the answer from the - * config, so it catches combinations a hand-written check would miss. - * `whenEmptied: "refill"`'s empty-fill fallback uses the same `fillBefore`, - * so this also guarantees that a refill repair can always complete. - */ -function assertContainersAreFillable(pmSchema: Schema) { - for (const type of Object.values(pmSchema.nodes)) { - if (!isContainerNode(type)) { - continue; - } - - if (!type.contentMatch.fillBefore(Fragment.empty, true)) { - throw new Error( - `Container block "${type.name}" can never be created empty: its \`children\` config compiles to \`${type.spec.content}\`, ` + - "which ProseMirror cannot auto-fill. Lower the minimum child count, or allow regular blocks.", - ); - } - } -} diff --git a/packages/core/src/schema/blocks/children.test.ts b/packages/core/src/schema/blocks/children.test.ts index 321283d966..e9cf16c064 100644 --- a/packages/core/src/schema/blocks/children.test.ts +++ b/packages/core/src/schema/blocks/children.test.ts @@ -67,12 +67,12 @@ describe("resolveChildren", () => { expect(resolveChildren({ allow })).toMatchObject(expected); }); - it("applies the defaults: min 1, unbounded, refill, isolated", () => { + it("applies the defaults: min 1, unbounded, refill, open", () => { const resolved = resolveChildren({ allow: "any" }); expect(resolved.min).toBe(1); expect(resolved.max).toBeUndefined(); expect(resolved.whenEmptied).toBe("refill"); - expect(resolved.boundary).toBe("isolated"); + expect(resolved.boundary).toBe("open"); }); it("returns the same object for the same config, without mutating it", () => { @@ -116,134 +116,35 @@ describe("validateChildrenConfigs", () => { ).not.toThrow(); }); - // Malformed configs, each rejected with a specific message (JS consumers - // don't get the type errors TS consumers do). `allow: ["heading"]` used to - // silently compile to "any regular block", so naming a regular block is a - // hard error until per-type filtering is supported. + // Only what nothing else catches. ProseMirror parses `{min,max}` without + // comparing the two, so an inverted range silently becomes "exactly min"; + // and `allow: ["heading"]` compiles to a perfectly valid schema that + // quietly restricts nothing, since every regular block is the same node. + // Every other way a `children` config can be wrong is reported by + // TypeScript at compile time or by ProseMirror with a message of its own. it.each<[string, ContainerFixture["children"], RegExp]>([ - ["missing `allow`", {} as unknown as ChildrenConfig, /`allow` is required/], [ - "unknown `allow` form", - { allow: "everything" } as unknown as ChildrenConfig, - /`allow` must be/, - ], - ["unknown type in allow array", { allow: ["nope"] }, /nope/], - [ - "regular block type in allow array", - { allow: ["heading"] }, - /not yet supported/, - ], - ["allow that permits nothing", { allow: [] }, /permits nothing/], - [ - "containers wildcard with no other containers", - { allow: "containers" }, - /no other container block types/, - ], - ["negative minimum", { allow: "any", min: -1 }, /non-negative integer/], - [ - "maximum smaller than minimum", + "a maximum smaller than the minimum", { allow: "any", min: 3, max: 2 }, /greater than or equal/, ], [ - "unknown boundary value", - { allow: "any", boundary: "shut" } as unknown as ChildrenConfig, - /`boundary` must be "open", "isolated" or "sealed"/, + "a regular block type in the allow array", + { allow: ["heading"] }, + /not yet supported/, ], + // The wildcard is a group the container itself joins, so requiring a + // container child means requiring a copy of itself: the same stack + // overflow as a named cycle, just spelled without a second type. [ - "`default` violating the child count", - { allow: "any", min: 2, default: [{ type: "paragraph" }] }, - /fewer than the 2 required/, + "a container-only wildcard that requires children", + { allow: "containers", min: 1 }, + /nested inside itself/, ], ])("rejects %s", (_name, children, message) => { expect(validate({ box: { children } })).toThrow(message); }); - it("rejects `default` containing a block that isn't permitted", () => { - expect( - validate({ - grid: { - children: { - allow: ["gridCell"], - min: 2, - default: [{ type: "paragraph" }, { type: "paragraph" }], - }, - }, - gridCell: { - children: { allow: "any" }, - placement: "containerOnly", - }, - }), - ).toThrow(/not permitted/); - }); - - // The wildcards compile to the containers placeable anywhere, so a - // containerOnly block only fits where a parent names it explicitly. Every - // configuration that would leave one unreachable, or in an unsatisfiable - // `default`, is rejected up front. - it("rejects containerOnly blocks that nothing can hold", () => { - // In a wildcard `default`, which would build an unsatisfiable node: - expect( - validate({ - box: { children: { allow: "any", default: [{ type: "cell" }] } }, - cell: { children: { allow: "any" }, placement: "containerOnly" }, - }), - ).toThrow(/not permitted/); - // Unreachable, even though a wildcard container exists: - expect( - validate({ - box: { children: { allow: "any" } }, - cell: { children: { allow: "any" }, placement: "containerOnly" }, - }), - ).toThrow(/could never be inserted/); - // Unreachable, because no container's allow list names it: - expect( - validate({ - grid: { children: { allow: ["gridCell"], min: 2 } }, - gridCell: { - children: { allow: "blocks" }, - placement: "containerOnly", - }, - orphan: { - children: { allow: "blocks" }, - placement: "containerOnly", - }, - }), - ).toThrow(/could never be inserted/); - // A `containers` wildcard needs at least one placeable-anywhere one: - expect( - validate({ - box: { children: { allow: "containers" } }, - cell: { children: { allow: "any" }, placement: "containerOnly" }, - }), - ).toThrow(/placeable anywhere/); - }); - - it("rejects placement on a block that isn't a container", () => { - expect(() => - validateChildrenConfigs({ - paragraph: { - type: "paragraph", - content: "inline", - placement: "containerOnly", - }, - }), - ).toThrow(/only applies to container blocks/); - }); - - // A container block's body is its children; combining `children` with any - // content of the block's own is not supported. - it.each(["inline", "plain", "table"] as const)( - 'rejects `children` combined with `content: "%s"`', - (content) => { - expect(() => - validateChildrenConfigs({ - bad: { type: "bad", content, children: { allow: "any" } }, - }), - ).toThrow(/`children` can only be combined with `content: "none"`/); - }, - ); - // `fillBefore` recurses across node types, so a cycle blows the stack // rather than returning null. It has to be caught before the schema is // built. A mutual reference is fine as soon as one side can be filled with diff --git a/packages/core/src/schema/blocks/children.ts b/packages/core/src/schema/blocks/children.ts index 3f67820b15..cef124319c 100644 --- a/packages/core/src/schema/blocks/children.ts +++ b/packages/core/src/schema/blocks/children.ts @@ -1,11 +1,6 @@ -import type { Node, NodeType } from "prosemirror-model"; +import type { Node, NodeType, Schema } from "prosemirror-model"; -import type { - BlockConfig, - ChildrenAllow, - ChildrenConfig, - PartialBlockNoDefaults, -} from "./types.js"; +import type { ChildDefault, ChildrenAllow, ChildrenConfig } from "./types.js"; /** A {@link ChildrenConfig} with every default filled in. */ export type ResolvedChildren = { @@ -15,9 +10,9 @@ export type ResolvedChildren = { /** What `whenEmptied` compares against. */ min: number; max: number | undefined; - default: readonly PartialBlockNoDefaults[] | undefined; + default: readonly ChildDefault[] | undefined; whenEmptied: "refill" | "unwrap"; - boundary: "open" | "isolated" | "sealed"; + boundary: "open" | "sealed"; }; export const CHILD_CONTAINER_GROUP = "childContainer"; @@ -31,10 +26,83 @@ export const BLOCK_GROUP_CHILD_GROUP = "blockGroupChild"; export const ANY_CONTAINER_GROUP = "anyContainer"; // Whether `type` is a node that holds child blocks directly: a container -// block's own node. (`blockGroup` is in the group too but is regular-block -// nesting machinery, not a container.) +// block's own node. A container is a child-holding node that is itself a +// block; `blockGroup` also holds children but is not a block (it's regular +// blocks' nesting machinery), so the `bnBlock` check excludes it. export function isContainerNode(type: NodeType): boolean { - return type.isInGroup(CHILD_CONTAINER_GROUP) && type.name !== "blockGroup"; + return type.isInGroup(CHILD_CONTAINER_GROUP) && type.isInGroup("bnBlock"); +} + +/** + * The regions a block node resolves into, answered once for every shape so no + * other code asks "which shape am I": + * + * - a container block: its own node holds the children (`childrenHolder.node + * === outer`, offset 0), no content region; + * - a `blockContainer`: a content head at offset 1, and a `blockGroup` + * children holder only once it has children. + * + * `offset` measures from just before `outer` to just before the region's + * node, so with `beforePos` pointing at `outer`, a region's node starts at + * `beforePos + offset` and its inside begins at `beforePos + offset + 1` — + * uniformly across shapes. + */ +export type BlockRegions = { + outer: Node; + content?: { node: Node; offset: number }; + childrenHolder?: { node: Node; offset: number }; +}; + +export function getBlockRegions(node: Node): BlockRegions { + if (isContainerNode(node.type)) { + return { outer: node, childrenHolder: { node, offset: 0 } }; + } + + if (node.type.name === "blockContainer") { + const content = node.firstChild; + if (!content) { + throw new Error( + "blockContainer node has no content node. This is a bug in BlockNote.", + ); + } + const lastChild = node.lastChild; + const holder = + lastChild !== content && + lastChild && + lastChild.type.isInGroup(CHILD_CONTAINER_GROUP) + ? { node: lastChild, offset: 1 + content.nodeSize } + : undefined; + + return { + outer: node, + content: { node: content, offset: 1 }, + ...(holder ? { childrenHolder: holder } : {}), + }; + } + + throw new Error( + `Node "${node.type.name}" is not a block node (container or blockContainer).`, + ); +} + +// Builds the `blockGroup` node that holds a block's children when converting +// blocks to nodes. Transaction-level nesting (`sinkItem`, `findWrapping` in the +// keyboard shortcuts) wraps existing nodes in a `blockGroup` instead, and the +// document's root `blockGroup` is created by the parsers and `y`/`yjs` utils. +export function createBlockGroup( + schema: Schema, + children: readonly Node[], +): Node { + return schema.nodes["blockGroup"].createChecked({}, children as Node[]); +} + +// Whether a node of `type` can sit where regular blocks go: as a direct child +// of a `blockGroup` or of an `allow: "any"` container. `blockContainer` and +// every anywhere-placeable container qualify; `containerOnly` containers +// don't, and must be dissolved into their children before landing in such a +// slot (see `dissolveContainerOnlyBlocks` in `moveBlocks.ts`). +export function isBlockGroupInsertable(type: NodeType): boolean { + return type.isInGroup(BLOCK_GROUP_CHILD_GROUP); } // Below `blockContainer`'s priority (50) so PM's `fillBefore` picks @@ -59,24 +127,6 @@ export function containerNodePriority(priority: number | undefined): number { ); } -export function getChildrenConfig(config: { - children?: ChildrenConfig; -}): ChildrenConfig | undefined { - return config.children; -} - -export function isContainerType(config: { - children?: ChildrenConfig; -}): boolean { - return config.children !== undefined; -} - -export function isPlaceableAnywhere(config: { - placement?: BlockConfig["placement"]; -}): boolean { - return config.placement !== "containerOnly"; -} - const resolvedCache = new WeakMap(); export function resolveChildren(children: ChildrenConfig): ResolvedChildren { @@ -89,15 +139,25 @@ export function resolveChildren(children: ChildrenConfig): ResolvedChildren { ...resolveAllow(children.allow), min: children.min ?? 1, max: children.max, - default: children.default, + default: children.default && withoutIds(children.default), whenEmptied: children.whenEmptied ?? "refill", - boundary: children.boundary ?? "isolated", + boundary: children.boundary ?? "open", }; resolvedCache.set(children, resolved); return resolved; } +// Clears any id an untyped config wrote into a default, so each copy of the +// container gets a freshly generated one instead of a shared duplicate. +function withoutIds(blocks: readonly ChildDefault[]): ChildDefault[] { + return blocks.map((block) => ({ + ...block, + id: undefined, + ...(block.children ? { children: withoutIds(block.children) } : {}), + })); +} + function resolveAllow( allow: ChildrenAllow, ): Pick { @@ -119,7 +179,7 @@ function resolveAllow( * Reads the block config off the node's spec. */ export function isSealed(node: Node): boolean { - const children = getChildrenConfig(node.type.spec.blockConfig ?? {}); + const children = node.type.spec.blockConfig?.children; return ( children !== undefined && resolveChildren(children).boundary === "sealed" ); @@ -153,10 +213,8 @@ function allowTerm(resolved: ResolvedChildren): string { } if (terms.length === 0) { - // Validation rejects this first; this is a bug-guard, not a user-facing - // error path. throw new Error( - "Container `allow` permits nothing. This is a bug in BlockNote.", + "Container `allow` permits nothing. A container must accept at least one block or container type; drop `children` entirely for a block that holds none.", ); } diff --git a/packages/core/src/schema/blocks/createSpec.ts b/packages/core/src/schema/blocks/createSpec.ts index 5c22e7f0fe..bebdba6e2b 100644 --- a/packages/core/src/schema/blocks/createSpec.ts +++ b/packages/core/src/schema/blocks/createSpec.ts @@ -23,9 +23,6 @@ import { CHILD_CONTAINER_GROUP, childrenContentExpression, containerNodePriority, - getChildrenConfig, - isPlaceableAnywhere, - resolveChildren, } from "./children.js"; import { applyContainerAttributes } from "./containerAttributes.js"; import { @@ -270,10 +267,10 @@ function buildContainerNode( blockImplementation: BlockImplementation, priority?: number, ) { - const children = getChildrenConfig(blockConfig)!; + const children = blockConfig.children!; const groups = ["bnBlock", CHILD_CONTAINER_GROUP]; - if (isPlaceableAnywhere(blockConfig)) { + if (blockConfig.placement !== "containerOnly") { groups.push(BLOCK_GROUP_CHILD_GROUP, ANY_CONTAINER_GROUP); } @@ -285,9 +282,14 @@ function buildContainerNode( return suggestionMarks(this.editor); }, selectable: blockImplementation.meta?.selectable ?? true, - // Derived from `boundary`: an "open" container lets everything cross its - // edge; "isolated" and "sealed" both map to PM `isolating: true`. - isolating: resolveChildren(children).boundary !== "open", + // Deliberately not `isolating`, not even for a sealed container. PM only + // honours that flag while no selection spans the edge, and nothing stops + // one being made: given a spanning slice, `Fitter` refuses to open into + // the container and wraps the content in a spurious `blockGroup` instead, + // so a copy-paste across the edge corrupts the document. Seals bind + // editing gestures, and those are enforced by BlockNote's own `isSealed` + // guards (see `containerNav.ts` and `KeyboardShortcutsExtension.ts`), + // which need no help from the schema. defining: true, priority: containerNodePriority(priority), addAttributes() { @@ -549,7 +551,7 @@ export function addNodeAndExtensionsToSpec< ): LooseBlockSpec { // A `children` config combined with any `content` other than `"none"` is // rejected by `validateChildrenConfigs` when the schema is built. - const childrenConfig = getChildrenConfig(blockConfig); + const childrenConfig = blockConfig.children; const isContainer = childrenConfig !== undefined; @@ -760,7 +762,7 @@ export function createBlockSpec< : extensionsOrCreator : undefined; - const isContainer = getChildrenConfig(blockConfig) !== undefined; + const isContainer = blockConfig.children !== undefined; return { config: blockConfig, diff --git a/packages/core/src/schema/blocks/types.ts b/packages/core/src/schema/blocks/types.ts index 813ea343fd..4c010a57ef 100644 --- a/packages/core/src/schema/blocks/types.ts +++ b/packages/core/src/schema/blocks/types.ts @@ -106,6 +106,18 @@ export interface BlockConfigMeta< */ export type ChildrenAllow = "any" | "blocks" | "containers" | readonly string[]; +/** + * One entry of {@link ChildrenConfig.default}: a partial block with no `id`. + * + * A default describes every copy of the container, but an id names one block + * in one document. An id written here would be stamped onto every copy, so + * `id` is rejected at the type level and ignored at runtime. + */ +export type ChildDefault = Omit, "id"> & { + id?: never; + children?: ChildDefault[]; +}; + /** * Marks a block as a *container*: a block whose body is other blocks, exposed * as `block.children` at runtime. @@ -130,7 +142,7 @@ export type ChildrenConfig = { * Also the seed that `whenEmptied: "refill"` tops up from, when children * drop below `min`. */ - default?: readonly PartialBlockNoDefaults[]; + default?: readonly ChildDefault[]; /** * What happens as children are emptied out (Backspace merges the last child * away, `removeBlocks` deletes children, ...) and fewer than `min` non-empty @@ -152,24 +164,19 @@ export type ChildrenConfig = { */ whenEmptied?: "refill" | "unwrap"; /** - * What may cross the container's edge. + * Whether editing gestures cross the container's edge. * - * - `"open"`: the caret, editing gestures and text selections all cross - * the edge (ProseMirror `isolating: false`). Right for flow regions like - * column lists, where a selection may span columns. - * - `"isolated"` (the default): the caret and editing gestures cross - * exactly as with `"open"`; only a text selection cannot span the edge - * (`isolating: true`). - * - `"sealed"`: atomic to gestures, like a table cell. The caret doesn't - * enter via arrows/Backspace, and the block selects as a unit - * (`isolating: true`). Key-agnostic, so compartments need no hand-written - * keyboard handlers. + * - `"open"` (the default): they do. Right for a callout or a column list, + * where the container is a region of the same flow of text. + * - `"sealed"`: they don't, like a table cell. The caret doesn't enter via + * arrows/Backspace, and the block selects as a unit. Key-agnostic, so + * compartments need no hand-written keyboard handlers. * - * Seals bind editing gestures only: the block manipulation API - * (`insertBlocks` etc.) ignores them. - * @default "isolated" + * A seal binds gestures only. A text selection may span any edge, and the + * block manipulation API (`insertBlocks` etc.) ignores seals entirely. + * @default "open" */ - boundary?: "open" | "isolated" | "sealed"; + boundary?: "open" | "sealed"; }; // `ResolvedChildren`, the fully-defaulted, desugared shape a `ChildrenConfig` diff --git a/packages/core/src/schema/blocks/validateChildren.ts b/packages/core/src/schema/blocks/validateChildren.ts index 942f4eddfe..a1495f21ba 100644 --- a/packages/core/src/schema/blocks/validateChildren.ts +++ b/packages/core/src/schema/blocks/validateChildren.ts @@ -1,21 +1,21 @@ -import { - getChildrenConfig, - isContainerType, - isPlaceableAnywhere, - resolveChildren, -} from "./children.js"; -import type { ResolvedChildren } from "./children.js"; +import { resolveChildren } from "./children.js"; import type { BlockConfig, ChildrenConfig } from "./types.js"; type ValidatableConfig = Pick & { children?: ChildrenConfig; placement?: BlockConfig["placement"]; + /** From the block's implementation rather than its config. */ + runsBefore?: string[]; }; /** - * Validates the `children` config of every block in a schema, so that - * misconfigurations are reported as a clear error at schema-creation time - * instead of as an opaque ProseMirror one (or a stack overflow) much later. + * Validates the parts of a container block's declaration that nothing else + * catches. + * + * Deliberately narrow: TypeScript already rejects malformed configs at compile + * time, and ProseMirror already reports unknown types, unsatisfiable content + * expressions and unfillable containers with usable messages of its own. Only + * the cases below fail silently or catastrophically without help. * * @param blockConfigs The configs of every block in the schema, keyed by type. */ @@ -23,277 +23,68 @@ export function validateChildrenConfigs( blockConfigs: Record, ) { const isContainerBlockType = (blockType: string) => - !!blockConfigs[blockType] && isContainerType(blockConfigs[blockType]); - const acceptCtx = { - isContainerBlockType, - isPlaceableAnywhereType: (blockType: string) => - !!blockConfigs[blockType] && isPlaceableAnywhere(blockConfigs[blockType]), - }; + blockConfigs[blockType]?.children !== undefined; for (const [type, config] of Object.entries(blockConfigs)) { - const children = getChildrenConfig(config); - - if (!children) { - // `placement: "anywhere"` is the documented default for every block, so - // writing it on a regular block is a harmless restatement. Only - // `"containerOnly"` is meaningless without `children`. - if (config.placement === "containerOnly") { - fail( - type, - '`placement: "containerOnly"` only applies to container blocks, but this block does not declare `children`. Regular blocks can always be placed anywhere.', - ); - } + if (!config.children) { continue; } - validateOne(type, config, children, blockConfigs, acceptCtx); - } - - validateContainerOnlyIsReachable(blockConfigs); - validateNoCycles(blockConfigs, isContainerBlockType); -} - -function fail(type: string, message: string): never { - throw new Error( - `Invalid \`children\` config for block "${type}": ${message}`, - ); -} - -type AllowAcceptContext = { - isContainerBlockType: (blockType: string) => boolean; - isPlaceableAnywhereType: (blockType: string) => boolean; -}; - -function validateOne( - type: string, - config: ValidatableConfig, - children: ChildrenConfig, - blockConfigs: Record, - acceptCtx: AllowAcceptContext, -) { - // A container block's body is its children; it has no content of its own. - // Combining the two (a "content container") is not supported — only - // `content: "none"` may be combined with `children`. Blocks wanting an - // editable title alongside their children can use a string prop instead. - if (config.content !== "none") { - fail( - type, - `\`children\` can only be combined with \`content: "none"\`, but this block declares \`content: "${config.content}"\`. ` + - "A container block holds child blocks instead of its own content. " + - "For an editable title or caption, use a string prop rendered as an input.", - ); - } - - // Mirror the type-level contract for JS consumers: `allow` is required, and - // takes exactly the four forms. Widened to `unknown` because the type - // narrowing would otherwise leave `never` for the message. - const allow: unknown = children.allow; - if (allow === undefined) { - fail( - type, - '`allow` is required. Use `children: { allow: "any" }` for a container that accepts any block.', - ); - } - if ( - !Array.isArray(allow) && - allow !== "any" && - allow !== "blocks" && - allow !== "containers" - ) { - fail( - type, - `\`allow\` must be "any", "blocks", "containers" or an array of container block types, but is ${JSON.stringify(allow)}.`, - ); - } - - const boundary: string | undefined = children.boundary; - if ( - boundary !== undefined && - boundary !== "open" && - boundary !== "isolated" && - boundary !== "sealed" - ) { - fail( - type, - `\`boundary\` must be "open", "isolated" or "sealed", but is "${boundary}".`, - ); - } - - const resolved = resolveChildren(children); - - if (!Number.isInteger(resolved.min) || resolved.min < 0) { - fail( - type, - `minimum child count must be a non-negative integer, but is ${resolved.min}.`, - ); - } - if (resolved.max !== undefined) { - if (!Number.isInteger(resolved.max) || resolved.max < 1) { - fail( - type, - `maximum child count must be a positive integer, but is ${resolved.max}.`, - ); - } - if (resolved.max < resolved.min) { - fail( - type, - `maximum child count (${resolved.max}) must be greater than or equal to the minimum (${resolved.min}).`, - ); - } - } - - validateAllow(type, resolved, blockConfigs, acceptCtx); - validateDefault(type, resolved, blockConfigs, acceptCtx); -} - -function validateAllow( - type: string, - resolved: ResolvedChildren, - blockConfigs: Record, - { isContainerBlockType, isPlaceableAnywhereType }: AllowAcceptContext, -) { - if (resolved.containers !== true) { - for (const allowed of resolved.containers) { - if (!(allowed in blockConfigs)) { - fail( - type, - `\`allow\` contains "${allowed}", which is not a block type in this schema.`, - ); - } - // An `allow` array is exact by construction: each named type is its own - // ProseMirror node. Every *regular* block, by contrast, is the same node - // (`blockContainer`), so naming one here would promise a restriction the - // schema cannot keep. - if (!isContainerBlockType(allowed)) { - fail( - type, - `\`allow\` contains "${allowed}", which is a regular block, not a container block. ` + - "Restricting which regular block types a container accepts is not yet supported, as every regular block is the same ProseMirror node. " + - 'Use `allow: "blocks"` to accept all regular blocks, or name only container block types.', - ); - } - } - } - - if ( - !resolved.blocks && - resolved.containers !== true && - resolved.containers.length === 0 - ) { - fail( - type, - "`allow` permits nothing. A container must accept at least one block or container type; drop `children` entirely for a block that holds none.", - ); - } - - if (!resolved.blocks && resolved.containers === true) { - // The wildcard compiles to the containers placeable anywhere, so only - // those make the container fillable. `containerOnly` blocks are never - // included. - const hasContainer = Object.keys(blockConfigs).some( - (blockType) => - isContainerBlockType(blockType) && - blockType !== type && - isPlaceableAnywhereType(blockType), - ); - if (!hasContainer) { - fail( - type, - "`allow` permits only container blocks, but this schema has no other container block types placeable anywhere. " + - 'The `"containers"` wildcard never includes `placement: "containerOnly"` blocks. Name those explicitly in an `allow` array.', - ); - } - } -} - -function validateDefault( - type: string, - resolved: ResolvedChildren, - blockConfigs: Record, - acceptCtx: AllowAcceptContext, -) { - const { default: defaultChildren, min, max } = resolved; - if (!defaultChildren) { - return; - } - - if (defaultChildren.length < min) { - fail( - type, - `\`default\` has ${defaultChildren.length} block(s), fewer than the ${min} required.`, - ); - } - if (max !== undefined && defaultChildren.length > max) { - fail( - type, - `\`default\` has ${defaultChildren.length} block(s), more than the ${max} allowed.`, - ); - } + const { blocks, min, max, containers } = resolveChildren(config.children); - for (const child of defaultChildren) { - const childType = child.type ?? "paragraph"; - if (!(childType in blockConfigs)) { + // ProseMirror's content-expression parser never compares the two, so an + // inverted range is silently read as "exactly `min`". + if (max !== undefined && max < min) { fail( type, - `\`default\` contains a block of type "${childType}", which is not a block type in this schema.`, + `maximum child count (${max}) must be greater than or equal to the minimum (${min}).`, ); } - if (!allowAccepts(resolved, childType, acceptCtx)) { + // `allow: "containers"` compiles to a group that the container itself is + // in, so "must hold a container" includes "must hold a copy of itself". + // ProseMirror fills that by nesting the container inside itself until the + // stack overflows, and which type it picks depends on the order block + // types were registered in — so this is rejected rather than left to + // chance. The cycle check below covers the same shape for named types. + if (containers === true && !blocks && min >= 1) { fail( type, - `\`default\` contains a block of type "${childType}", which is not permitted.`, + 'a container that allows only containers (`allow: "containers"`) cannot require any, as it counts as a container itself and would be nested inside itself forever. ' + + 'Use `min: 0`, allow regular blocks as well (`allow: "any"`), or name the container types it accepts.', ); } - } -} -/** - * Whether a container's `allow` accepts a block type. Matches what the schema - * enforces: the only lever for regular blocks is whether `blockContainer` is - * in the content expression, and the container wildcards compile to the - * containers placeable anywhere, so a `placement: "containerOnly"` block is - * only accepted where it is named explicitly. - */ -function allowAccepts( - resolved: ResolvedChildren, - blockType: string, - ctx: AllowAcceptContext, -): boolean { - if (ctx.isContainerBlockType(blockType)) { - return resolved.containers === true - ? ctx.isPlaceableAnywhereType(blockType) - : resolved.containers.includes(blockType); - } - return resolved.blocks; -} - -/** - * Container nodes register in a priority band strictly below `blockContainer` - * (see `containerNodePriority`), which is below every regular block. So a - * container's `runsBefore` can only order it against other containers. Naming - * a regular block there promises an ordering the schema cannot produce. - * - * @param blockConfigs The configs of every block in the schema, keyed by type. - * @param runsBefore The `runsBefore` each block's implementation declares. - */ -export function validateContainerRunsBefore( - blockConfigs: Record, - runsBefore: Record, -) { - for (const [type, config] of Object.entries(blockConfigs)) { - if (!isContainerType(config)) { - continue; + // An `allow` array is exact by construction: each named type is its own + // ProseMirror node. Every *regular* block, by contrast, is the same node + // (`blockContainer`), so naming one here compiles to a valid schema that + // quietly fails to restrict anything. + if (containers !== true) { + for (const allowed of containers) { + if (allowed in blockConfigs && !isContainerBlockType(allowed)) { + fail( + type, + `\`allow\` contains "${allowed}", which is a regular block, not a container block. ` + + "Restricting which regular block types a container accepts is not yet supported, as every regular block is the same ProseMirror node. " + + 'Use `allow: "blocks"` to accept all regular blocks, or name only container block types.', + ); + } + } } - for (const other of runsBefore[type] ?? []) { - // "default" is `sortByDependencies`' reference point rather than a - // block type. A type that isn't in the schema is not this check's - // concern. + // ProseMirror never sees `runsBefore`, so an entry it cannot act on is a + // silent no-op rather than an error. Container nodes register in a + // priority band below every regular block (see `containerNodePriority`), + // so ordering a container ahead of one is not something the schema could + // ever produce. + for (const other of config.runsBefore ?? []) { + // "default" is `sortByDependencies`' reference point rather than a block + // type. A type that isn't in the schema is not this check's concern. if (other === "default" || !(other in blockConfigs)) { continue; } - if (!isContainerType(blockConfigs[other])) { + + if (!isContainerBlockType(other)) { throw new Error( `Invalid \`runsBefore\` for container block "${type}": it names "${other}", which is a regular block, not a container block. ` + "Container block nodes always register below regular ones, so a container can never be ordered before a regular block. " + @@ -302,44 +93,14 @@ export function validateContainerRunsBefore( } } } -} -/** - * A `placement: "containerOnly"` block that no container accepts could never - * be inserted anywhere, which is always a mistake rather than a choice. - * - * Only explicit `allow` arrays count: the container wildcards compile to the - * containers placeable anywhere, so they never accept a `containerOnly` - * block. Otherwise deliberately conservative. Proving that the block is - * reachable from a block placeable at the root is full graph reachability, - * and this check only exists to catch typos. - */ -function validateContainerOnlyIsReachable( - blockConfigs: Record, -) { - const accepted = new Set(); - for (const config of Object.values(blockConfigs)) { - const children = getChildrenConfig(config); - if (!children) { - continue; - } - const { containers } = resolveChildren(children); - if (containers === true) { - continue; - } - for (const allowed of containers) { - accepted.add(allowed); - } - } + validateNoCycles(blockConfigs, isContainerBlockType); +} - for (const [type, config] of Object.entries(blockConfigs)) { - if (!isPlaceableAnywhere(config) && !accepted.has(type)) { - fail( - type, - `it declares \`placement: "containerOnly"\`, but no container's \`children.allow\` array includes it, so it could never be inserted.`, - ); - } - } +function fail(type: string, message: string): never { + throw new Error( + `Invalid \`children\` config for block "${type}": ${message}`, + ); } /** @@ -355,7 +116,7 @@ function validateNoCycles( // A container that allows regular blocks can always be filled with a plain // paragraph, so it never forces recursion. Only container-only lists do. const requiredContainers = (type: string): string[] => { - const children = getChildrenConfig(blockConfigs[type]); + const children = blockConfigs[type].children; if (!children) { return []; } diff --git a/packages/core/src/schema/index.ts b/packages/core/src/schema/index.ts index 967a65bb5e..e64132cb28 100644 --- a/packages/core/src/schema/index.ts +++ b/packages/core/src/schema/index.ts @@ -4,7 +4,6 @@ // `@blocknote/core/internal` (see `src/internal.ts`). Only the question a // block author asks, "is this a container?", belongs here; the config types // come from `./blocks/types.js` below. -export { isContainerType } from "./blocks/children.js"; export * from "./blocks/createSpec.js"; export * from "./blocks/internal.js"; export * from "./blocks/types.js"; diff --git a/packages/core/src/schema/schema.ts b/packages/core/src/schema/schema.ts index b69ba53fbf..94e9d9fed6 100644 --- a/packages/core/src/schema/schema.ts +++ b/packages/core/src/schema/schema.ts @@ -16,10 +16,7 @@ import { getInlineContentSchemaFromSpecs, getStyleSchemaFromSpecs, } from "./index.js"; -import { - validateChildrenConfigs, - validateContainerRunsBefore, -} from "./blocks/validateChildren.js"; +import { validateChildrenConfigs } from "./blocks/validateChildren.js"; function removeUndefined | undefined>(obj: T): T { if (!obj) { @@ -95,22 +92,16 @@ export class CustomBlockNoteSchema< })), ); - // Validation runs before the nodes are built, so misconfigurations - // surface as clear errors rather than as opaque ProseMirror ones. - const blockConfigs = Object.fromEntries( - Object.entries(this.opts.blockSpecs).map(([key, blockSpec]) => [ - key, - blockSpec.config, - ]), - ); - - validateChildrenConfigs(blockConfigs); - validateContainerRunsBefore( - blockConfigs, + // Validation runs before the nodes are built, so the misconfigurations + // ProseMirror cannot report on its own surface as clear errors. + validateChildrenConfigs( Object.fromEntries( Object.entries(this.opts.blockSpecs).map(([key, blockSpec]) => [ key, - blockSpec.implementation?.runsBefore, + { + ...blockSpec.config, + runsBefore: blockSpec.implementation?.runsBefore, + }, ]), ), ); diff --git a/packages/core/src/yjs/extensions/FixUpSchema.ts b/packages/core/src/yjs/extensions/FixUpSchema.ts index 7dc3f4253d..42234c9b45 100644 --- a/packages/core/src/yjs/extensions/FixUpSchema.ts +++ b/packages/core/src/yjs/extensions/FixUpSchema.ts @@ -25,15 +25,13 @@ export const FixUpSchemaExtension = createExtension(({ editor }) => { // create a copy that we can mutate (otherwise, assigning attrs is not safe and corrupts the pm state) const jsonNode = JSON.parse(JSON.stringify(ret.toJSON())); - // The first fill of the doc's blockGroup is guaranteed to be a - // `blockContainer` (container block nodes register at lower priority - // precisely so auto-fill picks `blockContainer` first), but guard on - // the node actually carrying an id attr in case a custom schema - // changes that. - const firstBlock = jsonNode.content?.[0]?.content?.[0]; - if (firstBlock?.attrs && "id" in firstBlock.attrs) { - firstBlock.attrs.id = "initialBlockId"; - } + // The first fill of the doc's blockGroup is always a `blockContainer`: + // container block nodes are clamped below its priority + // (`containerNodePriority`) precisely so auto-fill picks it first. If + // that ever stops holding, throwing here is better than silently + // leaving the id unset, which would let every peer generate its own + // initial block id. + jsonNode.content[0].content[0].attrs.id = "initialBlockId"; cache = Node.fromJSON(schema, jsonNode); return cache; diff --git a/packages/react/src/schema/ReactBlockSpec.tsx b/packages/react/src/schema/ReactBlockSpec.tsx index 0d49889901..9c72da3e41 100644 --- a/packages/react/src/schema/ReactBlockSpec.tsx +++ b/packages/react/src/schema/ReactBlockSpec.tsx @@ -12,7 +12,6 @@ import { Extension, ExtensionFactoryInstance, ExtractBlockConfigFromConfigOrCreator, - isContainerType, mergeCSSClasses, nodeToBlock, Props, @@ -244,52 +243,73 @@ export function createReactBlockSpec< : extensionsOrCreator : undefined; + // Container-ness is fixed per spec, so every render path can decide once. + const isContainer = blockConfig.children !== undefined; + + // Shared by the two paths that render to plain DOM (`toExternalHTML` and + // `render` outside a node view). A container block's output is the + // block's root element, with no wrapper: the attributes core stamps + // afterwards then land on the author's own element, the same element they + // land on in the live editor. + function renderStatic(args: { + BlockContent: FC; + block: any; + editor: any; + domAttributes?: Record; + isFileBlock?: boolean; + context?: any; + }) { + const { BlockContent, block, editor } = args; + + return renderToDOMSpec((refCB) => { + const content = ( + { + refCB(element); + if (element && !isContainer) { + element.className = mergeCSSClasses( + "bn-inline-content", + element.className, + ); + } + }} + context={args.context} + /> + ); + + return isContainer ? ( + content + ) : ( + + {content} + + ); + }, editor); + } + return { config: blockConfig, implementation: { ...blockImplementation, toExternalHTML(block, editor, context) { - const isContainer = isContainerType(blockConfig); - const BlockContent = (blockImplementation.toExternalHTML || - blockImplementation.render) as FC; - const output = renderToDOMSpec((refCB) => { - const content = ( - { - refCB(element); - if (element && !isContainer) { - element.className = mergeCSSClasses( - "bn-inline-content", - element.className, - ); - } - }} - context={context} - /> - ); - // A container block's render output is the block's root element, - // with no wrapper. The attributes core stamps afterwards then - // land on the author's own element, the same element they land - // on in the live editor. - return isContainer ? ( - content - ) : ( - - {content} - - ); - }, editor); - return output; + return renderStatic({ + BlockContent: (blockImplementation.toExternalHTML || + blockImplementation.render) as FC, + block, + editor, + domAttributes: this.blockContentDOMAttributes, + isFileBlock: + blockImplementation.meta?.fileBlockAccept !== undefined, + context, + }); }, render(block, editor) { if (this.renderType === "nodeView") { @@ -297,10 +317,8 @@ export function createReactBlockSpec< // constructed (itself guarded, via `getBlockFromNodeView`). Seeds // the fallback below so there is always something to render. const initialBlock = block; - // Container-ness is fixed per spec, so the node-view component - // can be chosen once. Each variant uses only the hooks and - // wrappers it needs. - const isContainer = isContainerType(blockConfig); + // Each node-view variant uses only the hooks and wrappers it + // needs, so the component is chosen once from `isContainer`. const BlockContent = blockImplementation.render as FC; const blockContentDOMAttributes = this.blockContentDOMAttributes; @@ -333,12 +351,12 @@ export function createReactBlockSpec< `Container block "${blockConfig.type}" is missing an id attribute.`, ); } - // The id lookup misses when the node was just removed from the - // document (e.g. a suggestion-mode deletion still rendering); - // fall back to converting the node the view was handed. - const block = - editor.getBlock(id) ?? - nodeToBlock(props.node, props.view.state.doc); + // Converted from the node the view was handed rather than + // looked up by id: the conversion is cached per node, while a + // lookup would scan the whole document on every render, and it + // also covers a node that was just removed from the document + // (e.g. a suggestion-mode deletion still rendering). + const block = nodeToBlock(props.node, props.view.state.doc); const ref = useReactNodeView().nodeViewContentRef; if (!ref) { @@ -349,8 +367,8 @@ export function createReactBlockSpec< // Stamped imperatively rather than spread as JSX props: the root // element belongs to the block's author, so there is nothing to - // spread onto. Runs after every render, since both the block's - // props and the author's root element can change. + // spread onto. `block` is derived from the node, so a new one + // also means the author's root may have been swapped. useLayoutEffect(() => { const root = authorRootDOM(); if (!root) { @@ -375,7 +393,7 @@ export function createReactBlockSpec< } else { root.removeAttribute("data-selected"); } - }); + }, [block, selected]); return ( @@ -464,40 +482,12 @@ export function createReactBlockSpec< return nodeView; } else { - const isContainer = isContainerType(blockConfig); - const BlockContent = blockImplementation.render as FC; - const output = renderToDOMSpec((refCB) => { - const content = ( - { - refCB(element); - if (element && !isContainer) { - element.className = mergeCSSClasses( - "bn-inline-content", - element.className, - ); - } - }} - /> - ); - // See `toExternalHTML` above: a container block owns its outer - // DOM, so its render output is the block's root element. - return isContainer ? ( - content - ) : ( - - {content} - - ); - }, editor); - return output; + return renderStatic({ + BlockContent: blockImplementation.render as FC, + block, + editor, + domAttributes: this.blockContentDOMAttributes, + }); } }, }, diff --git a/packages/xl-ai/src/api/formats/html-blocks/collabUpdate.test.ts b/packages/xl-ai/src/api/formats/html-blocks/collabUpdate.test.ts index 34d60aa6bf..b8a4405285 100644 --- a/packages/xl-ai/src/api/formats/html-blocks/collabUpdate.test.ts +++ b/packages/xl-ai/src/api/formats/html-blocks/collabUpdate.test.ts @@ -10,7 +10,7 @@ import { BlockNoteEditor, expandPMRangeToWords, - getBlockInfo, + getBlockInfoFromNode, getNodeById, } from "@blocknote/core"; import type { ForkYDocExtension } from "@blocknote/core/yjs"; @@ -79,12 +79,13 @@ function createCollabEditor(text: string) { */ function selectWholeFirstBlock(editor: BlockNoteEditor) { const id = editor.document[0].id; - const info = getBlockInfo(getNodeById(id, editor.prosemirrorState.doc)!); - if (!info.isWrappedBlock) { + const posInfo = getNodeById(id, editor.prosemirrorState.doc)!; + const info = getBlockInfoFromNode(posInfo.node, posInfo.posBeforeNode); + if (!info.hasContent) { throw new Error("not a block container"); } - const from = info.blockContent.beforePos + 1; - const to = info.blockContent.afterPos - 1; + const from = info.content.beforePos + 1; + const to = info.content.afterPos - 1; editor.transact((tr) => { tr.setSelection(TextSelection.create(tr.doc, from, to)); diff --git a/packages/xl-ai/src/prosemirror/agent.test.ts b/packages/xl-ai/src/prosemirror/agent.test.ts index d2a7d9178b..bc3392941c 100644 --- a/packages/xl-ai/src/prosemirror/agent.test.ts +++ b/packages/xl-ai/src/prosemirror/agent.test.ts @@ -1,7 +1,7 @@ import { BlockNoteEditor, expandPMRangeToWords, - getBlockInfo, + getBlockInfoFromNode, getNodeById, } from "@blocknote/core"; import { Fragment, Slice } from "prosemirror-model"; @@ -38,12 +38,12 @@ describe.skip("getStepsAsAgent", () => { const doc = editor.prosemirrorState.doc; // Get the position of the content in the paragraph const blockPos = getNodeById("1", doc)!; - const block = getBlockInfo(blockPos); - if (!block.isWrappedBlock) { + const block = getBlockInfoFromNode(blockPos.node, blockPos.posBeforeNode); + if (!block.hasContent) { throw new Error("Block is not a container"); } - const contentStart = block.blockContent.beforePos; + const contentStart = block.content.beforePos; // Create a ReplaceStep that replaces "Hello" with "Hi" const from = contentStart + 1; // +1 to skip the initial position @@ -71,13 +71,13 @@ describe.skip("getStepsAsAgent", () => { const doc = editor.prosemirrorState.doc; // Get the position of the content in the paragraph const blockPos = getNodeById("1", doc)!; - const block = getBlockInfo(blockPos); - if (!block.isWrappedBlock) { + const block = getBlockInfoFromNode(blockPos.node, blockPos.posBeforeNode); + if (!block.hasContent) { throw new Error("Block is not a container"); } const tr = editor.prosemirrorState.tr.setNodeMarkup( - block.blockContent.beforePos, + block.content.beforePos, editor.pmSchema.nodes.heading, ); @@ -97,13 +97,13 @@ describe.skip("getStepsAsAgent", () => { const doc = editor.prosemirrorState.doc; // Get the position of the content in the paragraph const blockPos = getNodeById("1", doc)!; - const block = getBlockInfo(blockPos); - if (!block.isWrappedBlock) { + const block = getBlockInfoFromNode(blockPos.node, blockPos.posBeforeNode); + if (!block.hasContent) { throw new Error("Block is not a container"); } const tr = editor.prosemirrorState.tr.setNodeMarkup( - block.blockContent.beforePos, + block.content.beforePos, undefined, { textAlignment: "right", @@ -127,17 +127,17 @@ describe.skip("getStepsAsAgent", () => { const doc = editor.prosemirrorState.doc; // Get the position of the content in the paragraph const blockPos = getNodeById("1", doc)!; - const block = getBlockInfo(blockPos); - if (!block.isWrappedBlock) { + const block = getBlockInfoFromNode(blockPos.node, blockPos.posBeforeNode); + if (!block.hasContent) { throw new Error("Block is not a container"); } const step = new ReplaceStep( - block.blockContent.beforePos, - block.blockContent.beforePos + 3, + block.content.beforePos, + block.content.beforePos + 3, // for simplicity, we're not actually changing the node type and content, but we just use the existing document // as replacement content - doc.slice(block.blockContent.beforePos, block.blockContent.beforePos + 3), + doc.slice(block.content.beforePos, block.content.beforePos + 3), ); const tr = new Transform(doc); @@ -156,12 +156,12 @@ describe.skip("getStepsAsAgent", () => { // Get the position of the content in the paragraph const blockPos = getNodeById("1", doc)!; - const block = getBlockInfo(blockPos); - if (!block.isWrappedBlock) { + const block = getBlockInfoFromNode(blockPos.node, blockPos.posBeforeNode); + if (!block.hasContent) { throw new Error("Block is not a container"); } - const contentStart = block.blockContent.beforePos; + const contentStart = block.content.beforePos; // Create two ReplaceSteps // 1. Replace "Hello" with "Hi" diff --git a/packages/xl-ai/src/prosemirror/rebaseTool.test.ts b/packages/xl-ai/src/prosemirror/rebaseTool.test.ts index edd8a3b1bb..24b0e712ee 100644 --- a/packages/xl-ai/src/prosemirror/rebaseTool.test.ts +++ b/packages/xl-ai/src/prosemirror/rebaseTool.test.ts @@ -1,4 +1,8 @@ -import { BlockNoteEditor, getBlockInfo, getNodeById } from "@blocknote/core"; +import { + BlockNoteEditor, + getBlockInfoFromNode, + getNodeById, +} from "@blocknote/core"; import { expect, it } from "vite-plus/test"; import { AttributionMarksExtension } from "./AttributionMarks.js"; import { getApplySuggestionsTr, rebaseTool } from "./rebaseTool.js"; @@ -20,21 +24,21 @@ function getExampleEditorWithSuggestions() { const blockPos = getNodeById("1", editor.prosemirrorState.doc)!; - const block = getBlockInfo(blockPos); - if (!block.isWrappedBlock) { + const block = getBlockInfoFromNode(blockPos.node, blockPos.posBeforeNode); + if (!block.hasContent) { throw new Error("Block is not a container"); } editor.transact((tr) => { tr.addMark( - block.blockContent.beforePos + 1, - block.blockContent.beforePos + 6, + block.content.beforePos + 1, + block.content.beforePos + 6, editor.pmSchema.mark("deletion", { id: 1 }), ); tr.addMark( - block.blockContent.beforePos + 6, - block.blockContent.beforePos + 8, + block.content.beforePos + 6, + block.content.beforePos + 8, editor.pmSchema.mark("insertion", { id: 2 }), ); }); @@ -54,13 +58,13 @@ it("should be able to apply changes to a clean doc (use invertMap)", async () => const blockPos = getNodeById("1", cleaned.doc)!; - const block = getBlockInfo(blockPos); + const block = getBlockInfoFromNode(blockPos.node, blockPos.posBeforeNode); - if (!block.isWrappedBlock) { + if (!block.hasContent) { throw new Error("Block is not a container"); } - const start = block.blockContent.beforePos + 1; + const start = block.content.beforePos + 1; const end = start + 2; expect(cleaned.doc.textBetween(start, end)).toBe("Hi"); @@ -83,13 +87,13 @@ it("should be able to apply changes to a clean doc (use rebaseTr)", async () => const blockPos = getNodeById("1", cleaned.doc)!; - const block = getBlockInfo(blockPos); + const block = getBlockInfoFromNode(blockPos.node, blockPos.posBeforeNode); - if (!block.isWrappedBlock) { + if (!block.hasContent) { throw new Error("Block is not a container"); } - const start = block.blockContent.beforePos + 1; + const start = block.content.beforePos + 1; const end = start + 2; expect(cleaned.doc.textBetween(start, end)).toBe("Hi"); diff --git a/packages/xl-ai/src/testUtil/cases/combinedOperationsTestCases.ts b/packages/xl-ai/src/testUtil/cases/combinedOperationsTestCases.ts index 8bbcb29315..8b004c9130 100644 --- a/packages/xl-ai/src/testUtil/cases/combinedOperationsTestCases.ts +++ b/packages/xl-ai/src/testUtil/cases/combinedOperationsTestCases.ts @@ -1,5 +1,5 @@ -// import { BlockNoteEditor, getBlockInfo, getNodeById } from "@blocknote/core"; -import { getBlockInfo, getNodeById } from "@blocknote/core"; +// import { BlockNoteEditor, getBlockInfoFromNode, getNodeById } from "@blocknote/core"; +import { getBlockInfoFromNode, getNodeById } from "@blocknote/core"; import { getEditorWithFormattingAndMentions } from "./editors/formattingAndMentions.js"; import { DocumentOperationTestCase } from "./index.js"; @@ -46,13 +46,13 @@ export const combinedOperationsTestCases: DocumentOperationTestCase[] = [ getTestSelection: (editor) => { const posInfo = getNodeById("ref2", editor.prosemirrorState.doc)!; - const block = getBlockInfo(posInfo); - if (!block.isWrappedBlock) { + const block = getBlockInfoFromNode(posInfo.node, posInfo.posBeforeNode); + if (!block.hasContent) { throw new Error("Block is not a block container"); } return { - from: block.blockContent.beforePos + 1, - to: block.blockContent.beforePos + 1 + "Hello".length, + from: block.content.beforePos + 1, + to: block.content.beforePos + 1 + "Hello".length, }; }, userPrompt: diff --git a/packages/xl-ai/src/testUtil/cases/updateOperationTestCases.ts b/packages/xl-ai/src/testUtil/cases/updateOperationTestCases.ts index 3d4d25f152..d483314f06 100644 --- a/packages/xl-ai/src/testUtil/cases/updateOperationTestCases.ts +++ b/packages/xl-ai/src/testUtil/cases/updateOperationTestCases.ts @@ -1,4 +1,8 @@ -import { BlockNoteEditor, getBlockInfo, getNodeById } from "@blocknote/core"; +import { + BlockNoteEditor, + getBlockInfoFromNode, + getNodeById, +} from "@blocknote/core"; import { AIExtension } from "../../AIExtension.js"; import { getEditorWithBlockFormatting } from "./editors/blockFormatting.js"; import { getEditorWithFormattingAndMentions } from "./editors/formattingAndMentions.js"; @@ -40,13 +44,13 @@ export const updateOperationTestCases: DocumentOperationTestCase[] = [ ], getTestSelection: (editor: BlockNoteEditor) => { const posInfo = getNodeById("ref2", editor.prosemirrorState.doc)!; - const block = getBlockInfo(posInfo); - if (!block.isWrappedBlock) { + const block = getBlockInfoFromNode(posInfo.node, posInfo.posBeforeNode); + if (!block.hasContent) { throw new Error("Block is not a block container"); } return { - from: block.blockContent.beforePos + 1, - to: block.blockContent.beforePos + 1 + "Hello".length, + from: block.content.beforePos + 1, + to: block.content.beforePos + 1 + "Hello".length, }; }, userPrompt: "translate to German", @@ -67,14 +71,14 @@ export const updateOperationTestCases: DocumentOperationTestCase[] = [ ], getTestSelection: (editor: BlockNoteEditor) => { const posInfo = getNodeById("ref1", editor.prosemirrorState.doc)!; - const block = getBlockInfo(posInfo); - if (!block.isWrappedBlock) { + const block = getBlockInfoFromNode(posInfo.node, posInfo.posBeforeNode); + if (!block.hasContent) { throw new Error("Block is not a block container"); } // 'ello, world! Dow are yo' return { - from: block.blockContent.beforePos + 2, - to: block.blockContent.afterPos - 3, + from: block.content.beforePos + 2, + to: block.content.afterPos - 3, }; }, userPrompt: "fix spelling", @@ -736,12 +740,12 @@ export const updateOperationTestCases: DocumentOperationTestCase[] = [ userPrompt: "turn into list (update existing blocks)", getTestSelection(editor) { const posInfo = getNodeById("ref2", editor.prosemirrorState.doc)!; - const block = getBlockInfo(posInfo); - if (!block.isWrappedBlock) { + const block = getBlockInfoFromNode(posInfo.node, posInfo.posBeforeNode); + if (!block.hasContent) { throw new Error("Block is not a block container"); } return { - from: block.blockContent.beforePos + 1, + from: block.content.beforePos + 1, to: editor.prosemirrorState.doc.content.size, }; }, diff --git a/packages/xl-docx-exporter/src/docx/docxExporter.ts b/packages/xl-docx-exporter/src/docx/docxExporter.ts index 98ea7ac8cb..85fe106112 100644 --- a/packages/xl-docx-exporter/src/docx/docxExporter.ts +++ b/packages/xl-docx-exporter/src/docx/docxExporter.ts @@ -183,12 +183,11 @@ export class DOCXExporter< numberingInstance, children, ); // TODO: any - if (this.isContainerBlock(b.type)) { - ret.push(self as Table); - } else if (Array.isArray(self)) { - ret.push(...self, ...children); - } else { - ret.push(self, ...children); + ret.push(...(Array.isArray(self) ? self : [self])); + // A container's mapping is handed its children and places them itself, + // so they must not be appended after it as well. + if (!this.isContainerBlock(b.type)) { + ret.push(...children); } } return ret; diff --git a/packages/xl-multi-column/src/extensions/DropCursor/multiColumnHandleDropPlugin.ts b/packages/xl-multi-column/src/extensions/DropCursor/multiColumnHandleDropPlugin.ts index 6f26bce4e8..f12865903e 100644 --- a/packages/xl-multi-column/src/extensions/DropCursor/multiColumnHandleDropPlugin.ts +++ b/packages/xl-multi-column/src/extensions/DropCursor/multiColumnHandleDropPlugin.ts @@ -3,7 +3,7 @@ import { UniqueID, createExtension, fragmentToBlocks, - getBlockInfoWithManualOffset, + getBlockInfoFromNode, isContainerNode, nodeToBlock, } from "@blocknote/core"; @@ -27,7 +27,7 @@ export function createMultiColumnHandleDropPlugin( return false; // Let ProseMirror handle the drop (e.g. outside editor bounds) } - const blockInfo = getBlockInfoWithManualOffset( + const blockInfo = getBlockInfoFromNode( edgePos.node, edgePos.posBeforeNode, ); @@ -49,7 +49,7 @@ export function createMultiColumnHandleDropPlugin( // Whether the edge target is a `columnList` (after `detectEdgePosition` // hoisted blocks inside a column to the column itself, the target's // parent is the columnList). - const $target = view.state.doc.resolve(blockInfo.bnBlock.beforePos); + const $target = view.state.doc.resolve(blockInfo.block.beforePos); const targetInHorizontalContainer = $target.node().type.name === "columnList"; @@ -62,7 +62,7 @@ export function createMultiColumnHandleDropPlugin( // A column is a pure container: its `children` node is the column // node itself. const columnChildren = - blockInfo.childContainer?.node ?? blockInfo.bnBlock.node; + blockInfo.children?.node ?? blockInfo.block.node; columnChildren.forEach((child) => { if (!draggedBlockIds.has(child.attrs.id)) { allTargetChildrenDragged = false; @@ -85,7 +85,7 @@ export function createMultiColumnHandleDropPlugin( // containers (like `column`) that wrap the actual blocks, or plain // blocks spliced in directly. const targetIsChildContainer = isContainerNode( - blockInfo.bnBlock.node.type, + blockInfo.block.node.type, ); // Normalize column widths to average of 1 @@ -122,7 +122,7 @@ export function createMultiColumnHandleDropPlugin( } } - const targetColumnId = blockInfo.bnBlock.node.attrs.id; + const targetColumnId = blockInfo.block.node.attrs.id; // The target itself is one of the dragged blocks (only possible // when the container holds plain blocks directly) - the dragged @@ -221,7 +221,7 @@ export function createMultiColumnHandleDropPlugin( }); } else { // Create new columnList with blocks as columns - const block = nodeToBlock(blockInfo.bnBlock.node, view.state.doc); + const block = nodeToBlock(blockInfo.block.node, view.state.doc); // The user is dropping next to one of the blocks being dragged - do // nothing. diff --git a/tests/src/unit/core/clipboard/copy/__snapshots__/text/html/containerChildToSiblingAfter.html b/tests/src/unit/core/clipboard/copy/__snapshots__/text/html/containerChildToSiblingAfter.html new file mode 100644 index 0000000000..b23de063c2 --- /dev/null +++ b/tests/src/unit/core/clipboard/copy/__snapshots__/text/html/containerChildToSiblingAfter.html @@ -0,0 +1,6 @@ +
+
+

Callout child 2

+
+
+

After callout

\ No newline at end of file diff --git a/tests/src/unit/core/clipboard/copy/__snapshots__/text/html/containerChildren.html b/tests/src/unit/core/clipboard/copy/__snapshots__/text/html/containerChildren.html new file mode 100644 index 0000000000..b608fb6e83 --- /dev/null +++ b/tests/src/unit/core/clipboard/copy/__snapshots__/text/html/containerChildren.html @@ -0,0 +1,6 @@ +
+
+

Callout child 1

+

Callout child 2

+
+
\ No newline at end of file diff --git a/tests/src/unit/core/clipboard/copy/__snapshots__/text/html/containerNestedChild.html b/tests/src/unit/core/clipboard/copy/__snapshots__/text/html/containerNestedChild.html new file mode 100644 index 0000000000..65339aa967 --- /dev/null +++ b/tests/src/unit/core/clipboard/copy/__snapshots__/text/html/containerNestedChild.html @@ -0,0 +1 @@ +Inner child \ No newline at end of file diff --git a/tests/src/unit/core/clipboard/copy/__snapshots__/text/plain/containerChildToSiblingAfter.md b/tests/src/unit/core/clipboard/copy/__snapshots__/text/plain/containerChildToSiblingAfter.md new file mode 100644 index 0000000000..f313aa5cda --- /dev/null +++ b/tests/src/unit/core/clipboard/copy/__snapshots__/text/plain/containerChildToSiblingAfter.md @@ -0,0 +1,3 @@ +Callout child 2 + +After callout diff --git a/tests/src/unit/core/clipboard/copy/__snapshots__/text/plain/containerChildren.md b/tests/src/unit/core/clipboard/copy/__snapshots__/text/plain/containerChildren.md new file mode 100644 index 0000000000..054a153e1c --- /dev/null +++ b/tests/src/unit/core/clipboard/copy/__snapshots__/text/plain/containerChildren.md @@ -0,0 +1,3 @@ +Callout child 1 + +Callout child 2 diff --git a/tests/src/unit/core/clipboard/copy/__snapshots__/text/plain/containerNestedChild.md b/tests/src/unit/core/clipboard/copy/__snapshots__/text/plain/containerNestedChild.md new file mode 100644 index 0000000000..72d309cf51 --- /dev/null +++ b/tests/src/unit/core/clipboard/copy/__snapshots__/text/plain/containerNestedChild.md @@ -0,0 +1 @@ +Inner child diff --git a/tests/src/unit/core/clipboard/copy/copyTestInstances.ts b/tests/src/unit/core/clipboard/copy/copyTestInstances.ts index 4bd34489c0..996d90b2c4 100644 --- a/tests/src/unit/core/clipboard/copy/copyTestInstances.ts +++ b/tests/src/unit/core/clipboard/copy/copyTestInstances.ts @@ -722,6 +722,80 @@ export const copyTestInstancesHTML: TestInstance< }, executeTest: testCopyHTML, }, + { + // The whole of a container's children, selected from inside it. + testCase: { + name: "containerChildren", + document: [ + { + type: "callout", + children: [ + { type: "paragraph", content: "Callout child 1" }, + { type: "paragraph", content: "Callout child 2" }, + ], + }, + ], + getCopySelection: (doc) => { + const startPos = getPosOfTextNode(doc, "Callout child 1"); + const endPos = getPosOfTextNode(doc, "Callout child 2", true); + + return TextSelection.create(doc, startPos, endPos); + }, + }, + executeTest: testCopyHTML, + }, + { + // A selection that leaves the container partway through, so the copied + // fragment is cut open on one side. + testCase: { + name: "containerChildToSiblingAfter", + document: [ + { + type: "callout", + children: [ + { type: "paragraph", content: "Callout child 1" }, + { type: "paragraph", content: "Callout child 2" }, + ], + }, + { type: "paragraph", content: "After callout" }, + ], + getCopySelection: (doc) => { + const startPos = getPosOfTextNode(doc, "Callout child 2"); + const endPos = getPosOfTextNode(doc, "After callout", true); + + return TextSelection.create(doc, startPos, endPos); + }, + }, + executeTest: testCopyHTML, + }, + { + // A single block two containers deep, so the fragment is cut open on both + // sides at two different levels. + testCase: { + name: "containerNestedChild", + document: [ + { + type: "callout", + props: { flavor: "warning" }, + children: [ + { type: "paragraph", content: "Outer child" }, + { + type: "callout", + props: { flavor: "info" }, + children: [{ type: "paragraph", content: "Inner child" }], + }, + ], + }, + ], + getCopySelection: (doc) => { + const startPos = getPosOfTextNode(doc, "Inner child"); + const endPos = getPosOfTextNode(doc, "Inner child", true); + + return TextSelection.create(doc, startPos, endPos); + }, + }, + executeTest: testCopyHTML, + }, ]; // text/plain payloads — exercises the same selections as above but snapshots diff --git a/tests/src/unit/core/formatConversion/export/__snapshots__/blocknoteHTML/container/basic.html b/tests/src/unit/core/formatConversion/export/__snapshots__/blocknoteHTML/container/basic.html new file mode 100644 index 0000000000..039f688a1b --- /dev/null +++ b/tests/src/unit/core/formatConversion/export/__snapshots__/blocknoteHTML/container/basic.html @@ -0,0 +1,13 @@ +
+
+
+
+
+
+

Callout child

+
+
+
+
+
+
\ No newline at end of file diff --git a/tests/src/unit/core/formatConversion/export/__snapshots__/blocknoteHTML/container/emptyChildren.html b/tests/src/unit/core/formatConversion/export/__snapshots__/blocknoteHTML/container/emptyChildren.html new file mode 100644 index 0000000000..68454f648d --- /dev/null +++ b/tests/src/unit/core/formatConversion/export/__snapshots__/blocknoteHTML/container/emptyChildren.html @@ -0,0 +1,5 @@ +
+
+
+
+
\ No newline at end of file diff --git a/tests/src/unit/core/formatConversion/export/__snapshots__/blocknoteHTML/container/nested.html b/tests/src/unit/core/formatConversion/export/__snapshots__/blocknoteHTML/container/nested.html new file mode 100644 index 0000000000..79dd38484a --- /dev/null +++ b/tests/src/unit/core/formatConversion/export/__snapshots__/blocknoteHTML/container/nested.html @@ -0,0 +1,24 @@ +
+
+
+
+
+
+

Nested heading

+
+
+
+
+
+
+
+
+

Inner callout child

+
+
+
+
+
+
+
+
\ No newline at end of file diff --git a/tests/src/unit/core/formatConversion/export/__snapshots__/html/container/basic.html b/tests/src/unit/core/formatConversion/export/__snapshots__/html/container/basic.html new file mode 100644 index 0000000000..4d56bc302f --- /dev/null +++ b/tests/src/unit/core/formatConversion/export/__snapshots__/html/container/basic.html @@ -0,0 +1,5 @@ +
+
+

Callout child

+
+
\ No newline at end of file diff --git a/tests/src/unit/core/formatConversion/export/__snapshots__/html/container/emptyChildren.html b/tests/src/unit/core/formatConversion/export/__snapshots__/html/container/emptyChildren.html new file mode 100644 index 0000000000..818e4ec746 --- /dev/null +++ b/tests/src/unit/core/formatConversion/export/__snapshots__/html/container/emptyChildren.html @@ -0,0 +1,3 @@ +
+
+
\ No newline at end of file diff --git a/tests/src/unit/core/formatConversion/export/__snapshots__/html/container/nested.html b/tests/src/unit/core/formatConversion/export/__snapshots__/html/container/nested.html new file mode 100644 index 0000000000..142c337a64 --- /dev/null +++ b/tests/src/unit/core/formatConversion/export/__snapshots__/html/container/nested.html @@ -0,0 +1,16 @@ +
+
+

Nested heading

+
+
+

Inner callout child

+
+
+
+
\ No newline at end of file diff --git a/tests/src/unit/core/formatConversion/export/__snapshots__/markdown/container/basic.md b/tests/src/unit/core/formatConversion/export/__snapshots__/markdown/container/basic.md new file mode 100644 index 0000000000..cbce8bbd73 --- /dev/null +++ b/tests/src/unit/core/formatConversion/export/__snapshots__/markdown/container/basic.md @@ -0,0 +1 @@ +Callout child diff --git a/tests/src/unit/core/formatConversion/export/__snapshots__/markdown/container/emptyChildren.md b/tests/src/unit/core/formatConversion/export/__snapshots__/markdown/container/emptyChildren.md new file mode 100644 index 0000000000..8b13789179 --- /dev/null +++ b/tests/src/unit/core/formatConversion/export/__snapshots__/markdown/container/emptyChildren.md @@ -0,0 +1 @@ + diff --git a/tests/src/unit/core/formatConversion/export/__snapshots__/markdown/container/nested.md b/tests/src/unit/core/formatConversion/export/__snapshots__/markdown/container/nested.md new file mode 100644 index 0000000000..dfbe05a322 --- /dev/null +++ b/tests/src/unit/core/formatConversion/export/__snapshots__/markdown/container/nested.md @@ -0,0 +1,3 @@ +# Nested heading + +Inner callout child diff --git a/tests/src/unit/core/formatConversion/export/__snapshots__/nodes/container/basic.json b/tests/src/unit/core/formatConversion/export/__snapshots__/nodes/container/basic.json new file mode 100644 index 0000000000..cd4b7fa368 --- /dev/null +++ b/tests/src/unit/core/formatConversion/export/__snapshots__/nodes/container/basic.json @@ -0,0 +1,33 @@ +[ + { + "attrs": { + "flavor": "tip", + "id": "1", + }, + "content": [ + { + "attrs": { + "id": "2", + }, + "content": [ + { + "attrs": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "content": [ + { + "text": "Callout child", + "type": "text", + }, + ], + "type": "paragraph", + }, + ], + "type": "blockContainer", + }, + ], + "type": "callout", + }, +] \ No newline at end of file diff --git a/tests/src/unit/core/formatConversion/export/__snapshots__/nodes/container/emptyChildren.json b/tests/src/unit/core/formatConversion/export/__snapshots__/nodes/container/emptyChildren.json new file mode 100644 index 0000000000..7b02725a5d --- /dev/null +++ b/tests/src/unit/core/formatConversion/export/__snapshots__/nodes/container/emptyChildren.json @@ -0,0 +1,27 @@ +[ + { + "attrs": { + "flavor": "tip", + "id": "1", + }, + "content": [ + { + "attrs": { + "id": "1", + }, + "content": [ + { + "attrs": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + ], + "type": "blockContainer", + }, + ], + "type": "callout", + }, +] \ No newline at end of file diff --git a/tests/src/unit/core/formatConversion/export/__snapshots__/nodes/container/nested.json b/tests/src/unit/core/formatConversion/export/__snapshots__/nodes/container/nested.json new file mode 100644 index 0000000000..4a45ff2fa9 --- /dev/null +++ b/tests/src/unit/core/formatConversion/export/__snapshots__/nodes/container/nested.json @@ -0,0 +1,66 @@ +[ + { + "attrs": { + "flavor": "warning", + "id": "1", + }, + "content": [ + { + "attrs": { + "id": "2", + }, + "content": [ + { + "attrs": { + "backgroundColor": "default", + "isToggleable": false, + "level": 1, + "textAlignment": "left", + "textColor": "default", + }, + "content": [ + { + "text": "Nested heading", + "type": "text", + }, + ], + "type": "heading", + }, + ], + "type": "blockContainer", + }, + { + "attrs": { + "flavor": "info", + "id": "3", + }, + "content": [ + { + "attrs": { + "id": "4", + }, + "content": [ + { + "attrs": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "content": [ + { + "text": "Inner callout child", + "type": "text", + }, + ], + "type": "paragraph", + }, + ], + "type": "blockContainer", + }, + ], + "type": "callout", + }, + ], + "type": "callout", + }, +] \ No newline at end of file diff --git a/tests/src/unit/core/formatConversion/export/exportTestInstances.ts b/tests/src/unit/core/formatConversion/export/exportTestInstances.ts index 1d901c81a9..54926c192a 100644 --- a/tests/src/unit/core/formatConversion/export/exportTestInstances.ts +++ b/tests/src/unit/core/formatConversion/export/exportTestInstances.ts @@ -3107,6 +3107,64 @@ export const exportTestInstancesBlockNoteHTML: TestInstance< }, executeTest: testExportBlockNoteHTML, }, + { + testCase: { + name: "container/basic", + content: [ + { + type: "callout", + children: [ + { + type: "paragraph", + content: "Callout child", + }, + ], + }, + ], + }, + executeTest: testExportBlockNoteHTML, + }, + { + testCase: { + name: "container/nested", + content: [ + { + type: "callout", + props: { flavor: "warning" }, + children: [ + { + type: "heading", + content: "Nested heading", + }, + { + type: "callout", + props: { flavor: "info" }, + children: [ + { + type: "paragraph", + content: "Inner callout child", + }, + ], + }, + ], + }, + ], + }, + executeTest: testExportBlockNoteHTML, + }, + { + // A container with no `children` key falls back to the spec's `default`, + // so this exports as a callout holding one empty paragraph. + testCase: { + name: "container/emptyChildren", + content: [ + { + type: "callout", + }, + ], + }, + executeTest: testExportBlockNoteHTML, + }, ]; export const exportTestInstancesHTML: TestInstance< diff --git a/tests/src/unit/core/formatConversion/exportParseEquality/exportParseEqualityTestInstances.ts b/tests/src/unit/core/formatConversion/exportParseEquality/exportParseEqualityTestInstances.ts index eb83a37e86..a1016f98a6 100644 --- a/tests/src/unit/core/formatConversion/exportParseEquality/exportParseEqualityTestInstances.ts +++ b/tests/src/unit/core/formatConversion/exportParseEquality/exportParseEqualityTestInstances.ts @@ -21,10 +21,18 @@ export const exportParseEqualityTestInstancesBlockNoteHTML: TestInstance< TestBlockSchema, TestInlineContentSchema, TestStyleSchema ->[] = exportTestInstancesBlockNoteHTML.map(({ testCase }) => ({ - testCase, - executeTest: testExportParseEqualityBlockNoteHTML, -})); +>[] = exportTestInstancesBlockNoteHTML + // `container/emptyChildren` round-trips asymmetrically by design. Exporting + // reads the partial blocks as given, so a container without a `children` key + // serialises an empty children holder. Parsing goes through a real document, + // where the container's `default` fills that holder. Both halves are correct, + // but they aren't each other's inverse. The export snapshot records the + // serialised form; asserting equality here would only assert the mismatch. + .filter(({ testCase }) => testCase.name !== "container/emptyChildren") + .map(({ testCase }) => ({ + testCase, + executeTest: testExportParseEqualityBlockNoteHTML, + })); export const exportParseEqualityTestInstancesHTML: TestInstance< ExportParseEqualityTestCase< diff --git a/tests/src/unit/core/formatConversion/parse/__snapshots__/html/container.json b/tests/src/unit/core/formatConversion/parse/__snapshots__/html/container.json new file mode 100644 index 0000000000..9c1e864bd8 --- /dev/null +++ b/tests/src/unit/core/formatConversion/parse/__snapshots__/html/container.json @@ -0,0 +1,29 @@ +[ + { + "children": [ + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Callout child", + "type": "text", + }, + ], + "id": "2", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + ], + "content": undefined, + "id": "1", + "props": { + "flavor": "tip", + }, + "type": "callout", + }, +] \ No newline at end of file diff --git a/tests/src/unit/core/formatConversion/parse/__snapshots__/html/containerEmptyChildren.json b/tests/src/unit/core/formatConversion/parse/__snapshots__/html/containerEmptyChildren.json new file mode 100644 index 0000000000..3333d9ac16 --- /dev/null +++ b/tests/src/unit/core/formatConversion/parse/__snapshots__/html/containerEmptyChildren.json @@ -0,0 +1,23 @@ +[ + { + "children": [ + { + "children": [], + "content": [], + "id": "1", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + ], + "content": undefined, + "id": "1", + "props": { + "flavor": "tip", + }, + "type": "callout", + }, +] \ No newline at end of file diff --git a/tests/src/unit/core/formatConversion/parse/__snapshots__/html/containerExternalHTML.json b/tests/src/unit/core/formatConversion/parse/__snapshots__/html/containerExternalHTML.json new file mode 100644 index 0000000000..7d0dc770cb --- /dev/null +++ b/tests/src/unit/core/formatConversion/parse/__snapshots__/html/containerExternalHTML.json @@ -0,0 +1,58 @@ +[ + { + "children": [ + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Nested heading", + "type": "text", + }, + ], + "id": "1", + "props": { + "backgroundColor": "default", + "isToggleable": false, + "level": 1, + "textAlignment": "left", + "textColor": "default", + }, + "type": "heading", + }, + { + "children": [ + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Inner callout child", + "type": "text", + }, + ], + "id": "2", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + ], + "content": undefined, + "id": "3", + "props": { + "flavor": "info", + }, + "type": "callout", + }, + ], + "content": undefined, + "id": "1", + "props": { + "flavor": "warning", + }, + "type": "callout", + }, +] \ No newline at end of file diff --git a/tests/src/unit/core/formatConversion/parse/__snapshots__/html/containerNested.json b/tests/src/unit/core/formatConversion/parse/__snapshots__/html/containerNested.json new file mode 100644 index 0000000000..f26c42c127 --- /dev/null +++ b/tests/src/unit/core/formatConversion/parse/__snapshots__/html/containerNested.json @@ -0,0 +1,58 @@ +[ + { + "children": [ + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Nested heading", + "type": "text", + }, + ], + "id": "2", + "props": { + "backgroundColor": "default", + "isToggleable": false, + "level": 1, + "textAlignment": "left", + "textColor": "default", + }, + "type": "heading", + }, + { + "children": [ + { + "children": [], + "content": [ + { + "styles": {}, + "text": "Inner callout child", + "type": "text", + }, + ], + "id": "4", + "props": { + "backgroundColor": "default", + "textAlignment": "left", + "textColor": "default", + }, + "type": "paragraph", + }, + ], + "content": undefined, + "id": "3", + "props": { + "flavor": "info", + }, + "type": "callout", + }, + ], + "content": undefined, + "id": "1", + "props": { + "flavor": "warning", + }, + "type": "callout", + }, +] \ No newline at end of file diff --git a/tests/src/unit/core/formatConversion/parse/parseTestInstances.ts b/tests/src/unit/core/formatConversion/parse/parseTestInstances.ts index 536a7ac784..7c565b19ae 100644 --- a/tests/src/unit/core/formatConversion/parse/parseTestInstances.ts +++ b/tests/src/unit/core/formatConversion/parse/parseTestInstances.ts @@ -1119,6 +1119,92 @@ l'utilisateur (bouton bleu en haut à droite de la conversation) +
+
+
+
+
+

Callout child

+
+
+
+
+
+`, + }, + executeTest: testParseHTML, + }, + { + // A container nested inside another, with a non-default prop on each. + testCase: { + name: "containerNested", + content: `
+
+
+
+
+
+

Nested heading

+
+
+
+
+
+
+
+
+

Inner callout child

+
+
+
+
+
+
+
+
`, + }, + executeTest: testParseHTML, + }, + { + // A container whose children holder is serialized empty. Parsing goes + // through a real document, so the spec's `default` children fill it back + // in. This is why `container/emptyChildren` is excluded from the + // export/parse equality matrix. + testCase: { + name: "containerEmptyChildren", + content: `
+
+
+
+
`, + }, + executeTest: testParseHTML, + }, + { + // The external (`blocksToHTMLLossy`) form, which is what lands on the + // clipboard and what another app would paste in. The holder carries no + // `data-children-of` marker. + testCase: { + name: "containerExternalHTML", + content: `
+
+

Nested heading

+
+
+

Inner callout child

+
+
+
+
`, + }, + executeTest: testParseHTML, + }, ]; export const parseTestInstancesMarkdown: TestInstance< diff --git a/tests/src/unit/core/schema/__snapshots__/blocks.json b/tests/src/unit/core/schema/__snapshots__/blocks.json index ee48987244..9ae0749202 100644 --- a/tests/src/unit/core/schema/__snapshots__/blocks.json +++ b/tests/src/unit/core/schema/__snapshots__/blocks.json @@ -73,6 +73,36 @@ "toExternalHTML": [Function], }, }, + "callout": { + "config": { + "children": { + "allow": "any", + "default": [ + { + "type": "paragraph", + }, + ], + }, + "content": "none", + "propSchema": { + "flavor": { + "default": "tip", + "values": [ + "tip", + "info", + "warning", + ], + }, + }, + "type": "callout", + }, + "extensions": undefined, + "implementation": { + "node": null, + "render": [Function], + "toExternalHTML": [Function], + }, + }, "checkListItem": { "config": { "content": "inline", diff --git a/tests/src/unit/core/testSchema.ts b/tests/src/unit/core/testSchema.ts index eca37363fa..c3e03c3227 100644 --- a/tests/src/unit/core/testSchema.ts +++ b/tests/src/unit/core/testSchema.ts @@ -99,6 +99,41 @@ const SimpleCustomParagraph = createBlockSpec( }, ); +// A container block: it holds no inline content of its own, and its `contentDOM` +// is where its child blocks go. Covers containers in the format-conversion, +// clipboard and selection matrices, which otherwise never see one. +const Callout = createBlockSpec( + { + type: "callout" as const, + propSchema: { + flavor: { + default: "tip" as const, + values: ["tip", "info", "warning"] as const, + }, + }, + content: "none", + children: { + allow: "any", + default: [{ type: "paragraph" }], + }, + }, + { + render: () => { + const callout = document.createElement("div"); + callout.className = "callout"; + + const body = document.createElement("div"); + body.className = "callout-body"; + callout.appendChild(body); + + return { + dom: callout, + contentDOM: body, + }; + }, + }, +); + // INLINE CONTENT -------------------------------------------------------------- const Mention = createInlineContentSpec( @@ -222,6 +257,7 @@ export const testSchema = BlockNoteSchema.create().extend({ customParagraph: CustomParagraph(), simpleCustomParagraph: SimpleCustomParagraph(), simpleImage: SimpleImage(), + callout: Callout(), }, inlineContentSpecs: { mention: Mention,