From d6275a0c3e4720c5e57ef4d9917682461bb418b9 Mon Sep 17 00:00:00 2001 From: Nick the Sick Date: Fri, 21 Aug 2026 21:52:08 +0200 Subject: [PATCH] feat(xl-multi-column): migrate columns onto the container block API --- .../replaceBlocks/util/fixColumnList.ts | 173 ------------------ .../containers/containerUI.ts | 11 +- .../containers/fixContainer.ts | 7 - .../html/util/serializeBlocksInternalHTML.ts | 14 -- .../src/api/nodeConversions/blockToNode.ts | 16 -- .../api/nodeConversions/fragmentToBlocks.ts | 17 -- .../managers/ExtensionManager/extensions.ts | 5 - packages/core/src/exporter/Exporter.ts | 6 - packages/core/src/index.ts | 3 - .../src/blocks/Columns/index.ts | 84 +++++++-- .../ColumnResize/ColumnResizeExtension.ts | 91 +++++++-- .../DropCursor/multiColumnDropCursor.ts | 18 +- .../DropCursor/multiColumnHandleDropPlugin.ts | 121 +++++++----- .../xl-multi-column/src/pm-nodes/Column.ts | 91 --------- .../src/pm-nodes/ColumnList.ts | 47 ----- .../src/test/commands/enter.test.ts | 100 ++++++++++ ...test.ts.snap => fixContainer.test.ts.snap} | 14 +- ...lumnLists.test.ts => fixContainer.test.ts} | 38 ++-- .../multi-column/undefined/external.html | 2 +- .../multi-column/undefined/internal.html | 2 +- .../src/test/extensions/columnResize.test.ts | 101 ++++++++++ .../multicolumn/multicolumn.test.tsx | 51 ++++++ 22 files changed, 520 insertions(+), 492 deletions(-) delete mode 100644 packages/core/src/api/blockManipulation/commands/replaceBlocks/util/fixColumnList.ts delete mode 100644 packages/xl-multi-column/src/pm-nodes/Column.ts delete mode 100644 packages/xl-multi-column/src/pm-nodes/ColumnList.ts create mode 100644 packages/xl-multi-column/src/test/commands/enter.test.ts rename packages/xl-multi-column/src/test/commands/util/__snapshots__/{fixColumnLists.test.ts.snap => fixContainer.test.ts.snap} (95%) rename packages/xl-multi-column/src/test/commands/util/{fixColumnLists.test.ts => fixContainer.test.ts} (91%) create mode 100644 packages/xl-multi-column/src/test/extensions/columnResize.test.ts diff --git a/packages/core/src/api/blockManipulation/commands/replaceBlocks/util/fixColumnList.ts b/packages/core/src/api/blockManipulation/commands/replaceBlocks/util/fixColumnList.ts deleted file mode 100644 index 3097851f47..0000000000 --- a/packages/core/src/api/blockManipulation/commands/replaceBlocks/util/fixColumnList.ts +++ /dev/null @@ -1,173 +0,0 @@ -import { Slice, type Node } from "prosemirror-model"; -import { type Transaction } from "prosemirror-state"; -import { ReplaceAroundStep } from "prosemirror-transform"; - -/** - * Checks if a `column` node is empty, i.e. if it has only a single empty - * paragraph. - * @param column The column to check. - * @returns Whether the column is empty. - */ -export function isEmptyColumn(column: Node) { - if (!column || column.type.name !== "column") { - throw new Error("Invalid columnPos: does not point to column node."); - } - - const blockContainer = column.firstChild; - if (!blockContainer) { - throw new Error("Invalid column: does not have child node."); - } - - const blockContent = blockContainer.firstChild; - if (!blockContent) { - throw new Error("Invalid blockContainer: does not have child node."); - } - - return ( - column.childCount === 1 && - blockContainer.childCount === 1 && - blockContent.type.name === "paragraph" && - blockContent.content.content.length === 0 - ); -} - -/** - * Removes all empty `column` nodes in a `columnList`. A `column` node is empty - * if it has only a single empty block. If, however, removing the `column`s - * leaves the `columnList` that has fewer than two, ProseMirror will re-add - * empty columns. - * @param tr The `Transaction` to add the changes to. - * @param columnListPos The position just before the `columnList` node. - */ -export function removeEmptyColumns(tr: Transaction, columnListPos: number) { - const $columnListPos = tr.doc.resolve(columnListPos); - const columnList = $columnListPos.nodeAfter; - if (!columnList || columnList.type.name !== "columnList") { - throw new Error( - "Invalid columnListPos: does not point to columnList node.", - ); - } - - for ( - let columnIndex = columnList.childCount - 1; - columnIndex >= 0; - columnIndex-- - ) { - const columnPos = tr.doc - .resolve($columnListPos.pos + 1) - .posAtIndex(columnIndex); - const $columnPos = tr.doc.resolve(columnPos); - const column = $columnPos.nodeAfter; - if (!column || column.type.name !== "column") { - throw new Error("Invalid columnPos: does not point to column node."); - } - - if (isEmptyColumn(column)) { - tr.delete(columnPos, columnPos + column.nodeSize); - } - } -} - -/** - * Fixes potential issues in a `columnList` node after a - * `blockContainer`/`column` node is (re)moved from it: - * - * - Removes all empty `column` nodes. A `column` node is empty if it has only - * a single empty block. - * - If all but one `column` nodes are empty, replaces the `columnList` with - * the content of the non-empty `column`. - * - If all `column` nodes are empty, removes the `columnList` entirely. - * @param tr The `Transaction` to add the changes to. - * @param columnListPos - * @returns The position just before the `columnList` node. - */ -export function fixColumnList(tr: Transaction, columnListPos: number) { - removeEmptyColumns(tr, columnListPos); - - const $columnListPos = tr.doc.resolve(columnListPos); - const columnList = $columnListPos.nodeAfter; - if (!columnList || columnList.type.name !== "columnList") { - throw new Error( - "Invalid columnListPos: does not point to columnList node.", - ); - } - - if (columnList.childCount > 2) { - // Do nothing if the `columnList` has more than two non-empty `column`s. In - // the case that the `columnList` has exactly two columns, we may need to - // still remove it, as it's possible that one or both columns are empty. - // This is because after `removeEmptyColumns` is called, if the - // `columnList` has fewer than two `column`s, ProseMirror will re-add empty - // `column`s until there are two total, in order to fit the schema. - return; - } - - if (columnList.childCount < 2) { - // Throw an error if the `columnList` has fewer than two columns. After - // `removeEmptyColumns` is called, if the `columnList` has fewer than two - // `column`s, ProseMirror will re-add empty `column`s until there are two - // total, in order to fit the schema. So if there are fewer than two here, - // either the schema, or ProseMirror's internals, must have changed. - throw new Error("Invalid columnList: contains fewer than two children."); - } - - const firstColumnBeforePos = columnListPos + 1; - const $firstColumnBeforePos = tr.doc.resolve(firstColumnBeforePos); - const firstColumn = $firstColumnBeforePos.nodeAfter; - - const lastColumnAfterPos = columnListPos + columnList.nodeSize - 1; - const $lastColumnAfterPos = tr.doc.resolve(lastColumnAfterPos); - const lastColumn = $lastColumnAfterPos.nodeBefore; - - if (!firstColumn || !lastColumn) { - throw new Error("Invalid columnList: does not contain children."); - } - - const firstColumnEmpty = isEmptyColumn(firstColumn); - const lastColumnEmpty = isEmptyColumn(lastColumn); - - if (firstColumnEmpty && lastColumnEmpty) { - // Removes `columnList` - tr.delete(columnListPos, columnListPos + columnList.nodeSize); - - return; - } - - if (firstColumnEmpty) { - tr.step( - new ReplaceAroundStep( - // Replaces `columnList`. - columnListPos, - columnListPos + columnList.nodeSize, - // Replaces with content of last `column`. - lastColumnAfterPos - lastColumn.nodeSize + 1, - lastColumnAfterPos - 1, - // Doesn't append anything. - Slice.empty, - 0, - false, - ), - ); - - return; - } - - if (lastColumnEmpty) { - tr.step( - new ReplaceAroundStep( - // Replaces `columnList`. - columnListPos, - columnListPos + columnList.nodeSize, - // Replaces with content of first `column`. - firstColumnBeforePos + 1, - firstColumnBeforePos + firstColumn.nodeSize - 1, - // Doesn't append anything. - Slice.empty, - 0, - false, - ), - ); - - return; - } -} diff --git a/packages/core/src/api/blockManipulation/containers/containerUI.ts b/packages/core/src/api/blockManipulation/containers/containerUI.ts index 08fae1f7cf..b4633d5ead 100644 --- a/packages/core/src/api/blockManipulation/containers/containerUI.ts +++ b/packages/core/src/api/blockManipulation/containers/containerUI.ts @@ -40,21 +40,14 @@ export function getContainerUIInfo( )) { const draggable = spec.implementation?.meta?.draggable !== false; - // Legacy: `@blocknote/xl-multi-column`'s hand-written specs, which have - // no `children` config. Removed once multi-column is migrated onto the - // container API. - const isLegacyColumnType = type === "columnList" || type === "column"; - - if (!isContainerType(spec.config) && !isLegacyColumnType) { + if (!isContainerType(spec.config)) { if (!draggable) { nonDraggableBlockTypes.add(type); } continue; } containerTypes.add(type); - // Legacy column nodes are never draggable themselves; only the blocks - // inside them are (matching the pre-container side menu behavior). - if (draggable && !isLegacyColumnType) { + if (draggable) { draggableContainerTypes.add(type); } } diff --git a/packages/core/src/api/blockManipulation/containers/fixContainer.ts b/packages/core/src/api/blockManipulation/containers/fixContainer.ts index a98a73386f..dec27e3660 100644 --- a/packages/core/src/api/blockManipulation/containers/fixContainer.ts +++ b/packages/core/src/api/blockManipulation/containers/fixContainer.ts @@ -14,7 +14,6 @@ import { import type { ResolvedChildren } from "../../../schema/blocks/children.js"; import { seedRefillChildren } from "../../nodeConversions/blockToNode.js"; import { getNodeById } from "../../nodeUtil.js"; -import { fixColumnList } from "../commands/replaceBlocks/util/fixColumnList.js"; // Defined in `children.ts` (it answers a schema-level question); re-exported // here because the public root export (`index.ts`) imports it from this @@ -149,12 +148,6 @@ export function fixContainer(tr: Transaction, containerPos: number) { const config = childrenConfig ? resolveChildren(childrenConfig) : undefined; if (!config) { - // Legacy repair for `@blocknote/xl-multi-column`'s hand-written PM nodes, - // which have no `children` config but sit in the `childContainer` group. - // Removed once multi-column is migrated onto the container API. - if (target.blockNode.type.name === "columnList") { - fixColumnList(tr, target.blockPos); - } return; } diff --git a/packages/core/src/api/exporters/html/util/serializeBlocksInternalHTML.ts b/packages/core/src/api/exporters/html/util/serializeBlocksInternalHTML.ts index 37f1cdfce5..bf9ca43e06 100644 --- a/packages/core/src/api/exporters/html/util/serializeBlocksInternalHTML.ts +++ b/packages/core/src/api/exporters/html/util/serializeBlocksInternalHTML.ts @@ -256,20 +256,6 @@ function serializeBlock< return ret.dom; } - // Legacy path for `@blocknote/xl-multi-column`'s hand-written PM nodes, - // which sit in the `bnBlock` group but have no `children` config. They own - // their outer DOM and hold their children directly in their `contentDOM`. - // Removed once multi-column is migrated onto the container API. - const pmType = editor.pmSchema.nodes[block.type!]; - if (pmType?.isInGroup("bnBlock")) { - if (block.children && block.children.length > 0) { - ret.contentDOM?.append( - serializeBlocks(editor, block.children, serializer, options), - ); - } - return ret.dom; - } - // wrap the block in a blockContainer const bc = BC_NODE.spec?.toDOM?.( BC_NODE.create({ diff --git a/packages/core/src/api/nodeConversions/blockToNode.ts b/packages/core/src/api/nodeConversions/blockToNode.ts index a3beec552b..f662d56952 100644 --- a/packages/core/src/api/nodeConversions/blockToNode.ts +++ b/packages/core/src/api/nodeConversions/blockToNode.ts @@ -601,22 +601,6 @@ export function blockToNode( childrenNode, ]), ); - } else if ( - schema.nodes[block.type].isInGroup("bnBlock") && - !getChildrenConfig(schema.nodes[block.type].spec.blockConfig ?? {}) - ) { - // Legacy path for `@blocknote/xl-multi-column`'s hand-written PM nodes, - // which sit in the `bnBlock` group but have no `children` config. Plain - // `create` (not `createChecked` and no fill), so invalid structures - // surface via `node.check()` when the caller mutates the doc. Removed - // once multi-column is migrated onto the container API. - return schema.nodes[block.type].create( - { - id: id, - ...block.props, - }, - children, - ); } else if (isContainerNode(schema.nodes[block.type])) { const type = schema.nodes[block.type]; const attrs = { id: id, ...block.props }; diff --git a/packages/core/src/api/nodeConversions/fragmentToBlocks.ts b/packages/core/src/api/nodeConversions/fragmentToBlocks.ts index 26a0a82b54..8b99ae669d 100644 --- a/packages/core/src/api/nodeConversions/fragmentToBlocks.ts +++ b/packages/core/src/api/nodeConversions/fragmentToBlocks.ts @@ -92,23 +92,6 @@ export function fragmentToBlocks< } if (node.type.isInGroup("bnBlock")) { - // Legacy path for `@blocknote/xl-multi-column`'s hand-written PM nodes, - // which have no `children` config: flatten only a single-column - // columnList (not the entire column list has been selected), and keep - // every other column list intact, as before. Removed once multi-column - // is migrated onto the container API. - const blockConfig = getBlockSchema(node.type.schema)[node.type.name]; - if (isContainerNode(node.type) && !getChildrenConfig(blockConfig ?? {})) { - if (node.type.name === "columnList" && node.childCount === 1) { - node.firstChild?.forEach((child) => { - blocks.push(nodeToBlock(child, node)); - }); - return false; - } - blocks.push(nodeToBlock(node, node)); - return false; - } - pushFlattened(node, node); return false; } diff --git a/packages/core/src/editor/managers/ExtensionManager/extensions.ts b/packages/core/src/editor/managers/ExtensionManager/extensions.ts index 1d54a508d4..0302caaa7d 100644 --- a/packages/core/src/editor/managers/ExtensionManager/extensions.ts +++ b/packages/core/src/editor/managers/ExtensionManager/extensions.ts @@ -65,11 +65,6 @@ export function getDefaultTiptapExtensions( // everything from bnBlock group (nodes that represent a BlockNote block should have an id) types: [ "blockContainer", - // Legacy: `@blocknote/xl-multi-column`'s hand-written PM nodes, which - // have no `children` config and so aren't picked up below. Removed - // once multi-column is migrated onto the container API. - "columnList", - "column", // Container block specs whose PM node is itself in the `bnBlock` // group (column, columnList, callout, etc.). The bnBlock node is the // block itself, so the id lives on its attrs rather than on a diff --git a/packages/core/src/exporter/Exporter.ts b/packages/core/src/exporter/Exporter.ts index 5427718ad9..4430c5f399 100644 --- a/packages/core/src/exporter/Exporter.ts +++ b/packages/core/src/exporter/Exporter.ts @@ -85,12 +85,6 @@ export abstract class Exporter< * after the container's own output. */ public isContainerBlock(blockType: string): boolean { - // Legacy: `@blocknote/xl-multi-column`'s hand-written specs, which have - // no `children` config. Removed once multi-column is migrated onto the - // container API. - if (blockType === "columnList" || blockType === "column") { - return true; - } const spec = (this.blockNoteSchema.blockSpecs as Record)[ blockType ]; diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index c1e7a3e7a4..240b0c762a 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -6,9 +6,6 @@ export * from "./api/blockManipulation/commands/replaceBlocks/replaceBlocks.js"; // node type a container?") that integrations legitimately ask. It is defined // in `children.ts` and re-exported via `fixContainer.ts`. export { isContainerNode } from "./api/blockManipulation/containers/fixContainer.js"; -// Legacy column repair for `@blocknote/xl-multi-column`'s hand-written PM -// nodes. Removed once multi-column is migrated onto the container API. -export * from "./api/blockManipulation/commands/replaceBlocks/util/fixColumnList.js"; export * from "./api/blockManipulation/commands/updateBlock/updateBlock.js"; export * from "./api/exporters/html/externalHTMLExporter.js"; export * from "./api/exporters/html/internalHTMLSerializer.js"; diff --git a/packages/xl-multi-column/src/blocks/Columns/index.ts b/packages/xl-multi-column/src/blocks/Columns/index.ts index 2e49261ec6..198de85923 100644 --- a/packages/xl-multi-column/src/blocks/Columns/index.ts +++ b/packages/xl-multi-column/src/blocks/Columns/index.ts @@ -1,28 +1,82 @@ +import { createBlockSpec } from "@blocknote/core"; + +import { ColumnResizeExtension } from "../../extensions/ColumnResize/ColumnResizeExtension.js"; import { MultiColumnDropHandlerExtension } from "../../extensions/DropCursor/multiColumnHandleDropPlugin.js"; -import { Column } from "../../pm-nodes/Column.js"; -import { ColumnList } from "../../pm-nodes/ColumnList.js"; -import { createBlockSpecFromTiptapNode } from "@blocknote/core"; +const COLUMN_WIDTH_DEFAULT = 1; -export const ColumnBlock = createBlockSpecFromTiptapNode( +export const ColumnBlock = createBlockSpec( { - node: Column, - type: "column", + type: "column" as const, + propSchema: { + width: { + default: COLUMN_WIDTH_DEFAULT, + }, + }, content: "none", + children: { allow: "any" }, + placement: "containerOnly", }, { - width: { - default: 1, + meta: { + draggable: false, + }, + render: (block) => { + const dom = document.createElement("div"); + dom.className = "bn-block-column"; + dom.style.flexGrow = String(block.props.width ?? COLUMN_WIDTH_DEFAULT); + + return { + dom, + contentDOM: dom, + update: (newNode: { + type: { name: string }; + attrs: { width?: number }; + }) => { + if (newNode.type.name !== "column") { + return false; + } + dom.style.flexGrow = String( + newNode.attrs.width ?? COLUMN_WIDTH_DEFAULT, + ); + return true; + }, + }; }, }, - [MultiColumnDropHandlerExtension()], -); + [MultiColumnDropHandlerExtension(), ColumnResizeExtension()], +)(); -export const ColumnListBlock = createBlockSpecFromTiptapNode( +export const ColumnListBlock = createBlockSpec( { - node: ColumnList, - type: "columnList", + type: "columnList" as const, + propSchema: {}, content: "none", + children: { + allow: ["column"], + min: 2, + whenEmptied: "unwrap", + // Everything crosses the column list's edge, e.g. a text selection + // dragged across columns. + boundary: "open", + }, + }, + { + meta: { + draggable: false, + }, + render: () => { + const dom = document.createElement("div"); + dom.className = "bn-block-column-list"; + dom.style.display = "flex"; + + return { + dom, + contentDOM: dom, + update: (newNode: { type: { name: string } }) => { + return newNode.type.name === "columnList"; + }, + }; + }, }, - {}, -); +)(); diff --git a/packages/xl-multi-column/src/extensions/ColumnResize/ColumnResizeExtension.ts b/packages/xl-multi-column/src/extensions/ColumnResize/ColumnResizeExtension.ts index 5713466a6d..1d2da18690 100644 --- a/packages/xl-multi-column/src/extensions/ColumnResize/ColumnResizeExtension.ts +++ b/packages/xl-multi-column/src/extensions/ColumnResize/ColumnResizeExtension.ts @@ -1,6 +1,5 @@ -import { BlockNoteEditor, getNodeById } from "@blocknote/core"; +import { BlockNoteEditor, createExtension, getNodeById } from "@blocknote/core"; import { SideMenuExtension } from "@blocknote/core/extensions"; -import { Extension } from "@tiptap/core"; import { Node } from "prosemirror-model"; import { Plugin, PluginKey, PluginView } from "prosemirror-state"; import { Decoration, DecorationSet, EditorView } from "prosemirror-view"; @@ -41,13 +40,71 @@ type ColumnResizeState = { columnList: ColumnData; }; -type ColumnState = +// Exported for tests only - not part of the package's public API. +export type ColumnState = | ColumnDefaultState | ColumnHoverState | ColumnHoverColumnListState | ColumnResizeState; -const columnResizePluginKey = new PluginKey("ColumnResizePlugin"); +// Exported for tests only - not part of the package's public API. +export const columnResizePluginKey = new PluginKey( + "ColumnResizePlugin", +); + +// Re-resolves stored column data against a (possibly changed) doc, since the +// stored node and position may be stale. Returns undefined if the node no +// longer exists in the doc. +function refreshColumnData( + data: T, + doc: Node, +): T | undefined { + const nodeAndPos = getNodeById(data.id, doc); + if (!nodeAndPos) { + return undefined; + } + + return { ...data, ...nodeAndPos }; +} + +// Re-resolves all column data stored in the plugin state against a (possibly +// changed) doc. Falls back to the default state if any of the referenced +// nodes no longer exist - e.g. when a backspace removes a hovered column, or +// unwraps the column list entirely - so decorations are never built from +// positions that are invalid in the new doc. +function refreshColumnState(state: ColumnState, doc: Node): ColumnState { + switch (state.type) { + case "default": + return state; + case "hover-column-list": { + const columnList = refreshColumnData(state.columnList, doc); + + return columnList ? { ...state, columnList } : { type: "default" }; + } + case "hover-column": { + const columnList = refreshColumnData(state.columnList, doc); + const leftColumn = refreshColumnData(state.leftColumn, doc); + const rightColumn = refreshColumnData(state.rightColumn, doc); + + if (!columnList || !leftColumn || !rightColumn) { + return { type: "default" }; + } + + return { ...state, columnList, leftColumn, rightColumn }; + } + case "resize": { + const columnList = refreshColumnData(state.columnList, doc); + const leftColumn = refreshColumnData(state.leftColumn, doc); + const rightColumn = refreshColumnData(state.rightColumn, doc); + + if (!columnList || !leftColumn || !rightColumn) { + return { type: "default" }; + } + + return { ...state, columnList, leftColumn, rightColumn }; + } + } +} class ColumnResizePluginView implements PluginView { editor: BlockNoteEditor; @@ -428,22 +485,26 @@ const createColumnResizePlugin = (editor: BlockNoteEditor) => state: { init: () => ({ type: "default" }) as ColumnState, apply: (tr, oldPluginState) => { - const newPluginState = tr.getMeta(columnResizePluginKey) as + const metaPluginState = tr.getMeta(columnResizePluginKey) as | ColumnState | undefined; - return newPluginState === undefined ? oldPluginState : newPluginState; + const pluginState = + metaPluginState === undefined ? oldPluginState : metaPluginState; + + // The stored column nodes & positions were resolved against an older + // doc, so when the doc changes they must be re-resolved against the + // new one - a backspace may have removed a hovered column or + // unwrapped the column list entirely. + return tr.docChanged + ? refreshColumnState(pluginState, tr.doc) + : pluginState; }, }, view: (view) => new ColumnResizePluginView(editor, view), }); -export const createColumnResizeExtension = ( - editor: BlockNoteEditor, -) => - Extension.create({ - name: "columnResize", - addProseMirrorPlugins() { - return [createColumnResizePlugin(editor)]; - }, - }); +export const ColumnResizeExtension = createExtension(({ editor }) => ({ + key: "columnResize", + prosemirrorPlugins: [createColumnResizePlugin(editor)], +})); diff --git a/packages/xl-multi-column/src/extensions/DropCursor/multiColumnDropCursor.ts b/packages/xl-multi-column/src/extensions/DropCursor/multiColumnDropCursor.ts index 77d93b7f4a..8f05068da3 100644 --- a/packages/xl-multi-column/src/extensions/DropCursor/multiColumnDropCursor.ts +++ b/packages/xl-multi-column/src/extensions/DropCursor/multiColumnDropCursor.ts @@ -1,4 +1,8 @@ -import { type DropCursorHooks, getNearestBlockPos } from "@blocknote/core"; +import { + type DropCursorHooks, + getNearestBlockPos, + isContainerNode, +} from "@blocknote/core"; import type { EditorState } from "prosemirror-state"; import type { EditorView } from "prosemirror-view"; @@ -31,10 +35,16 @@ export function detectEdgePosition( const blockPos = getNearestBlockPos(state.doc, eventPos.pos); - // If we're at a block that's in a column, we want to compare the mouse position to the column, not the block inside it - // Why? Because we want to insert a new column in the columnList, instead of a new columnList inside of the column + // If we're at a block inside a column of a columnList, we want to compare + // the mouse position to the column, not the block inside it. + // Why? Because we want to insert a new sibling column in the columnList + // instead of a new container inside the column. let resolved = state.doc.resolve(blockPos.posBeforeNode); - if (resolved.parent.type.name === "column") { + if ( + isContainerNode(resolved.parent.type) && + resolved.depth > 0 && + state.doc.resolve(resolved.before()).parent.type.name === "columnList" + ) { resolved = state.doc.resolve(resolved.before()); } diff --git a/packages/xl-multi-column/src/extensions/DropCursor/multiColumnHandleDropPlugin.ts b/packages/xl-multi-column/src/extensions/DropCursor/multiColumnHandleDropPlugin.ts index a762f78d96..1d9b4bcf21 100644 --- a/packages/xl-multi-column/src/extensions/DropCursor/multiColumnHandleDropPlugin.ts +++ b/packages/xl-multi-column/src/extensions/DropCursor/multiColumnHandleDropPlugin.ts @@ -4,6 +4,7 @@ import { createExtension, fragmentToBlocks, getBlockInfo, + isContainerNode, nodeToBlock, } from "@blocknote/core"; import { Plugin } from "prosemirror-state"; @@ -42,7 +43,14 @@ export function createMultiColumnHandleDropPlugin( } const draggedBlockIds = new Set(draggedBlocks.map((block) => block.id)); - if (blockInfo.blockNoteType === "column") { + // 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 targetInHorizontalContainer = + $target.node().type.name === "columnList"; + + if (targetInHorizontalContainer) { // The user is dropping the target column's entire contents on the // column's own edge - the new column would just replace the // emptied target in the same position, so do nothing. This also @@ -57,16 +65,22 @@ export function createMultiColumnHandleDropPlugin( return true; } - // Insert new column in existing columnList - const parentBlock = view.state.doc - .resolve(blockInfo.bnBlock.beforePos) - .node(); + // Insert a new sibling child in the existing horizontal container + // (e.g. a new column in the columnList). + const parentBlock = $target.node(); const columnList = nodeToBlock( parentBlock, view.state.doc, ); + // Whether the horizontal container's children are typed child + // containers (like `column`) that wrap the actual blocks, or plain + // blocks spliced in directly. + const targetIsChildContainer = isContainerNode( + blockInfo.bnBlock.node.type, + ); + // Normalize column widths to average of 1 // In a `columnList`, we expect that the average width of each column // is 1. However, there are cases in which this stops being true. For @@ -74,24 +88,31 @@ export function createMultiColumnHandleDropPlugin( // the average width to go down. This isn't really an issue until the // user tries to add a new column, which will, in this case, be wider // than expected. Therefore, we normalize the column widths to an - // average of 1 here to avoid this issue. - let sumColumnWidthPercent = 0; - columnList.children.forEach((column) => { - sumColumnWidthPercent += column.props.width as number; - }); - const avgColumnWidthPercent = - sumColumnWidthPercent / columnList.children.length; - - // If the average column width is not 1, normalize it. We're dealing - // with floats so we need a small margin to account for precision - // errors. - if (avgColumnWidthPercent < 0.99 || avgColumnWidthPercent > 1.01) { - const scalingFactor = 1 / avgColumnWidthPercent; - + // average of 1 here to avoid this issue. (Only applies to child + // containers with a numeric `width` prop, i.e. columns.) + if ( + columnList.children.every( + (column) => typeof column.props.width === "number", + ) + ) { + let sumColumnWidthPercent = 0; columnList.children.forEach((column) => { - column.props.width = - (column.props.width as number) * scalingFactor; + sumColumnWidthPercent += column.props.width as number; }); + const avgColumnWidthPercent = + sumColumnWidthPercent / columnList.children.length; + + // If the average column width is not 1, normalize it. We're + // dealing with floats so we need a small margin to account for + // precision errors. + if (avgColumnWidthPercent < 0.99 || avgColumnWidthPercent > 1.01) { + const scalingFactor = 1 / avgColumnWidthPercent; + + columnList.children.forEach((column) => { + column.props.width = + (column.props.width as number) * scalingFactor; + }); + } } const targetColumnId = blockInfo.bnBlock.node.attrs.id; @@ -103,20 +124,26 @@ export function createMultiColumnHandleDropPlugin( const remainingColumns = columnList.children // If any of the dragged blocks are in one of the columns, remove // them. - .map((column) => ({ - ...column, - children: column.children.filter((block) => { - if (!draggedBlockIds.has(block.id)) { - return true; - } - - blocksAlreadyInColumnList.add(block.id); - return false; - }), - })) + .map((column) => + targetIsChildContainer + ? { + ...column, + children: column.children.filter((block) => { + if (!draggedBlockIds.has(block.id)) { + return true; + } + + blocksAlreadyInColumnList.add(block.id); + return false; + }), + } + : column, + ) // Remove empty columns (can happen when dragged blocks are // removed). - .filter((column) => column.children.length > 0); + .filter( + (column) => !targetIsChildContainer || column.children.length > 0, + ); // The insertion index is computed on the remaining columns, as // removing an emptied column before the drop target shifts the @@ -134,15 +161,25 @@ export function createMultiColumnHandleDropPlugin( const insertionIndex = edgePos.position === "left" ? targetIndex : targetIndex + 1; - // Insert the dragged blocks as a new column in the correct - // position. - const newChildren = remainingColumns.toSpliced(insertionIndex, 0, { - type: "column", - children: draggedBlocks, - props: {}, - content: undefined, - id: UniqueID.options.generateID(), - }); + // Insert the dragged blocks in the correct position, wrapped in a + // new child container (e.g. a new `column`) when the container's + // children are typed containers, or spliced in directly otherwise. + const insertedChildren = targetIsChildContainer + ? [ + { + type: blockInfo.blockNoteType, + children: draggedBlocks, + props: {}, + content: undefined, + id: UniqueID.options.generateID(), + }, + ] + : draggedBlocks; + const newChildren = remainingColumns.toSpliced( + insertionIndex, + 0, + ...insertedChildren, + ); const blocksToRemove = draggedBlocks.filter( (block) => diff --git a/packages/xl-multi-column/src/pm-nodes/Column.ts b/packages/xl-multi-column/src/pm-nodes/Column.ts deleted file mode 100644 index dccf60c74b..0000000000 --- a/packages/xl-multi-column/src/pm-nodes/Column.ts +++ /dev/null @@ -1,91 +0,0 @@ -import { suggestionMarks } from "@blocknote/core"; -import { Node } from "@tiptap/core"; - -import { createColumnResizeExtension } from "../extensions/ColumnResize/ColumnResizeExtension.js"; - -export const Column = Node.create({ - name: "column", - group: "bnBlock childContainer", - // A block always contains content, and optionally a blockGroup which contains nested blocks - content: "blockContainer+", - priority: 40, - defining: true, - marks() { - return suggestionMarks(this.editor); - }, - addAttributes() { - return { - width: { - // Why does each column have a default width of 1, i.e. 100%? Because - // when creating a new column, we want to make sure that existing - // column widths are preserved, while the new one also has a sensible - // width. If we'd set it so all column widths must add up to 100% - // instead, then each time a new column is created, we'd have to assign - // it a width depending on the total number of columns and also adjust - // the widths of the other columns. The same can be said for using px - // instead of percent widths and making them add to the editor width. So - // using this method is both simpler and computationally cheaper. This - // is possible because we can set the `flex-grow` property to the width - // value, which handles all the resizing for us, instead of manually - // having to set the `width` property of each column. - default: 1, - parseHTML: (element) => { - const attr = element.getAttribute("data-width"); - if (attr === null) { - return null; - } - - const parsed = parseFloat(attr); - if (isFinite(parsed)) { - return parsed; - } - - return null; - }, - renderHTML: (attributes) => { - return { - "data-width": (attributes.width as number).toString(), - style: `flex-grow: ${attributes.width as number};`, - }; - }, - }, - }; - }, - - parseHTML() { - return [ - { - tag: "div", - getAttrs: (element) => { - if (typeof element === "string") { - return false; - } - - if (element.getAttribute("data-node-type") === this.name) { - return {}; - } - - return false; - }, - }, - ]; - }, - - renderHTML({ HTMLAttributes }) { - const column = document.createElement("div"); - column.className = "bn-block-column"; - column.setAttribute("data-node-type", this.name); - for (const [attribute, value] of Object.entries(HTMLAttributes)) { - column.setAttribute(attribute, value as any); // TODO as any - } - - return { - dom: column, - contentDOM: column, - }; - }, - - addExtensions() { - return [createColumnResizeExtension(this.options.editor)]; - }, -}); diff --git a/packages/xl-multi-column/src/pm-nodes/ColumnList.ts b/packages/xl-multi-column/src/pm-nodes/ColumnList.ts deleted file mode 100644 index eeb06f4d4e..0000000000 --- a/packages/xl-multi-column/src/pm-nodes/ColumnList.ts +++ /dev/null @@ -1,47 +0,0 @@ -import { suggestionMarks } from "@blocknote/core"; -import { Node } from "@tiptap/core"; - -export const ColumnList = Node.create({ - name: "columnList", - group: "childContainer bnBlock blockGroupChild", - // A block always contains content, and optionally a blockGroup which contains nested blocks - content: "column column+", // min two columns - priority: 40, // should be below blockContainer - defining: true, - marks() { - return suggestionMarks(this.editor); - }, - parseHTML() { - return [ - { - tag: "div", - getAttrs: (element) => { - if (typeof element === "string") { - return false; - } - - if (element.getAttribute("data-node-type") === this.name) { - return {}; - } - - return false; - }, - }, - ]; - }, - - renderHTML({ HTMLAttributes }) { - const columnList = document.createElement("div"); - columnList.className = "bn-block-column-list"; - columnList.setAttribute("data-node-type", this.name); - for (const [attribute, value] of Object.entries(HTMLAttributes)) { - columnList.setAttribute(attribute, value as any); // TODO as any - } - columnList.style.display = "flex"; - - return { - dom: columnList, - contentDOM: columnList, - }; - }, -}); diff --git a/packages/xl-multi-column/src/test/commands/enter.test.ts b/packages/xl-multi-column/src/test/commands/enter.test.ts new file mode 100644 index 0000000000..9c121b97b3 --- /dev/null +++ b/packages/xl-multi-column/src/test/commands/enter.test.ts @@ -0,0 +1,100 @@ +import { describe, expect, it } from "vite-plus/test"; + +import { BlockNoteEditor } from "@blocknote/core"; + +import { setupTestEnv } from "../setupTestEnv.js"; + +const getEditor = setupTestEnv(); + +function pressEnter(editor: BlockNoteEditor) { + const view = editor._tiptapEditor.view; + const event = new KeyboardEvent("keydown", { + key: "Enter", + code: "Enter", + keyCode: 13, + bubbles: true, + }); + view.someProp("handleKeyDown", (f: any) => f(view, event)); +} + +// Columns have no special Enter config: like any non-sealed container, an +// empty last block escapes on Enter. The generic mechanics (escape, ascent +// past levels that can't hold the block, mid-container stays) are covered in +// core's `containers.browser.test.ts`; these two tests use the real column +// schema and its interaction with the column-list repair. +describe("Enter exit from columns", () => { + it("typing then double-Enter escapes in two presses", () => { + const editor = getEditor(); + editor.replaceBlocks(editor.document, [ + { + type: "columnList", + id: "cl-0", + children: [ + { + type: "column", + id: "col-1", + children: [{ id: "col1-para", type: "paragraph", content: "col1" }], + }, + { + type: "column", + id: "col-2", + children: [{ id: "col2-para", type: "paragraph", content: "col2" }], + }, + ], + }, + ]); + + editor.setTextCursorPosition("col2-para", "end"); + pressEnter(editor); + + // First press: a new empty block inside the column. + expect(editor.document.map((block) => block.id)).toEqual(["cl-0"]); + const children = editor.getBlock("col-2")!.children; + expect(children).toHaveLength(2); + const created = children[1].id; + expect(editor.getTextCursorPosition().block.id).toBe(created); + + pressEnter(editor); + + // Second press: that block moves below the column list (a block can't sit + // between columns, so the escape lands below the whole list), caret along. + expect(editor.getBlock("col-2")!.children.map((child) => child.id)).toEqual( + ["col2-para"], + ); + expect(editor.document.map((block) => block.id)).toEqual(["cl-0", created]); + expect(editor.getTextCursorPosition().block.id).toBe(created); + }); + + it("escaping a column's only block dissolves it and unwraps the list", () => { + // The exit empties the column, so the column list's `whenEmptied: "unwrap"` + // repair kicks in: the emptied column disappears, and the one-column + // list unwraps to the surviving column's blocks. + const editor = getEditor(); + editor.replaceBlocks(editor.document, [ + { + type: "columnList", + id: "cl-0", + children: [ + { + type: "column", + id: "col-1", + children: [{ id: "col1-para", type: "paragraph", content: "col1" }], + }, + { + type: "column", + id: "col-2", + children: [{ id: "col2-empty", type: "paragraph", content: "" }], + }, + ], + }, + ]); + + editor.setTextCursorPosition("col2-empty", "end"); + pressEnter(editor); + + expect(editor.document.map((block) => block.id)).toEqual([ + "col1-para", + "col2-empty", + ]); + }); +}); diff --git a/packages/xl-multi-column/src/test/commands/util/__snapshots__/fixColumnLists.test.ts.snap b/packages/xl-multi-column/src/test/commands/util/__snapshots__/fixContainer.test.ts.snap similarity index 95% rename from packages/xl-multi-column/src/test/commands/util/__snapshots__/fixColumnLists.test.ts.snap rename to packages/xl-multi-column/src/test/commands/util/__snapshots__/fixContainer.test.ts.snap index 87b5f2e588..a5d8ddf91f 100644 --- a/packages/xl-multi-column/src/test/commands/util/__snapshots__/fixColumnLists.test.ts.snap +++ b/packages/xl-multi-column/src/test/commands/util/__snapshots__/fixContainer.test.ts.snap @@ -1,6 +1,6 @@ // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html -exports[`Test fixColumnList > First of two columns empty 1`] = ` +exports[`Test fixContainer > First of two columns empty 1`] = ` { "content": [ { @@ -35,7 +35,7 @@ exports[`Test fixColumnList > First of two columns empty 1`] = ` } `; -exports[`Test fixColumnList > Last of two columns empty 1`] = ` +exports[`Test fixContainer > Last of two columns empty 1`] = ` { "content": [ { @@ -70,7 +70,7 @@ exports[`Test fixColumnList > Last of two columns empty 1`] = ` } `; -exports[`Test fixColumnList > Two empty columns 1`] = ` +exports[`Test fixContainer > Two empty columns 1`] = ` { "content": [ { @@ -99,7 +99,7 @@ exports[`Test fixColumnList > Two empty columns 1`] = ` } `; -exports[`Test removeEmptyColumns > First of two columns empty 1`] = ` +exports[`Test removeEmptyChildren > First of two columns empty 1`] = ` { "content": [ { @@ -176,7 +176,7 @@ exports[`Test removeEmptyColumns > First of two columns empty 1`] = ` } `; -exports[`Test removeEmptyColumns > Last of two columns empty 1`] = ` +exports[`Test removeEmptyChildren > Last of two columns empty 1`] = ` { "content": [ { @@ -253,7 +253,7 @@ exports[`Test removeEmptyColumns > Last of two columns empty 1`] = ` } `; -exports[`Test removeEmptyColumns > Start and end columns empty 1`] = ` +exports[`Test removeEmptyChildren > Start and end columns empty 1`] = ` { "content": [ { @@ -336,7 +336,7 @@ exports[`Test removeEmptyColumns > Start and end columns empty 1`] = ` } `; -exports[`Test removeEmptyColumns > Two empty columns 1`] = ` +exports[`Test removeEmptyChildren > Two empty columns 1`] = ` { "content": [ { diff --git a/packages/xl-multi-column/src/test/commands/util/fixColumnLists.test.ts b/packages/xl-multi-column/src/test/commands/util/fixContainer.test.ts similarity index 91% rename from packages/xl-multi-column/src/test/commands/util/fixColumnLists.test.ts rename to packages/xl-multi-column/src/test/commands/util/fixContainer.test.ts index b5bd190c6d..d41cc00f72 100644 --- a/packages/xl-multi-column/src/test/commands/util/fixColumnLists.test.ts +++ b/packages/xl-multi-column/src/test/commands/util/fixContainer.test.ts @@ -2,14 +2,14 @@ import { describe, expect, it } from "vite-plus/test"; import { setupTestEnv } from "../../setupTestEnv.js"; import { - fixColumnList, - isEmptyColumn, - removeEmptyColumns, -} from "@blocknote/core"; + fixContainer, + isEmptyContainerChild, + removeEmptyChildren, +} from "@blocknote/core/internal"; const getEditor = setupTestEnv(); -describe("Test isEmptyColumn", () => { +describe("Test isEmptyContainerChild", () => { it("Empty blocks", () => { const schema = getEditor()._tiptapEditor.schema; @@ -19,7 +19,7 @@ describe("Test isEmptyColumn", () => { ]), ]); - expect(isEmptyColumn(column)).toBeTruthy(); + expect(isEmptyContainerChild(column)).toBeTruthy(); }); it("Multiple blocks", () => { @@ -34,7 +34,7 @@ describe("Test isEmptyColumn", () => { ]), ]); - expect(isEmptyColumn(column)).toBeFalsy(); + expect(isEmptyContainerChild(column)).toBeFalsy(); }); it("Block with children", () => { @@ -51,7 +51,7 @@ describe("Test isEmptyColumn", () => { ]), ]); - expect(isEmptyColumn(column)).toBeFalsy(); + expect(isEmptyContainerChild(column)).toBeFalsy(); }); it("Block with text", () => { @@ -65,7 +65,7 @@ describe("Test isEmptyColumn", () => { ]), ]); - expect(isEmptyColumn(column)).toBeFalsy(); + expect(isEmptyContainerChild(column)).toBeFalsy(); }); it("Non-text block", () => { @@ -77,11 +77,11 @@ describe("Test isEmptyColumn", () => { ]), ]); - expect(isEmptyColumn(column)).toBeFalsy(); + expect(isEmptyContainerChild(column)).toBeFalsy(); }); }); -describe("Test removeEmptyColumns", () => { +describe("Test removeEmptyChildren", () => { it("Start and end columns empty", () => { const editor = getEditor(); const schema = editor._tiptapEditor.schema; @@ -116,7 +116,7 @@ describe("Test removeEmptyColumns", () => { const tr = editor.prosemirrorState.tr; tr.replaceRangeWith(1, tr.doc.firstChild!.content.size, columnList); - removeEmptyColumns(tr, 1); + removeEmptyChildren(tr, 1); expect(tr.doc).toMatchSnapshot(); }); @@ -143,7 +143,7 @@ describe("Test removeEmptyColumns", () => { const tr = editor.prosemirrorState.tr; tr.replaceRangeWith(1, tr.doc.firstChild!.content.size, columnList); - removeEmptyColumns(tr, 1); + removeEmptyChildren(tr, 1); expect(tr.doc).toMatchSnapshot(); }); @@ -170,7 +170,7 @@ describe("Test removeEmptyColumns", () => { const tr = editor.prosemirrorState.tr; tr.replaceRangeWith(1, tr.doc.firstChild!.content.size, columnList); - removeEmptyColumns(tr, 1); + removeEmptyChildren(tr, 1); expect(tr.doc).toMatchSnapshot(); }); @@ -195,13 +195,13 @@ describe("Test removeEmptyColumns", () => { const tr = editor.prosemirrorState.tr; tr.replaceRangeWith(1, tr.doc.firstChild!.content.size, columnList); - removeEmptyColumns(tr, 1); + removeEmptyChildren(tr, 1); expect(tr.doc).toMatchSnapshot(); }); }); -describe("Test fixColumnList", () => { +describe("Test fixContainer", () => { it("First of two columns empty", () => { const editor = getEditor(); const schema = editor._tiptapEditor.schema; @@ -224,7 +224,7 @@ describe("Test fixColumnList", () => { const tr = editor.prosemirrorState.tr; tr.replaceRangeWith(1, tr.doc.firstChild!.content.size, columnList); - fixColumnList(tr, 1); + fixContainer(tr, 1); expect(tr.doc).toMatchSnapshot(); }); @@ -251,7 +251,7 @@ describe("Test fixColumnList", () => { const tr = editor.prosemirrorState.tr; tr.replaceRangeWith(1, tr.doc.firstChild!.content.size, columnList); - fixColumnList(tr, 1); + fixContainer(tr, 1); expect(tr.doc).toMatchSnapshot(); }); @@ -276,7 +276,7 @@ describe("Test fixColumnList", () => { const tr = editor.prosemirrorState.tr; tr.replaceRangeWith(1, tr.doc.firstChild!.content.size, columnList); - fixColumnList(tr, 1); + fixContainer(tr, 1); expect(tr.doc).toMatchSnapshot(); }); diff --git a/packages/xl-multi-column/src/test/conversions/__snapshots__/multi-column/undefined/external.html b/packages/xl-multi-column/src/test/conversions/__snapshots__/multi-column/undefined/external.html index 2237513b6b..72b0f2d7ab 100644 --- a/packages/xl-multi-column/src/test/conversions/__snapshots__/multi-column/undefined/external.html +++ b/packages/xl-multi-column/src/test/conversions/__snapshots__/multi-column/undefined/external.html @@ -1 +1 @@ -

Column Paragraph 0

Column Paragraph 1

Column Paragraph 2

Column Paragraph 3

\ No newline at end of file +

Column Paragraph 0

Column Paragraph 1

Column Paragraph 2

Column Paragraph 3

\ No newline at end of file diff --git a/packages/xl-multi-column/src/test/conversions/__snapshots__/multi-column/undefined/internal.html b/packages/xl-multi-column/src/test/conversions/__snapshots__/multi-column/undefined/internal.html index 5876b3bd03..0d6612056e 100644 --- a/packages/xl-multi-column/src/test/conversions/__snapshots__/multi-column/undefined/internal.html +++ b/packages/xl-multi-column/src/test/conversions/__snapshots__/multi-column/undefined/internal.html @@ -1 +1 @@ -

Column Paragraph 0

Column Paragraph 1

Column Paragraph 2

Column Paragraph 3

\ No newline at end of file +

Column Paragraph 0

Column Paragraph 1

Column Paragraph 2

Column Paragraph 3

\ No newline at end of file diff --git a/packages/xl-multi-column/src/test/extensions/columnResize.test.ts b/packages/xl-multi-column/src/test/extensions/columnResize.test.ts new file mode 100644 index 0000000000..964a61de1a --- /dev/null +++ b/packages/xl-multi-column/src/test/extensions/columnResize.test.ts @@ -0,0 +1,101 @@ +import { getNodeById } from "@blocknote/core"; +import { describe, expect, it } from "vite-plus/test"; + +import { + ColumnState, + columnResizePluginKey, +} from "../../extensions/ColumnResize/ColumnResizeExtension.js"; +import { setupTestEnv } from "../setupTestEnv.js"; + +const getEditor = setupTestEnv(); + +// Puts the column resize plugin into the state it would be in when the user +// hovers the boundary between the two columns of "column-list-0" in the test +// document, as the plugin's mouse handlers would. +function hoverColumnBoundary() { + const editor = getEditor(); + const view = editor._tiptapEditor.view; + + const columnList = getNodeById("column-list-0", view.state.doc); + const leftColumn = getNodeById("column-0", view.state.doc); + const rightColumn = getNodeById("column-1", view.state.doc); + + if (!columnList || !leftColumn || !rightColumn) { + throw new Error("Test document is missing expected columns"); + } + + const hoverState: ColumnState = { + type: "hover-column", + columnList: { + element: document.createElement("div"), + id: "column-list-0", + ...columnList, + }, + leftColumn: { + element: document.createElement("div"), + id: "column-0", + ...leftColumn, + }, + rightColumn: { + element: document.createElement("div"), + id: "column-1", + ...rightColumn, + }, + }; + + view.dispatch(view.state.tr.setMeta(columnResizePluginKey, hoverState)); +} + +describe("Column resize plugin state after doc changes", () => { + it("falls back to default when a hovered column's removal unwraps the column list", () => { + const editor = getEditor(); + + hoverColumnBoundary(); + + // Removing one of the two columns brings the column list below its + // minimum of 2 children, so it gets unwrapped entirely. This used to + // throw a RangeError from the plugin's decorations, as they were built + // from positions resolved against the old, larger doc. + editor.removeBlocks(["column-1"]); + + expect( + columnResizePluginKey.getState(editor._tiptapEditor.view.state), + ).toEqual({ type: "default" }); + // The surviving column's two paragraphs are unwrapped to the top level. + expect(editor.document.map((block) => block.type)).toEqual([ + "paragraph", + "paragraph", + "paragraph", + "paragraph", + "paragraph", + ]); + }); + + it("falls back to default when the whole doc is replaced", () => { + const editor = getEditor(); + + hoverColumnBoundary(); + + // Mimics select-all + backspace clearing the document while columns are + // hovered. + editor.replaceBlocks(editor.document, [{ type: "paragraph" }]); + + expect( + columnResizePluginKey.getState(editor._tiptapEditor.view.state), + ).toEqual({ type: "default" }); + expect(editor.document).toHaveLength(1); + }); + + it("keeps the hover state when an unrelated block changes", () => { + const editor = getEditor(); + + hoverColumnBoundary(); + + editor.updateBlock("paragraph-1", { content: "Updated Paragraph 1" }); + + const pluginState = columnResizePluginKey.getState( + editor._tiptapEditor.view.state, + ); + expect(pluginState?.type).toBe("hover-column"); + }); +}); diff --git a/tests/src/end-to-end/multicolumn/multicolumn.test.tsx b/tests/src/end-to-end/multicolumn/multicolumn.test.tsx index 77cd1cd21d..97116684d2 100644 --- a/tests/src/end-to-end/multicolumn/multicolumn.test.tsx +++ b/tests/src/end-to-end/multicolumn/multicolumn.test.tsx @@ -11,6 +11,7 @@ import { import { compareDocToSnapshot, focusOnEditor, + sleep, waitForSelector, } from "../../utils/editor.js"; import { @@ -134,3 +135,53 @@ describe("Check Multi-Column Behaviour", () => { await compareDocToSnapshot("deleteEndOfColumnList"); }); }); + +// Which block the side menu attaches to is resolved from live layout +// (`elementsFromPoint` / `posAtCoords`); the geometry pieces below that are +// unit-tested in `packages/core/src/extensions/SideMenu/ +// sideMenuContainerGeometry.browser.test.ts`. This tests the whole path, +// through a real column list. Hovering a column's left padding hands the +// lookup coordinates that horizontally overlap the previous column, and +// `SideMenu.ts` only resolves the right block by re-probing further right +// once `isHorizontalContainer` recognises the column list. If that +// compensation (or the detection) breaks, the menu attaches to a block in +// the previous column. +describe("Check side menu placement inside a column list", () => { + /** Vertical centre of a rect, which the menu lines itself up with. */ + const centerY = (rect: DOMRect) => rect.y + rect.height / 2; + + test("Check drag handle resolves the block on the hovered row of a column", async () => { + await focusOnEditor(); + + // The last column is the only one holding several blocks, so it's the only + // place a wrongly resolved block is distinguishable by its row. + const target = page.getByText("Block 2").element(); + const columnRect = getRect(target.closest(".bn-block-column")!); + + await mouseSequence([ + { + type: "move", + x: columnRect.x + 5, + y: centerY(getRect(target)), + steps: 5, + }, + ]); + await waitForSelector(DRAG_HANDLE_SELECTOR); + await sleep(150); + const handleRect = getRect(DRAG_HANDLE_SELECTOR); + + expect(handleRect.x).toBeLessThan(getRect(target).x); + + // The handle lines up with the hovered block's row rather than any other + // block's. This is a stronger check than a pixel tolerance, since every + // candidate is only a line-height away, and it is what distinguishes + // this column's blocks from the neighbouring column's. + const distance = (rect: DOMRect) => + Math.abs(centerY(handleRect) - centerY(rect)); + for (const other of ["Block 1", "Block 3", "So is this heading!"]) { + expect(distance(getRect(target))).toBeLessThan( + distance(getRect(page.getByText(other).element())), + ); + } + }); +});