refactor(core): consolidate block positioning and selection helpers

Author: nperez0111

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

@@ -13,10 +13,7 @@ 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
@@ -63,13 +60,14 @@ export function getInsertionPos(
   const info = getBlockInfoFromNode(node, posBeforeNode);
 
   if (info.children) {
-    // The descent helpers can stop at sealed boundaries but this caller lets
-    // them cross: an explicit `insertBlocks` placement is an intentional
+    // The descent helper can stop at sealed boundaries but this caller lets
+    // it cross: an explicit `insertBlocks` placement is an intentional
     // crossing.
-    const pos =
-      placement === "first-child"
-        ? descendToFirstInsertionPos(info, nodeType)
-        : descendToLastInsertionPos(info, nodeType);
+    const pos = descendToInsertionPos(
+      info,
+      nodeType,
+      placement === "first-child" ? "first" : "last",
+    );
 
     return pos === null ? null : { pos };
   }

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

@@ -49,21 +49,28 @@ export const mergeBlocksCommand =
       return false;
     }
 
-    // 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.children) {
-      const childBlocksStart = state.doc.resolve(
-        nextBlockInfo.children.childrenStart,
-      );
-      const childBlocksEnd = state.doc.resolve(
-        nextBlockInfo.children.childrenEnd,
-      );
-      const childBlocksRange = childBlocksStart.blockRange(childBlocksEnd);
+    // 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 (dispatch) {
-        const pos = state.doc.resolve(nextBlockInfo.block.beforePos);
-        state.tr.lift(childBlocksRange!, pos.depth);
+      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

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

@@ -1,3 +1,4 @@
+import type { Schema } from "prosemirror-model";
 import {
   NodeSelection,
   Selection,
@@ -9,15 +10,43 @@ import { CellSelection } from "prosemirror-tables";
 import { Block } from "../../../../blocks/defaultBlocks.js";
 import type { BlockNoteEditor } from "../../../../editor/BlockNoteEditor";
 import { BlockIdentifier } from "../../../../schema/index.js";
+import {
+  isBlockGroupInsertable,
+  isContainerNode,
+} 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<any, any, any>[],
+  pmSchema: Schema,
+): Block<any, any, any>[] {
+  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";
@@ -162,9 +191,7 @@ export function moveBlocks(
     removeAndInsertBlocks(tr, blocks, [], { fixContainers: false });
     insertBlocks<any, any, any>(
       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,
     );
@@ -213,17 +240,16 @@ function checkPlacementIsValid(
   placement: "before" | "after",
   movedBlock: Block<any, any, any>,
 ): boolean {
-  // The PM node type to validate the 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.
-  const first = flattenNonInsertableBlocks([movedBlock], editor.pmSchema)[0];
-  const firstType = first?.type ? editor.pmSchema.nodes[first.type] : undefined;
-  const nodeType = firstType?.isInGroup("bnBlock")
-    ? firstType
-    : editor.pmSchema.nodes["blockContainer"];
+  // 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);

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<any, any, any>) {
-  return editor.transact((tr) => {
-    return sinkItem(
+function nestCommand(editor: BlockNoteEditor<any, any, any>) {
+  return (tr: Transaction) =>
+    sinkItem(
       tr,
       editor.pmSchema.nodes["blockContainer"],
       editor.pmSchema.nodes["blockGroup"],
     );
-  });
+}
+
+export function nestBlock(editor: BlockNoteEditor<any, any, any>) {
+  return editor.transact(nestCommand(editor));
 }
 
 /**
@@ -177,50 +179,28 @@ export function liftItem(
   return false;
 }
 
-export function unnestBlock(editor: BlockNoteEditor<any, any, any>) {
-  return editor.transact((tr) =>
+function unnestCommand(editor: BlockNoteEditor<any, any, any>) {
+  return (tr: Transaction) =>
     liftItem(
       tr,
       editor.pmSchema.nodes["blockContainer"],
       editor.pmSchema.nodes["blockGroup"],
-    ),
-  );
+    );
 }
 
+export function unnestBlock(editor: BlockNoteEditor<any, any, any>) {
+  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<any, any, any>) {
-  return editor.transact((tr) => {
-    const { block: 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<any, any, any>) {
-  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));
 }

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)) {
@@ -112,9 +111,6 @@ export function removeAndInsertBlocks<
       tr.delete($pos.pos, $pos.pos + node.nodeSize);
     }
 
-    const newDocSize = tr.doc.nodeSize;
-    removedSize += oldDocSize - newDocSize;
-
     return false;
   });
 

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

@@ -106,28 +106,11 @@ export function updateBlockTr<
       ? replaceToPos - blockInfo.contentStart
       : undefined;
 
-  if (
-    blockInfo.hasContent &&
-    blockInfo.block.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.hasContent && newNodeType.isInGroup("bnBlock")) {
-    updateChildren(block, tr, blockInfo);
-    // old node was a block type (like column or columnList) and new block as well
-    // No op, we just update the block 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.
@@ -165,6 +148,22 @@ export function updateBlockTr<
     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