From c347c060651e61ce08a390f9cef2c384ce5df31d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Hamburger=20Gr=C3=B8ngaard?= Date: Thu, 3 Sep 2026 11:41:30 +0200 Subject: [PATCH 1/2] fix: stop clearing the drop position on `dragenter` The clearing behavior treated `drag.dragenter` as an end-of-interaction signal, but it is a carrier event: it precedes every `dragover` when the pointer crosses an element boundary. Each crossing therefore ran a clear+set cycle: the indicator unmounted and remounted, and the caret-color suppression on the editor root was restored and re-written, two inline style writes whose layout invalidation turns the next drag event's rect reads into forced reflows on large pages. Same mechanism class as the `drag.drag` clearing fixed before; `dragenter` was the remaining carrier in the guard. Exclude `drag.dragenter` from the clearing guard. `dragleave` fires on every genuine departure and still clears, so no stale indicator can survive leaving the editor. Pinned by a test sending a `dragenter` between two `dragover`s and asserting the drop position and the hidden drop caret survive it (red on the old guard: the clear restored the caret color synchronously). --- .changeset/dnd-dragenter-clear.md | 7 +++++ packages/plugin-dnd/src/plugin.dnd.test.tsx | 31 +++++++++++++++++++++ packages/plugin-dnd/src/plugin.dnd.tsx | 18 +++++++----- 3 files changed, 49 insertions(+), 7 deletions(-) create mode 100644 .changeset/dnd-dragenter-clear.md diff --git a/.changeset/dnd-dragenter-clear.md b/.changeset/dnd-dragenter-clear.md new file mode 100644 index 0000000000..a2792e1ffc --- /dev/null +++ b/.changeset/dnd-dragenter-clear.md @@ -0,0 +1,7 @@ +--- +'@portabletext/plugin-dnd': patch +--- + +fix: stop clearing the drop position on `dragenter` + +Crossing a block boundary during a drag no longer clears and re-sets the drop position. `dragenter` precedes every `dragover` on a crossing, so clearing on it toggled the indicator and wrote the editor's caret color twice per crossing; on large pages each style write costs a layout reflow. The caret color and the indicator now only change when the drop position genuinely transitions, and the drop position still clears on `dragstart`, `dragend`, `dragleave`, and `drop`. diff --git a/packages/plugin-dnd/src/plugin.dnd.test.tsx b/packages/plugin-dnd/src/plugin.dnd.test.tsx index af47dd06a7..d825cd5f3d 100644 --- a/packages/plugin-dnd/src/plugin.dnd.test.tsx +++ b/packages/plugin-dnd/src/plugin.dnd.test.tsx @@ -168,6 +168,37 @@ describe('DndProvider', () => { await expectDropPositions({b0: 'none', b1: 'none', b2: 'end'}) }) + test('Scenario: A `dragenter` on a boundary crossing does not clear the drop position', async () => { + const {editor} = await renderEditorWithProbes() + const editorElement = getEditorElement(editor) + + editor.send( + dragover({ + dragOrigin: blockSelection('b0'), + over: caretIn('b2'), + block: 'end', + }), + ) + await expectDropPositions({b2: 'end'}) + expect(editorElement.style.caretColor).toBe('transparent') + + editor.send({ + type: 'drag.dragenter', + originEvent: {dataTransfer: new DataTransfer()}, + position: { + block: 'end', + isEditor: false, + isContainer: false, + selection: caretIn('b1'), + }, + }) + + // The caret-color write happens synchronously in the store, so a clear + // would already have restored it here. + expect(editorElement.style.caretColor).toBe('transparent') + await expectDropPositions({b0: 'none', b1: 'none', b2: 'end'}) + }) + test('Scenario: Ending the drag clears the drop position', async () => { const {editor} = await renderEditorWithProbes() diff --git a/packages/plugin-dnd/src/plugin.dnd.tsx b/packages/plugin-dnd/src/plugin.dnd.tsx index a896182624..869503c3c4 100644 --- a/packages/plugin-dnd/src/plugin.dnd.tsx +++ b/packages/plugin-dnd/src/plugin.dnd.tsx @@ -276,13 +276,17 @@ function createDropPositionBehaviors( defineBehavior({ on: 'drag.*', guard: ({event}) => - // `drag.drag` fires continuously on the drag source between - // `dragover`s. Clearing on it would toggle the drop position (and - // the caret-color write on the editor root) on every pointer move, - // and each write invalidates layout for the whole page right before - // the next drag event reads element rects: a forced reflow per move, - // scaling with page size. - event.type !== 'drag.dragover' && event.type !== 'drag.drag', + // `drag.drag` (continuous on the drag source) and `drag.dragenter` + // (preceding every `dragover` on a boundary crossing) are carrier + // events, not end-of-interaction signals. Clearing on them would + // toggle the drop position (and the caret-color write on the editor + // root) per pointer move or per crossing, and each style write + // invalidates layout for the whole page right before the next drag + // event reads element rects: a forced reflow, scaling with page + // size. `dragleave` still clears every genuine departure. + event.type !== 'drag.dragover' && + event.type !== 'drag.drag' && + event.type !== 'drag.dragenter', actions: [ ({event}) => [ effect(() => { From ee1bb6275c1bef8c5818c4a66b89a61d5c33dd8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Hamburger=20Gr=C3=B8ngaard?= Date: Thu, 3 Sep 2026 11:41:30 +0200 Subject: [PATCH 2/2] fix: derive the dragged-block facts once per drag instead of per `dragover` The `dragover` guard re-derived drag-constant facts at pointer frequency: `getDragSelection`, `getSelectedBlocks`, a `getFocusBlock` resolution of the origin, and `isSelectingEntireBlocks`, each scanning the document, so the per-event cost grew with document size. Cache them in the plugin closure next to the drop-position store, keyed on the document value by identity (the value array is replaced on every applied operation) and on the drag origin by selection equality. Identity would be the cheaper origin key, but the origin object's identity does not survive the event pipeline, so an identity-keyed memo silently degrades to a per-event recompute; a probe counting recomputes caught exactly that. A mid-drag document change (a remote edit landing) invalidates through the value key, pinned by a test where deleting a character turns the same drag origin into an entire-blocks drag and the indicator must appear (red when the memo ignores the value). The guard's observable decisions are unchanged; the self-drop comparison now uses the cached key set and origin focus path instead of re-deriving both. --- .changeset/dnd-drag-constants-memo.md | 7 ++ packages/plugin-dnd/src/plugin.dnd.test.tsx | 45 ++++++++ packages/plugin-dnd/src/plugin.dnd.tsx | 112 ++++++++++++++------ 3 files changed, 129 insertions(+), 35 deletions(-) create mode 100644 .changeset/dnd-drag-constants-memo.md diff --git a/.changeset/dnd-drag-constants-memo.md b/.changeset/dnd-drag-constants-memo.md new file mode 100644 index 0000000000..7ef01be7a6 --- /dev/null +++ b/.changeset/dnd-drag-constants-memo.md @@ -0,0 +1,7 @@ +--- +'@portabletext/plugin-dnd': patch +--- + +fix: derive the dragged-block facts once per drag instead of per `dragover` + +The `dragover` guard no longer re-derives which blocks are being dragged (the drag selection, the dragged block keys, and the entire-blocks check, all of which scan the document) on every pointer move. These facts are now computed once per drag and reused, so dragging over large documents does less work per `dragover`. They are recomputed when the document changes mid-drag, for example when a remote edit lands, so the indicator keeps reflecting the current document. diff --git a/packages/plugin-dnd/src/plugin.dnd.test.tsx b/packages/plugin-dnd/src/plugin.dnd.test.tsx index d825cd5f3d..664f56c500 100644 --- a/packages/plugin-dnd/src/plugin.dnd.test.tsx +++ b/packages/plugin-dnd/src/plugin.dnd.test.tsx @@ -199,6 +199,51 @@ describe('DndProvider', () => { await expectDropPositions({b0: 'none', b1: 'none', b2: 'end'}) }) + test('Scenario: A mid-drag document change recomputes the derived drag state', async () => { + const {editor} = await renderEditorWithProbes() + + // The same origin selection is used across the whole drag, like the + // editor's own drag tracking does, so only the document change can + // invalidate the derived state. It covers `b0`'s text minus the last + // character, so the drag starts as a partial text drag. + const origin: NonNullable = { + anchor: {path: spanPath('b0'), offset: 0}, + focus: {path: spanPath('b0'), offset: 'first'.length - 1}, + } + + editor.send( + dragover({ + dragOrigin: origin, + over: caretIn('b2'), + block: 'end', + }), + ) + // A partial text drag shows nothing. + await expectDropPositions({b0: 'none', b1: 'none', b2: 'none'}) + + // Shrink `b0` so the same origin selection now spans the whole block: + // the drag becomes an entire-blocks drag. + editor.send({ + type: 'delete', + at: { + anchor: {path: spanPath('b0'), offset: 'first'.length - 1}, + focus: {path: spanPath('b0'), offset: 'first'.length}, + }, + }) + + editor.send( + dragover({ + dragOrigin: origin, + over: caretIn('b2'), + block: 'end', + }), + ) + + // Only a recomputed drag selection can show the indicator here; state + // cached against the pre-delete document keeps it hidden. + await expectDropPositions({b2: 'end'}) + }) + test('Scenario: Ending the drag clears the drop position', async () => { const {editor} = await renderEditorWithProbes() diff --git a/packages/plugin-dnd/src/plugin.dnd.tsx b/packages/plugin-dnd/src/plugin.dnd.tsx index 869503c3c4..9bc71ef38e 100644 --- a/packages/plugin-dnd/src/plugin.dnd.tsx +++ b/packages/plugin-dnd/src/plugin.dnd.tsx @@ -1,4 +1,9 @@ -import {useEditor, type Path} from '@portabletext/editor' +import { + useEditor, + type EditorSelection, + type EditorSnapshot, + type Path, +} from '@portabletext/editor' import { defineBehavior, effect, @@ -19,6 +24,7 @@ import { isEmptyTextBlock, isEqualPaths, isEqualSelectionPoints, + isEqualSelections, isKeyedSegment, isSelectionCollapsed, } from '@portabletext/editor/utils' @@ -153,9 +159,74 @@ function createDropPositionStore( * drag itself. (The editor's internal drop-position tracking gets away * without forwarding because it registers below core priority.) */ +type DragOriginState = { + value: EditorSnapshot['context']['value'] + dragOriginSelection: NonNullable + draggedBlockKeys: Set + draggedFocusBlockPath: Path | undefined + draggingEntireBlocks: boolean +} + function createDropPositionBehaviors( setDropPosition: (next: DropPosition | undefined) => void, ): Array { + // `dragover` fires at pointer frequency, but this derived state only changes with + // the drag origin (a new drag) or the document value (a remote patch + // landing mid-drag), so it is derived once per (value, origin) pair + // instead of per event. The value key compares by identity (the value + // array is replaced on every applied operation); the origin key compares + // by selection equality, because the origin object's identity does not + // survive the event pipeline. + let dragOriginState: DragOriginState | undefined + + function getDragOriginState( + snapshot: EditorSnapshot, + dragOriginSelection: NonNullable, + ): DragOriginState { + if ( + dragOriginState && + dragOriginState.value === snapshot.context.value && + isEqualSelections( + dragOriginState.dragOriginSelection, + dragOriginSelection, + ) + ) { + return dragOriginState + } + + const dragSelection = getDragSelection({ + eventSelection: dragOriginSelection, + snapshot, + }) + const dragSnapshot = { + ...snapshot, + context: { + ...snapshot.context, + selection: dragSelection, + }, + } + const draggedBlocks = getSelectedBlocks(dragSnapshot) + // `getSelectedBlocks` only looks at root-level children, so a drag + // origin nested inside a container (a callout paragraph, a table + // cell) resolves to the container, not the nested block actually + // being dragged. `getFocusBlock` resolves the origin's own focus to + // the same depth drop focus blocks are resolved to, so comparing the + // two catches a nested block dragged over itself too. + const draggedFocusBlock = getFocusBlock(dragSnapshot) + + dragOriginState = { + value: snapshot.context.value, + dragOriginSelection, + draggedBlockKeys: new Set( + draggedBlocks.map((draggedBlock) => draggedBlock.node._key), + ), + draggedFocusBlockPath: draggedFocusBlock?.path, + draggingEntireBlocks: isSelectingEntireBlocks(dragSnapshot), + } + + return dragOriginState + } + return [ defineBehavior({ on: 'drag.dragover', @@ -178,34 +249,13 @@ function createDropPositionBehaviors( return false } - const dragSelection = getDragSelection({ - eventSelection: dragOrigin.selection, - snapshot, - }) - const dragSnapshot = { - ...snapshot, - context: { - ...snapshot.context, - selection: dragSelection, - }, - } - - const draggedBlocks = getSelectedBlocks(dragSnapshot) - // `getSelectedBlocks` only looks at root-level children, so a drag - // origin nested inside a container (a callout paragraph, a table - // cell) resolves to the container, not the nested block actually - // being dragged. `getFocusBlock` resolves the origin's own focus to - // the same depth `dropFocusBlock` was resolved to, so comparing the - // two catches a nested block dragged over itself too. - const draggedFocusBlock = getFocusBlock(dragSnapshot) + const {draggedBlockKeys, draggedFocusBlockPath, draggingEntireBlocks} = + getDragOriginState(snapshot, dragOrigin.selection) const selfDrop = - draggedBlocks.some( - (draggedBlock) => - draggedBlock.node._key === dropFocusBlock.node._key, - ) || - (draggedFocusBlock !== undefined && - isEqualPaths(draggedFocusBlock.path, dropFocusBlock.path)) + draggedBlockKeys.has(dropFocusBlock.node._key) || + (draggedFocusBlockPath !== undefined && + isEqualPaths(draggedFocusBlockPath, dropFocusBlock.path)) if (selfDrop) { // Core cancels the drop, so no position is valid here, including @@ -213,14 +263,6 @@ function createDropPositionBehaviors( return {dropFocusBlock, droppedInsideTextBlock: false, clear: true} } - const draggingEntireBlocks = isSelectingEntireBlocks({ - ...snapshot, - context: { - ...snapshot.context, - selection: dragSelection, - }, - }) - if (!draggingEntireBlocks) { return false }