diff --git a/.changeset/brave-drops-settle.md b/.changeset/brave-drops-settle.md new file mode 100644 index 0000000000..61adabff30 --- /dev/null +++ b/.changeset/brave-drops-settle.md @@ -0,0 +1,6 @@ +--- +'slate-dom': minor +'slate-react': patch +--- + +Stop drops from throwing when the browser resolves the drop position outside of the editor. `` resolved the target range from the drop coordinates via `caretRangeFromPoint`, which can land on a DOM node outside of the editor even though the event's target is inside it (e.g. when the page scrolled between the last `dragover` and the `drop`); the resulting "Cannot resolve a Slate point from DOM point" error escaped the event handler and the drop was lost. The dropped data now goes to the current selection instead (the end of the document without one), and internally dragged content stays where it is. `DOMEditor.findEventRange` gained a `suppressThrow` option for this and returns `null` when the range cannot be resolved. diff --git a/docs/libraries/slate-react/react-editor.md b/docs/libraries/slate-react/react-editor.md index 23cbc7b194..97b1fe50cb 100644 --- a/docs/libraries/slate-react/react-editor.md +++ b/docs/libraries/slate-react/react-editor.md @@ -74,9 +74,9 @@ Find a native DOM range from a Slate `range`. Find a Slate node from a native DOM `element`. -#### `ReactEditor.findEventRange(editor: ReactEditor, event: any): Range` +#### `ReactEditor.findEventRange(editor: ReactEditor, event: any, options?: { suppressThrow?: boolean }): Range | null` -Get the target range from a DOM `event`. +Get the target range from a DOM `event`. The range is resolved from the event's coordinates, which can point outside of the editor even when the event's target lies inside it; with `suppressThrow: true` this returns `null` instead of throwing in that case. #### `ReactEditor.toSlatePoint(editor: ReactEditor, domPoint: DOMPoint): Point | null` diff --git a/packages/slate-dom/src/plugin/dom-editor.ts b/packages/slate-dom/src/plugin/dom-editor.ts index 34aff28e9e..1d77da1838 100644 --- a/packages/slate-dom/src/plugin/dom-editor.ts +++ b/packages/slate-dom/src/plugin/dom-editor.ts @@ -105,8 +105,16 @@ export interface DOMEditorInterface { /** * Get the target range from a DOM `event`. + * + * The range is resolved from the event's coordinates, which can point + * outside of the editor even when the event's target lies inside it. With + * `suppressThrow`, `null` is returned instead of throwing in that case. */ - findEventRange: (editor: DOMEditor, event: any) => Range + findEventRange: ( + editor: DOMEditor, + event: any, + options?: { suppressThrow?: T } + ) => T extends true ? Range | null : Range /** * Find a key for a Slate node. @@ -311,7 +319,13 @@ export const DOMEditor: DOMEditorInterface = { return el.ownerDocument }, - findEventRange: (editor, event) => { + findEventRange: ( + editor: DOMEditor, + event: any, + options: { suppressThrow?: T } = {} + ): T extends true ? Range | null : Range => { + const { suppressThrow = false } = options + if ('nativeEvent' in event) { event = event.nativeEvent } @@ -319,6 +333,9 @@ export const DOMEditor: DOMEditorInterface = { const { clientX: x, clientY: y, target } = event if (x == null || y == null) { + if (suppressThrow) { + return null as T extends true ? Range | null : Range + } throw new Error(`Cannot resolve a Slate range from a DOM event: ${event}`) } @@ -343,7 +360,7 @@ export const DOMEditor: DOMEditorInterface = { if (point) { const range = Editor.range(editor, point) - return range + return range as T extends true ? Range | null : Range } } @@ -365,15 +382,18 @@ export const DOMEditor: DOMEditorInterface = { } if (!domRange) { + if (suppressThrow) { + return null as T extends true ? Range | null : Range + } throw new Error(`Cannot resolve a Slate range from a DOM event: ${event}`) } // Resolve a Slate range from the DOM range. const range = DOMEditor.toSlateRange(editor, domRange, { exactMatch: false, - suppressThrow: false, + suppressThrow, }) - return range + return range as T extends true ? Range | null : Range }, findKey: (editor, node) => { diff --git a/packages/slate-react/src/components/editable.tsx b/packages/slate-react/src/components/editable.tsx index 04e24e7f76..2ae29d4f06 100644 --- a/packages/slate-react/src/components/editable.tsx +++ b/packages/slate-react/src/components/editable.tsx @@ -1491,22 +1491,42 @@ export const Editable = forwardRef( // Keep a reference to the dragged range before updating selection const draggedRange = editor.selection - // Find the range where the drop happened - const range = ReactEditor.findEventRange(editor, event) + // Find the range where the drop happened. It is resolved from + // the drop coordinates, which can point outside of the editor + // even though the event's target is inside it (e.g. when the + // page scrolled during the drag), leaving no range to drop at. + const range = ReactEditor.findEventRange(editor, event, { + suppressThrow: true, + }) const data = event.dataTransfer - Transforms.select(editor, range) + if (!range && state.isDraggingInternally) { + // The dragged content already sits in the editor, so without + // a target it stays where it is. + return + } - if (state.isDraggingInternally) { - if ( - draggedRange && - !Range.equals(draggedRange, range) && - !Editor.void(editor, { at: range, voids: true }) - ) { - Transforms.delete(editor, { - at: draggedRange, - }) + if (range) { + Transforms.select(editor, range) + + if (state.isDraggingInternally) { + if ( + draggedRange && + !Range.equals(draggedRange, range) && + !Editor.void(editor, { at: range, voids: true }) + ) { + Transforms.delete(editor, { + at: draggedRange, + }) + } } + } else if ( + !editor.selection && + editor.children.length > 0 + ) { + // Without a drop position the data goes to the current + // selection, or to the end of the document if there is none. + Transforms.select(editor, Editor.end(editor, [])) } ReactEditor.insertData(editor, data) diff --git a/packages/slate-react/test/editable.spec.tsx b/packages/slate-react/test/editable.spec.tsx index 5b66fd3c05..da1c9b1755 100644 --- a/packages/slate-react/test/editable.spec.tsx +++ b/packages/slate-react/test/editable.spec.tsx @@ -284,5 +284,139 @@ describe('slate-react', () => { Transforms.select(editor, { path: [0, 0], offset: 2 }) }) }) + describe('onDrop', () => { + const initialValue = [{ type: 'block', children: [{ text: 'test' }] }] + + // jsdom implements neither `DataTransfer` nor `caretRangeFromPoint` + const makeDataTransfer = (text: string) => + ({ + getData: (type: string) => (type === 'text/plain' ? text : ''), + setData: () => {}, + }) as unknown as DataTransfer + + const renderEditor = () => { + const editor = withReact(createEditor()) + + act(() => { + render( + {}} + > + + + ) + }) + + return editor + } + + const dispatch = async ( + editor: ReactEditor, + type: 'dragstart' | 'drop', + text: string + ) => { + const event = new MouseEvent(type, { + bubbles: true, + cancelable: true, + clientX: 1, + clientY: 1, + }) + Object.defineProperty(event, 'dataTransfer', { + value: makeDataTransfer(text), + }) + + await act(async () => { + ReactEditor.toDOMNode(editor, editor).dispatchEvent(event) + }) + } + + const resolveDropPositionTo = (node: Node, offset: number) => { + const caretRangeFromPoint = jest.fn(() => { + const range = document.createRange() + range.setStart(node, offset) + range.collapse(true) + return range + }) + document.caretRangeFromPoint = caretRangeFromPoint + return caretRangeFromPoint + } + + // A text node the browser can resolve the drop position to although the + // event's target is the editor, e.g. after the page scrolled during the drag. + let outside: HTMLElement | null = null + const renderTextOutsideEditor = () => { + outside = document.body.appendChild(document.createElement('p')) + outside.textContent = 'outside' + return outside.firstChild! + } + + afterEach(() => { + delete (document as Partial).caretRangeFromPoint + outside?.remove() + outside = null + }) + + test('inserts the dropped data at the drop position', async () => { + const editor = renderEditor() + const text = ReactEditor.toDOMNode(editor, editor).querySelector( + '[data-slate-string]' + )!.firstChild! + resolveDropPositionTo(text, 2) + + await dispatch(editor, 'drop', 'drop') + + expect(editor.children).toEqual([ + { type: 'block', children: [{ text: 'tedropst' }] }, + ]) + }) + + test('inserts at the selection when the drop position lies outside of the editor', async () => { + const editor = renderEditor() + const caretRangeFromPoint = resolveDropPositionTo( + renderTextOutsideEditor(), + 7 + ) + await act(async () => { + Transforms.select(editor, { path: [0, 0], offset: 2 }) + }) + + await dispatch(editor, 'drop', 'drop') + + expect(caretRangeFromPoint).toHaveBeenCalled() + expect(editor.children).toEqual([ + { type: 'block', children: [{ text: 'tedropst' }] }, + ]) + }) + + test('inserts at the end when the drop position lies outside of the editor and nothing is selected', async () => { + const editor = renderEditor() + resolveDropPositionTo(renderTextOutsideEditor(), 7) + expect(editor.selection).toBeNull() + + await dispatch(editor, 'drop', 'drop') + + expect(editor.children).toEqual([ + { type: 'block', children: [{ text: 'testdrop' }] }, + ]) + }) + + test('leaves internally dragged content in place when the drop position lies outside of the editor', async () => { + const editor = renderEditor() + resolveDropPositionTo(renderTextOutsideEditor(), 7) + await act(async () => { + Transforms.select(editor, { + anchor: { path: [0, 0], offset: 0 }, + focus: { path: [0, 0], offset: 2 }, + }) + }) + await dispatch(editor, 'dragstart', 'te') + + await dispatch(editor, 'drop', 'te') + + expect(editor.children).toEqual(initialValue) + }) + }) }) })