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/drag-event-guard.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@portabletext/editor': patch
---

fix: guard `drag` and `dragleave` handlers without resolving an event position

The editor no longer runs a caret hit-test and block rect reads for every `drag` and `dragleave` event during a drag. Both events forward to behaviors without a position, so the resolution was pure per-pointer-move cost; the handlers now only check that the event target belongs to the editor. In rare cases where position resolution would have failed (for example while the editor is tearing down mid-drag), the `drag.drag` and `drag.dragleave` behavior events now still fire.
29 changes: 15 additions & 14 deletions packages/editor/src/editor/Editable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -687,13 +687,14 @@ export const PortableTextEditable = forwardRef<
return
}

const position = getEventPosition({
editorActor,
editorEngine,
event: event.nativeEvent,
})

if (!position) {
// The forwarded `drag.drag` event carries no position, so resolving
// one (a caret hit-test plus block rect reads) is wasted work on an
// event the browser fires continuously on the drag source. A
// containment check is all the guard needs.
if (
editorActor.getSnapshot().matches({setup: 'setting up'}) ||
!DOMEditor.hasTarget(editorEngine, event.target)
) {
return
}

Expand Down Expand Up @@ -860,13 +861,13 @@ export const PortableTextEditable = forwardRef<
return
}

const position = getEventPosition({
editorActor,
editorEngine,
event: event.nativeEvent,
})

if (!position) {
// The forwarded `drag.dragleave` event carries no position either;
// the same cheap guard as `drag.drag` applies. `dragleave` fires on
// every element boundary inside the editor during a drag.
if (
editorActor.getSnapshot().matches({setup: 'setting up'}) ||
!DOMEditor.hasTarget(editorEngine, event.target)
) {
return
}

Expand Down
69 changes: 69 additions & 0 deletions packages/editor/tests/event.drag.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
import {describe, expect, test, vi} from 'vitest'
import {defineBehavior} from '../src/behaviors/behavior.types.behavior'
import {BehaviorPlugin} from '../src/plugins/plugin.behavior'
import {createTestEditor} from '../src/test/vitest'

describe('event.drag', () => {
test('Scenario: `drag.drag` and `drag.dragleave` reach behaviors without a caret hit-test', async () => {
const onDrag = vi.fn()
const onDragLeave = vi.fn()

const {locator} = await createTestEditor({
children: (
<BehaviorPlugin
behaviors={[
defineBehavior({
on: 'drag.drag',
guard: () => {
onDrag()
return false
},
actions: [],
}),
defineBehavior({
on: 'drag.dragleave',
guard: () => {
onDragLeave()
return false
},
actions: [],
}),
]}
/>
),
})

const editorElement = locator.element()

const documentWithCaret = window.document as Document & {
caretPositionFromPoint(x: number, y: number): unknown
}
const caretHitTest = vi.spyOn(documentWithCaret, 'caretPositionFromPoint')

editorElement.dispatchEvent(
new DragEvent('drag', {
bubbles: true,
cancelable: true,
dataTransfer: new DataTransfer(),
}),
)
editorElement.dispatchEvent(
new DragEvent('dragleave', {
bubbles: true,
cancelable: true,
dataTransfer: new DataTransfer(),
}),
)

await vi.waitFor(() => {
expect(onDrag).toHaveBeenCalledTimes(1)
expect(onDragLeave).toHaveBeenCalledTimes(1)
})

// These events carry no position, so the handlers must not pay for
// resolving one at pointer-move frequency.
expect(caretHitTest).not.toHaveBeenCalled()

caretHitTest.mockRestore()
})
})
Loading