diff --git a/packages/core/src/extensions/TableHandles/TableHandles.browser.test.ts b/packages/core/src/extensions/TableHandles/TableHandles.browser.test.ts new file mode 100644 index 0000000000..30c7549284 --- /dev/null +++ b/packages/core/src/extensions/TableHandles/TableHandles.browser.test.ts @@ -0,0 +1,305 @@ +import { afterEach, beforeEach, describe, expect, it } from "vite-plus/test"; + +import { BlockNoteSchema } from "../../blocks/BlockNoteSchema.js"; +import { defaultBlockSpecs } from "../../blocks/defaultBlocks.js"; +import { BlockNoteEditor } from "../../editor/BlockNoteEditor.js"; +import { createBlockSpec } from "../../schema/index.js"; +import { TableHandlesExtension } from "./TableHandles.js"; + +// Unit tests for the mouse handling in `TableHandlesView`, which decides +// whether the row/column handles should be attached to the cell under the +// cursor. It reads the DOM (`getBoundingClientRect`, node views, hit testing), +// so it runs in the browser suite rather than as a node unit test. + +// A custom block which renders a real `` of its own, without having any +// table content in the document. Table cells like these are indistinguishable +// from a table block's cells when only looking at the DOM. +const fakeTableBlock = createBlockSpec( + { + type: "fakeTable", + propSchema: {}, + content: "none", + }, + { + render: () => { + const dom = document.createElement("div"); + dom.className = "fake-table"; + dom.innerHTML = + "
Not a real cell
"; + + return { dom }; + }, + }, +); + +// A custom block which embeds a second editor, itself containing a table. The +// nested editor's cells live inside the outer editor's DOM, so they reach the +// outer editor's mouse handlers, but its block IDs are unknown to the outer +// document. +const nestedEditorBlock = createBlockSpec( + { + type: "nestedEditor", + propSchema: {}, + content: "none", + }, + { + render: () => { + const dom = document.createElement("div"); + dom.className = "nested-editor"; + dom.contentEditable = "false"; + + const nestedEditor = BlockNoteEditor.create({ + initialContent: [ + { + id: "nested-table", + type: "table", + content: { + type: "tableContent", + rows: [{ cells: ["Nested cell"] }], + }, + }, + ], + }); + nestedEditor.mount(dom, { portalTarget: document.body }); + + return { dom, destroy: () => nestedEditor.unmount() }; + }, + }, +); + +const schema = BlockNoteSchema.create({ + blockSpecs: { + ...defaultBlockSpecs, + fakeTable: fakeTableBlock(), + nestedEditor: nestedEditorBlock(), + }, +}); + +describe("TableHandlesView mouse handling", () => { + let editor: BlockNoteEditor; + let mountPoint: HTMLElement; + + beforeEach(() => { + mountPoint = document.createElement("div"); + document.body.appendChild(mountPoint); + + editor = BlockNoteEditor.create({ + schema, + initialContent: [ + { + id: "table-0", + type: "table", + content: { + type: "tableContent", + rows: [ + { cells: ["Cell 1", "Cell 2", "Cell 3"] }, + { cells: ["Cell 4", "Cell 5", "Cell 6"] }, + ], + }, + }, + { id: "paragraph-0", type: "paragraph", content: "Paragraph" }, + { id: "fake-table-0", type: "fakeTable" }, + { id: "nested-editor-0", type: "nestedEditor" }, + ], + }) as BlockNoteEditor; + editor.mount(mountPoint); + }); + + afterEach(() => { + editor.unmount(); + editor._tiptapEditor.destroy(); + mountPoint.remove(); + }); + + function tableHandlesState() { + return editor.getExtension(TableHandlesExtension)!.store.state; + } + + function queryElement(selector: string) { + const element = mountPoint.querySelector(selector); + if (!element) { + throw new Error(`No element matching "${selector}"`); + } + return element; + } + + // The cell in the (only) real table block, at the given row and column. + function tableCell(rowIndex: number, colIndex: number) { + const cell = queryElement( + `[data-id="table-0"] tbody tr:nth-child(${rowIndex + 1})`, + ).children[colIndex]; + if (!(cell instanceof HTMLElement)) { + throw new Error(`No cell at row ${rowIndex}, column ${colIndex}`); + } + return cell; + } + + function dispatchMouseEvent( + type: "mousemove" | "mousedown", + el: HTMLElement, + ) { + const rect = el.getBoundingClientRect(); + el.dispatchEvent( + new MouseEvent(type, { + bubbles: true, + clientX: rect.left + rect.width / 2, + clientY: rect.top + rect.height / 2, + }), + ); + } + + // Moves the mouse over `el` and asserts the handler survived it. An event + // listener that throws doesn't propagate the error to `dispatchEvent`'s + // caller - the browser reports it as an `error` event on `window` instead - + // so the error has to be picked up from there to be visible to the test. + function moveMouseOver(el: HTMLElement) { + const errors: unknown[] = []; + const collectError = (event: ErrorEvent) => { + errors.push(event.error ?? event.message); + }; + + window.addEventListener("error", collectError); + try { + dispatchMouseEvent("mousemove", el); + } finally { + window.removeEventListener("error", collectError); + } + + expect(errors).toEqual([]); + } + + it("attaches the handles to the hovered cell", () => { + moveMouseOver(tableCell(1, 1)); + + expect(tableHandlesState()).toMatchObject({ + show: true, + rowIndex: 1, + colIndex: 1, + block: { id: "table-0" }, + }); + }); + + it("only offers the add/remove buttons on the last row and column", () => { + moveMouseOver(tableCell(0, 0)); + + expect(tableHandlesState()).toMatchObject({ + showAddOrRemoveRowsButton: false, + showAddOrRemoveColumnsButton: false, + }); + + moveMouseOver(tableCell(1, 2)); + + expect(tableHandlesState()).toMatchObject({ + showAddOrRemoveRowsButton: true, + showAddOrRemoveColumnsButton: true, + }); + }); + + it("hides the handles when the mouse leaves the table", () => { + moveMouseOver(tableCell(0, 0)); + expect(tableHandlesState()?.show).toBe(true); + + moveMouseOver(queryElement('[data-id="paragraph-0"]')); + + expect(tableHandlesState()?.show).toBe(false); + }); + + it("hides the handles while text is being selected with the mouse", () => { + moveMouseOver(tableCell(0, 0)); + expect(tableHandlesState()?.show).toBe(true); + + dispatchMouseEvent("mousedown", tableCell(0, 0)); + moveMouseOver(tableCell(0, 1)); + + expect(tableHandlesState()?.show).toBe(false); + }); + + it("keeps the handles on the same cell while frozen", () => { + moveMouseOver(tableCell(0, 0)); + editor.getExtension(TableHandlesExtension)!.freezeHandles(); + + moveMouseOver(tableCell(1, 2)); + + expect(tableHandlesState()).toMatchObject({ + show: true, + rowIndex: 0, + colIndex: 0, + }); + + editor.getExtension(TableHandlesExtension)!.unfreezeHandles(); + moveMouseOver(tableCell(1, 2)); + + expect(tableHandlesState()).toMatchObject({ + show: true, + rowIndex: 1, + colIndex: 2, + }); + }); + + it("does not show the handles when the editor is not editable", () => { + editor.isEditable = false; + + moveMouseOver(tableCell(0, 0)); + + expect(tableHandlesState()?.show).toBeFalsy(); + }); + + // Regression test for https://github.com/TypeCellOS/BlockNote/issues/2964. + // The hovered block used to be treated as a table as soon as the schema had + // a table block in it, so a custom block rendering a `` ended up on + // the table path and crashed on its (absent) table content. + it("ignores cells of a custom block that renders its own table", () => { + moveMouseOver(queryElement(".fake-table td")); + + expect(tableHandlesState()?.show).toBeFalsy(); + }); + + it("hides the handles when moving from a table onto a custom block's table", () => { + moveMouseOver(tableCell(0, 0)); + expect(tableHandlesState()?.show).toBe(true); + + moveMouseOver(queryElement(".fake-table td")); + + expect(tableHandlesState()?.show).toBe(false); + }); + + // The other half of #2964: the hovered cell belongs to a nested editor, so + // its block ID cannot be resolved in the outer editor's document. That used + // to throw `Block with ID not found` on every mouse move. + it("ignores table cells belonging to a nested editor", () => { + moveMouseOver(queryElement(".nested-editor td")); + + expect(tableHandlesState()?.show).toBeFalsy(); + }); + + it("hides the handles when moving from a table onto a nested editor's table", () => { + moveMouseOver(tableCell(0, 0)); + expect(tableHandlesState()?.show).toBe(true); + + moveMouseOver(queryElement(".nested-editor td")); + + expect(tableHandlesState()?.show).toBe(false); + }); + + // The table's position is captured when the handles attach to a cell, but + // the table shifts whenever content before it changes - a collaborator or an + // extension editing while a handle menu sits open, say. Acting on the handles + // afterwards used to resolve the stale position, landing in the wrong node. + it("acts on the hovered table after content is inserted before it", () => { + moveMouseOver(tableCell(0, 0)); + + editor.insertBlocks( + [{ type: "paragraph", content: "Inserted" }], + "table-0", + "before", + ); + + editor + .getExtension(TableHandlesExtension)! + .addRowOrColumn(0, { orientation: "row", side: "below" }); + + expect( + mountPoint.querySelectorAll('[data-id="table-0"] tbody tr'), + ).toHaveLength(3); + }); +}); diff --git a/packages/core/src/extensions/TableHandles/TableHandles.ts b/packages/core/src/extensions/TableHandles/TableHandles.ts index 25d09380f1..bb396fbdd7 100644 --- a/packages/core/src/extensions/TableHandles/TableHandles.ts +++ b/packages/core/src/extensions/TableHandles/TableHandles.ts @@ -1,3 +1,4 @@ +import type { Node } from "prosemirror-model"; import { EditorState, Plugin, PluginKey, PluginView } from "prosemirror-state"; import { CellSelection, @@ -27,7 +28,7 @@ import { import { nodeToBlock } from "../../api/nodeConversions/nodeToBlock.js"; import { getNodeById } from "../../api/nodeUtil.js"; import { - editorHasBlockWithType, + blockHasType, isTableCellNode, isTableCellSelection, } from "../../blocks/defaultBlockTypeGuards.js"; @@ -188,6 +189,17 @@ export class TableHandlesView implements PluginView { this.mouseState = "down"; }; + // Hides the handles if they're currently shown. Used whenever the mouse moves + // somewhere that shouldn't have table handles attached to it. + hideHandles = () => { + if (this.state?.show) { + this.state.show = false; + this.state.showAddOrRemoveRowsButton = false; + this.state.showAddOrRemoveColumnsButton = false; + this.emitUpdate(); + } + }; + mouseUpHandler = (event: MouseEvent) => { this.mouseState = "up"; this.mouseMoveHandler(event); @@ -219,22 +231,12 @@ export class TableHandlesView implements PluginView { // hide draghandles when selecting text as they could be in the way of the user this.mouseState = "selecting"; - if (this.state?.show) { - this.state.show = false; - this.state.showAddOrRemoveRowsButton = false; - this.state.showAddOrRemoveColumnsButton = false; - this.emitUpdate(); - } + this.hideHandles(); return; } if (!target || !this.editor.isEditable) { - if (this.state?.show) { - this.state.show = false; - this.state.showAddOrRemoveRowsButton = false; - this.state.showAddOrRemoveColumnsButton = false; - this.emitUpdate(); - } + this.hideHandles(); return; } @@ -248,38 +250,40 @@ export class TableHandlesView implements PluginView { if (!blockEl) { return; } - this.tableElement = blockEl.node; - - let tableBlock: - | BlockFromConfigNoChildren - | undefined; const { pmNodeInfo, doc } = this.editor.transact((tr) => ({ pmNodeInfo: getNodeById(blockEl.id, tr.doc), doc: tr.doc, })); + + // The hovered cell may belong to a document other than this editor's, as a + // custom block can embed a nested editor which itself contains a table. The + // nested editor's DOM is inside this view's DOM, so its cells still reach + // this handler, but its block IDs are unknown here. if (!pmNodeInfo) { - throw new Error(`Block with ID ${blockEl.id} not found`); + this.hideHandles(); + return; } - const block = nodeToBlock( - pmNodeInfo.node, - doc, - ) as unknown as BlockFromConfigNoChildren< + const block = nodeToBlock(pmNodeInfo.node, doc); + + // `domCellAround` matches any `TD`/`TH` in the DOM, so the hovered block may + // just as well be a custom block that renders a table of its own. Checking + // the block itself (rather than only whether the schema has a table block) + // keeps the handles off blocks that have no table content to work with. + if (!blockHasType(block, this.editor, "table")) { + this.hideHandles(); + return; + } + + const tableBlock = block as unknown as BlockFromConfigNoChildren< DefaultBlockSchema["table"], any, any >; - if (editorHasBlockWithType(this.editor, "table")) { - this.tablePos = pmNodeInfo.posBeforeNode + 1; - tableBlock = block; - } - - if (!tableBlock) { - return; - } - + this.tablePos = pmNodeInfo.posBeforeNode + 1; + this.tableElement = blockEl.node; this.tableId = blockEl.id; const widgetContainer = target.domNode .closest(".tableWrapper") @@ -527,6 +531,22 @@ export class TableHandlesView implements PluginView { return true; }; + + // Resolves the position just inside the hovered table, within `doc`. The + // position is looked up by ID on each use rather than read from a cached one, + // as anything changing before the table (a collaborator inserting a block + // above it, say) shifts it: a stale position resolves into the wrong node, or + // past the end of the document entirely. + getTablePos(doc: Node): number | undefined { + if (this.tableId === undefined) { + return undefined; + } + + const pmNodeInfo = getNodeById(this.tableId, doc); + + return pmNodeInfo && pmNodeInfo.posBeforeNode + 1; + } + // Updates drag handles when the table is modified or removed. update() { if (!this.state || !this.state.show) { @@ -544,6 +564,7 @@ export class TableHandlesView implements PluginView { ) { this.state = undefined; this.tableId = undefined; + this.tablePos = undefined; this.tableElement = undefined; this.emitUpdate(); @@ -551,6 +572,9 @@ export class TableHandlesView implements PluginView { } this.state.block = refreshedBlock as typeof this.state.block; + // The table may have shifted in this update, so re-resolve its position. + this.tablePos = this.getTablePos(this.pmView.state.doc); + const { height: rowCount, width: colCount } = getDimensionsOfTable( this.state.block, ); @@ -647,12 +671,21 @@ export const TableHandlesExtension = createExtension(({ editor }) => { if ( view === undefined || view.state === undefined || - view.state.draggingState === undefined || - view.tablePos === undefined + view.state.draggingState === undefined ) { return; } + // Resolved against the state being rendered rather than read from + // `view.tablePos`, as ProseMirror computes decorations for a + // transaction before it calls the plugin view's `update` - so the + // cached position is still the pre-transaction one here. + const tablePos = view.getTablePos(state.doc); + + if (tablePos === undefined) { + return; + } + const newIndex = view.state.draggingState.draggedCellOrientation === "row" ? view.state.rowIndex @@ -683,7 +716,7 @@ export const TableHandlesExtension = createExtension(({ editor }) => { } // Gets the table to show the drop cursor in. - const tableResolvedPos = state.doc.resolve(view.tablePos + 1); + const tableResolvedPos = state.doc.resolve(tablePos + 1); if (view.state.draggingState.draggedCellOrientation === "row") { const cellsInRow = getCellsAtRowHandle( @@ -916,11 +949,8 @@ export const TableHandlesExtension = createExtension(({ editor }) => { * interfering with open submenus. */ hideHandlesIfNotFrozen() { - if (!view!.menuFrozen && view!.state?.show) { - view!.state.show = false; - view!.state.showAddOrRemoveRowsButton = false; - view!.state.showAddOrRemoveColumnsButton = false; - view!.emitUpdate(); + if (!view!.menuFrozen) { + view!.hideHandles(); } }, @@ -954,7 +984,18 @@ export const TableHandlesExtension = createExtension(({ editor }) => { throw new Error("Table handles view not initialized"); } - const tableResolvedPos = state.doc.resolve(view.tablePos! + 1); + // Resolved against `state`, which may be several transactions ahead of + // the mouse move that attached the handles (a menu can stay open across + // edits from elsewhere, e.g. a collaborator). + const tablePos = view.getTablePos(state.doc); + + if (tablePos === undefined) { + throw new Error( + "Attempted to select table cells, but the hovered table is no longer in the document.", + ); + } + + const tableResolvedPos = state.doc.resolve(tablePos + 1); const startRowResolvedPos = state.doc.resolve( tableResolvedPos.posAtIndex(relativeStartCell.row) + 1, ); diff --git a/packages/core/vite.config.ts b/packages/core/vite.config.ts index eca8efa6a5..2763b9723c 100644 --- a/packages/core/vite.config.ts +++ b/packages/core/vite.config.ts @@ -1,6 +1,6 @@ import * as path from "path"; import { webpackStats } from "rollup-plugin-webpack-stats"; -import { defineConfig } from "vite-plus"; +import { configDefaults, defineConfig } from "vite-plus"; import pkg from "./package.json"; // import eslintPlugin from "vite-plugin-eslint"; @@ -21,6 +21,9 @@ export default defineConfig({ test: { environment: "jsdom", setupFiles: ["./vitestSetup.ts"], + // `.browser.test` files need a real browser; the tests package's browser + // suite runs them. + exclude: [...configDefaults.exclude, "**/*.browser.test.*"], }, plugins: [webpackStats()], build: {