Skip to content

Commit ceea356

Browse files
committed
Address review: resolve blocks with less DOM work, pick drop editors within dropMargin
- blockDOM.ts moves into SideMenu/ and holds the DOM-structure helpers; getBlockFromElement becomes getBlockElement, and the second lookup after the descent is gone. Tests for it are co-located. - findDropEditor only considers editors within dropMargin, then picks the one with the nearest blocks. - resolveHoveredBlock takes isWithinEditorUI and hoverMargin as parameters. - Comments: safePolygon note, SideMenuState.block, update() branches, FIXME on resolving blocks through ProseMirror. - Remove the unused SideMenuState duplicate in dragging.ts.
1 parent 4f45a56 commit ceea356

10 files changed

Lines changed: 220 additions & 148 deletions

‎packages/core/src/extensions/SideMenu/SideMenu.ts‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,12 @@ export type SideMenuState<
2424
I extends InlineContentSchema,
2525
S extends StyleSchema,
2626
> = UiElementPosition & {
27-
// The block that the side menu is attached to.
27+
/**
28+
* The block that the side menu is attached to. While the menu is hidden
29+
* (`show` is `false`), the block it was last attached to, so it can animate
30+
* out next to it. The store's state is `undefined` until the menu first
31+
* shows.
32+
*/
2833
block: Block<BSchema, I, S>;
2934
};
3035

