Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/dnd-drag-constants-memo.md
Original file line number Diff line number Diff line change
@@ -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.
7 changes: 7 additions & 0 deletions .changeset/dnd-dragenter-clear.md
Original file line number Diff line number Diff line change
@@ -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`.
76 changes: 76 additions & 0 deletions packages/plugin-dnd/src/plugin.dnd.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,82 @@ 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: 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<EditorSelection> = {
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()

Expand Down
130 changes: 88 additions & 42 deletions packages/plugin-dnd/src/plugin.dnd.tsx
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -19,6 +24,7 @@ import {
isEmptyTextBlock,
isEqualPaths,
isEqualSelectionPoints,
isEqualSelections,
isKeyedSegment,
isSelectionCollapsed,
} from '@portabletext/editor/utils'
Expand Down Expand Up @@ -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<EditorSelection>
draggedBlockKeys: Set<string>
draggedFocusBlockPath: Path | undefined
draggingEntireBlocks: boolean
}

function createDropPositionBehaviors(
setDropPosition: (next: DropPosition | undefined) => void,
): Array<Behavior> {
// `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<EditorSelection>,
): 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',
Expand All @@ -178,49 +249,20 @@ 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
// one left over from the previous hover.
return {dropFocusBlock, droppedInsideTextBlock: false, clear: true}
}

const draggingEntireBlocks = isSelectingEntireBlocks({
...snapshot,
context: {
...snapshot.context,
selection: dragSelection,
},
})

if (!draggingEntireBlocks) {
return false
}
Expand Down Expand Up @@ -276,13 +318,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(() => {
Expand Down
Loading