@@ -150,6 +155,7 @@ export const SideMenuExtension = createExtension(
150155
// removes the drag image) would never fire. The drop's document change
151156
// otherwise re-resolves the block at the pointer's pre-drag position.
152157
if (menuFrozen || isDragOrigin) {
158+
// Frozen or dragging: keep the menu as it is.
153159
return;
154160
}
155161
if (target && menuElement?.contains(target)) {
@@ -159,25 +165,26 @@ export const SideMenuExtension = createExtension(
159165
// anywhere but onto its block follows the pointer at once.
160166
clearIntent();
161167
lastOnActive = point;
168+
// On the menu: keep the menu as it is.
162169
return;
163170
}
164171

165172
if (!editor.isEditable) {
166173
clearIntent();
167174
hideMenu();
175+
// Read-only editor: hide the menu.
168176
return;
169177
}
170178

171179
const resolution = resolveHoveredBlock(
172180
editor.prosemirrorView,
173181
target,
174182
point,
175-
{
176-
isWithinEditorUI: editor.isWithinEditor,
177-
hoverMargin,
178-
},
183+
editor.isWithinEditor,
184+
hoverMargin,
179185
);
180186
if (resolution.kind === "editorUI") {
187+
// On other UI of the editor (e.g. a toolbar): keep the menu as it is.
181188
return;
182189
}
183190

@@ -186,6 +193,7 @@ export const SideMenuExtension = createExtension(
186193
if (resolution.kind === "block" && resolution.block.id === active) {
187194
clearIntent();
188195
lastOnActive = point;
196+
// On the block of the menu: keep the menu as it is.
189197
return;
190198
}
191199

@@ -195,10 +203,13 @@ export const SideMenuExtension = createExtension(
195203
// Restarts the timeout, so it measures how long the pointer rests.
196204
clearIntent();
197205
intent = { exit, timer: setTimeout(releaseIntent, INTENT_TIMEOUT) };
206+
// On the way to the menu: keep the menu on its block for now.
198207
return;
199208
}
200209
}
201210

211+
// Not on the way to the menu: show the menu for the block under the
212+
// pointer, or hide it if there is no block.
202213
clearIntent();
203214
if (resolution.kind === "block") {
204215
lastOnActive = point;
Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
import { afterEach, describe, expect, it } from "vite-plus/test";
2+
3+
import { getBlockElement, getDirectChildBlocks } from "./blockDOM.js";
4+
5+
let root: HTMLElement | undefined;
6+
7+
afterEach(() => {
8+
root?.remove();
9+
root = undefined;
10+
});
11+
12+
/** Attaches the markup to the document and returns a lookup by `data-id`. */
13+
function mount(html: string) {
14+
root = document.createElement("div");
15+
root.innerHTML = html;
16+
document.body.append(root);
17+
return (id: string) =>
18+
root!.querySelector(
19+
`[data-id="${id}"]:not([data-node-type="blockOuter"])`,
20+
)!;
21+
}
22+
23+
// A column list with one block per column, the way BlockNote renders it: each
24+
// regular block is a `blockContainer` wrapped in `blockOuter` chrome.
25+
const columns = `
26+
<div class="bn-editor">
27+
<div data-node-type="columnList" data-id="list">
28+
<div data-node-type="column" data-id="col-a">
29+
<div data-node-type="blockOuter" data-id="a">
30+
<div data-node-type="blockContainer" data-id="a"><span>A</span></div>
31+
</div>
32+
</div>
33+
<div data-node-type="column" data-id="col-b">
34+
<div data-node-type="blockOuter" data-id="b">
35+
<div data-node-type="blockContainer" data-id="b">B</div>
36+
</div>
37+
</div>
38+
</div>
39+
</div>`;
40+
41+
// A block that embeds another editor, with a block of its own.
42+
const nestedEditor = `
43+
<div class="bn-editor">
44+
<div data-node-type="blockOuter" data-id="outer">
45+
<div data-node-type="blockContainer" data-id="outer">
46+
<div class="bn-editor" data-test="inner">
47+
<div data-node-type="blockOuter" data-id="inner">
48+
<div data-node-type="blockContainer" data-id="inner">inner</div>
49+
</div>
50+
</div>
51+
</div>
52+
</div>
53+
</div>`;
54+
55+
describe("getDirectChildBlocks", () => {
56+
it("returns direct child blocks, skipping nested grandchildren", () => {
57+
const byId = mount(columns);
58+
// The blocks inside each column must not come back as the list's own
59+
// children.
60+
expect(getDirectChildBlocks(byId("list"))).toEqual([
61+
byId("col-a"),
62+
byId("col-b"),
63+
]);
64+
});
65+
66+
it("sees through blockOuter wrappers to the blockContainer child", () => {
67+
const byId = mount(columns);
68+
expect(getDirectChildBlocks(byId("col-a"))).toEqual([byId("a")]);
69+
});
70+
71+
it("skips the blocks of an editor nested in the block", () => {
72+
const byId = mount(nestedEditor);
73+
expect(getDirectChildBlocks(byId("outer"))).toEqual([]);
74+
});
75+
});
76+
77+
describe("getBlockElement", () => {
78+
it("returns the block an element is in", () => {
79+
const byId = mount(columns);
80+
const editor = root!.querySelector<HTMLElement>(".bn-editor")!;
81+
const text = byId("a").querySelector("span")!;
82+
expect(getBlockElement(text, { dom: editor })).toBe(byId("a"));
83+
expect(getBlockElement(byId("col-a"), { dom: editor })).toBe(byId("col-a"));
84+
});
85+
86+
it("returns nothing for the editor element itself", () => {
87+
mount(columns);
88+
const editor = root!.querySelector<HTMLElement>(".bn-editor")!;
89+
expect(getBlockElement(editor, { dom: editor })).toBeUndefined();
90+
});
91+
92+
it("leaves the blocks of a nested editor to that editor", () => {
93+
const byId = mount(nestedEditor);
94+
const outer = root!.querySelector<HTMLElement>(".bn-editor")!;
95+
const inner = root!.querySelector<HTMLElement>('[data-test="inner"]')!;
96+
expect(getBlockElement(byId("inner"), { dom: outer })).toBeUndefined();
97+
expect(getBlockElement(byId("inner"), { dom: inner })).toBe(byId("inner"));
98+
// The inner editor's own padding is in the outer block, which belongs to
99+
// the outer editor.
100+
expect(getBlockElement(inner, { dom: inner })).toBeUndefined();
101+
});
102+
});
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
import type { EditorView } from "prosemirror-view";
2+
3+
// FIXME: this file finds blocks through their rendered DOM (selectors,
4+
// `closest`). Resolving through ProseMirror instead would drop the selectors:
5+
// `view.posAtDOM` for the block an element is in, the document for a block's
6+
// children, and `view.nodeDOM` for their boxes. Check `posAtDOM` inside node
7+
// views and nested editors first. The same applies to
8+
// `getDraggableBlockFromElement`.
9+
10+
// UniqueID and the block renderers put data-id on every block root.
11+
// Ordinary blocks also copy it to blockOuter, which is only rendering chrome.
12+
export const BLOCK_SELECTOR =
13+
'[data-node-type][data-id]:not([data-node-type="blockOuter"])';
14+
15+
/**
16+
* Returns the root element of the block of this editor that the element is
17+
* in, if there is one.
18+
*/
19+
export function getBlockElement(
20+
element: Element,
21+
view: Pick<EditorView, "dom">,
22+
): Element | undefined {
23+
// Embedded editors own their blocks, even though their DOM is inside ours.
24+
if (element.closest(".bn-editor") !== view.dom) {
25+
return undefined;
26+
}
27+
const block = element.closest(BLOCK_SELECTOR);
28+
return block && block !== view.dom && view.dom.contains(block)
29+
? block
30+
: undefined;
31+
}
32+
33+
/**
34+
* Returns the child blocks of a block element: its nested blocks, or the
35+
* children of a container. The blocks of an editor nested in the block are
36+
* not its children: they belong to that editor.
37+
*/
38+
export function getDirectChildBlocks(container: Element): Element[] {
39+
const editor = container.closest(".bn-editor");
40+
const children: Element[] = [];
41+
for (const child of container.querySelectorAll(BLOCK_SELECTOR)) {
42+
if (
43+
child.parentElement?.closest(BLOCK_SELECTOR) === container &&
44+
child.closest(".bn-editor") === editor
45+
) {
46+
children.push(child);
47+
}
48+
}
49+
return children;
50+
}

‎packages/core/src/extensions/SideMenu/blockDropHandling.ts‎

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -15,19 +15,29 @@ function isSynthetic(event: Event): boolean {
1515
* Finds the editor that a drag event is about, and the distance from the
1616
* event to the blocks of that editor. If the event target is in an editor,
1717
* that editor is the only candidate: where editors overlap, it is the editor
18-
* on top. Otherwise, it is the editor closest to the event.
18+
* on top. Otherwise, the candidates are the editors within `dropMargin` of the
19+
* event, and it is the one closest to the event.
1920
*
20-
* Distances count to the blocks, not to the editor element (as `dropMargin`
21-
* does): the drop lands at the nearest block position, so with uneven editor
22-
* padding, the editor whose blocks are nearest is the right pick. A drop
23-
* outside the blocks, even in the editor's padding, is moved onto them.
21+
* `dropMargin` counts from the editor element, like `hoverMargin`. The
22+
* distance that picks the closest editor counts to its blocks: the drop lands
23+
* at the nearest block position, so with uneven editor padding, the editor
24+
* whose blocks are nearest is the right pick. A drop outside the blocks, even
25+
* in the editor's padding, is moved onto them.
2426
*/
25-
function findDropEditor(view: EditorView, event: DragEvent) {
27+
function findDropEditor(
28+
view: EditorView,
29+
event: DragEvent,
30+
dropMargin: number,
31+
) {
32+
const point = { x: event.clientX, y: event.clientY };
2633
const target =
2734
event.target instanceof Element ? event.target.closest(".bn-editor") : null;
2835
const editors = target
2936
? [target]
30-
: Array.from(view.root.querySelectorAll(".bn-editor"));
37+
: Array.from(view.root.querySelectorAll(".bn-editor")).filter(
38+
(editor) =>
39+
distanceToRect(point, editor.getBoundingClientRect()) <= dropMargin,
40+
);
3141
const coords = event;
3242

3343
if (editors.length === 0) {
@@ -148,17 +158,9 @@ export function registerBlockDropHandlers(
148158
// Tells us that the current editor instance has a drag ongoing (either text or side menu)
149159
const isOrigin = textContentIsBeingDragged || sideMenuIsBeingDragged;
150160

151-
// Tells us which editor instance is the closest to the drag event (whether or not it is actually reasonably close)
152-
const closestEditor = findDropEditor(view, event);
153-
154-
// `dropMargin` counts from the editor element's box, like `hoverMargin`.
155-
if (
156-
!closestEditor ||
157-
distanceToRect(
158-
{ x: event.clientX, y: event.clientY },
159-
closestEditor.element.getBoundingClientRect(),
160-
) > dropMargin
161-
) {
161+
// The editor the drag event is about, if any is within `dropMargin`
162+
const closestEditor = findDropEditor(view, event, dropMargin);
163+
if (!closestEditor) {
162164
return undefined;
163165
}
164166

‎packages/core/src/extensions/SideMenu/blockGeometry.browser.test.ts‎

Lines changed: 1 addition & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,6 @@
11
import { afterEach, describe, expect, it } from "vite-plus/test";
22

3-
import {
4-
getNestedBlockAtCursor,
5-
getDirectChildBlocks,
6-
} from "./blockGeometry.js";
3+
import { getNestedBlockAtCursor } from "./blockGeometry.js";
74

85
// Exercise container hit testing with real layout, including nested columns.
96

@@ -79,24 +76,6 @@ function buildVerticalContainer() {
7976
return { callout, first, second };
8077
}
8178

82-
describe("getDirectChildBlocks", () => {
83-
it("returns direct child blocks, skipping nested grandchildren", () => {
84-
const { columnList, columnA, columnB } = buildColumnList();
85-
86-
// The blocks inside each column must not come back as the list's own
87-
// children. The `closest` check stops the walk one level down.
88-
expect(getDirectChildBlocks(columnList)).toEqual([columnA, columnB]);
89-
});
90-
91-
it("sees through blockOuter wrappers to the blockContainer child", () => {
92-
const { columnA, childA } = buildColumnList();
93-
94-
// The column's own direct child is the wrapped blockContainer, not the
95-
// blockOuter chrome (which isn't a block in the selector's sense).
96-
expect(getDirectChildBlocks(columnA)).toEqual([childA.blockContainer]);
97-
});
98-
});
99-
10079
describe("getNestedBlockAtCursor", () => {
10180
it("keeps a regular block as the probe target", () => {
10281
const { childA } = buildColumnList();

‎packages/core/src/extensions/SideMenu/blockGeometry.ts‎

Lines changed: 9 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,4 @@
1-
import { BLOCK_SELECTOR } from "../blockDOM.js";
2-
3-
/**
4-
* Returns the child blocks of a block element: its nested blocks, or the
5-
* children of a container. The blocks of an editor nested in the block are
6-
* not its children: they belong to that editor.
7-
*/
8-
export function getDirectChildBlocks(container: Element): Element[] {
9-
const editor = container.closest(".bn-editor");
10-
const children: Element[] = [];
11-
for (const child of container.querySelectorAll(BLOCK_SELECTOR)) {
12-
if (
13-
child.parentElement?.closest(BLOCK_SELECTOR) === container &&
14-
child.closest(".bn-editor") === editor
15-
) {
16-
children.push(child);
17-
}
18-
}
19-
return children;
20-
}
1+
import { BLOCK_SELECTOR, getDirectChildBlocks } from "./blockDOM.js";
212

223
export type BlockRect = {
234
top: number;
@@ -45,7 +26,14 @@ export function rectIndexAtCursor(
4526
return verticalMatch;
4627
}
4728

48-
// Descend through child blocks, including regular blocks with padded frames.
29+
/**
30+
* Returns the deepest block at the height of the pointer, from a block that
31+
* contains it: the block itself, or one of its descendants at that height.
32+
* The children of a block are below its own content (nested blocks) or fill
33+
* it (a container), so the pointer on the block's own content stays on the
34+
* block. The pointer in the indentation of nested blocks, or in the padding
35+
* of a container beside a child, goes to the child at that height.
36+
*/
4937
export function getNestedBlockAtCursor(
5038
element: Element,
5139
mousePos: { x: number; y: number },

‎packages/core/src/extensions/SideMenu/dragging.ts‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import { fragmentToBlocks } from "../../api/nodeConversions/fragmentToBlocks.js"
88
import { getNodeById } from "../../api/nodeUtil.js";
99
import { Block } from "../../blocks/defaultBlocks.js";
1010
import type { BlockNoteEditor } from "../../editor/BlockNoteEditor.js";
11-
import { UiElementPosition } from "../../extensions-shared/UiElementPosition.js";
1211
import {
1312
BlockSchema,
1413
InlineContentSchema,
@@ -18,15 +17,6 @@ import { MultipleNodeSelection } from "./MultipleNodeSelection.js";
1817

1918
let dragImageElement: Element | undefined;
2019

21-
export type SideMenuState<
22-
BSchema extends BlockSchema,
23-
I extends InlineContentSchema,
24-
S extends StyleSchema,
25-
> = UiElementPosition & {
26-
// The block that the side menu is attached to.
27-
block: Block<BSchema, I, S>;
28-
};
29-
3020
function blockPositionsFromSelection(selection: Selection, doc: Node) {
3121
// Absolute positions just before the first block spanned by the selection, and just after the last block. Having the
3222
// selection start and end just before and just after the target blocks ensures no whitespace/line breaks are left

0 commit comments

Comments
 (0)