Skip to content

Commit de13468

Browse files
draganescuclaude
andcommitted
Compose: Use the useDropZone fix from its own PR
Swaps this PR's `useDropZone` change for the one in fix/use-drop-zone-iframe-dragover, which starts a drag on the document's first `dragover` and ignores mouse moves with a button held. This copy goes once that PR lands, and the CHANGELOG entry lives there. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 8b27fea commit de13468

3 files changed

Lines changed: 75 additions & 20 deletions

File tree

‎packages/compose/CHANGELOG.md‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
### Bug Fixes
2222

2323
- `useConstrainedTabbing`: Retain focus when the constrained area has no tabbable elements ([#84344](https://github.com/WordPress/gutenberg/pull/84344)).
24-
- `useDropZone`: Start a drag on the first `dragover` when the document got no `dragenter`, which Chrome can skip in an iframe, so that `onDragEnd` is still called ([#84500](https://github.com/WordPress/gutenberg/pull/84500)).
2524

2625
### Internal
2726

‎packages/compose/src/hooks/use-drop-zone/index.ts‎

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -108,12 +108,6 @@ export default function useDropZone( {
108108
}
109109

110110
function onDragOver( event: DragEvent ): void {
111-
// Chrome can send `dragover` to an iframe without a `dragenter`
112-
// first, after a previous drag left the iframe over an element
113-
// of the parent document. Start the drag here too, so that
114-
// its end is still handled.
115-
maybeDragStart( event );
116-
117111
if ( ! event.defaultPrevented && _onDragOver ) {
118112
onDragOverEvent( event );
119113
}
@@ -167,6 +161,15 @@ export default function useDropZone( {
167161
return;
168162
}
169163

164+
// A mouse move with a button held is still part of the drag,
165+
// in browsers that send them while dragging.
166+
if (
167+
event.type === 'mousemove' &&
168+
( event as MouseEvent ).buttons !== 0
169+
) {
170+
return;
171+
}
172+
170173
isDragging = false;
171174

172175
ownerDocument.removeEventListener( 'dragend', maybeDragEnd );
@@ -185,6 +188,12 @@ export default function useDropZone( {
185188
// The `dragstart` event doesn't fire if the drag started outside
186189
// the document.
187190
ownerDocument.addEventListener( 'dragenter', maybeDragStart );
191+
// Chrome can send `dragover` to an iframe without a `dragenter`
192+
// first, after a previous drag left the iframe over an element of
193+
// the parent document. Start the drag on the first `dragover` too,
194+
// so that its end is still handled. It's a capture listener, so
195+
// that the drag starts before the drop zone's own `dragover`.
196+
ownerDocument.addEventListener( 'dragover', maybeDragStart, true );
188197

189198
return () => {
190199
element.removeAttribute( 'data-is-drop-zone' );
@@ -198,6 +207,11 @@ export default function useDropZone( {
198207
'dragenter',
199208
maybeDragStart
200209
);
210+
ownerDocument.removeEventListener(
211+
'dragover',
212+
maybeDragStart,
213+
true
214+
);
201215
};
202216
},
203217
[ isDisabled, dropZoneElement ] // Refresh when the passed in dropZoneElement changes.

‎packages/compose/src/hooks/use-drop-zone/test/index.jsdom.test.tsx‎

Lines changed: 55 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -51,28 +51,70 @@ describe( 'useDropZone', () => {
5151
'data-is-drop-zone'
5252
);
5353
} );
54+
const ComponentWithDragCallbacks = ( {
55+
onDragStart,
56+
onDragEnd,
57+
}: {
58+
onDragStart: () => void;
59+
onDragEnd: () => void;
60+
} ) => {
61+
const dropZoneRef = useDropZone( { onDragStart, onDragEnd } );
5462

55-
it( 'ends a drag that reaches the drop zone without a `dragenter` on the document', () => {
63+
return (
64+
<div role="main">
65+
<div role="region" ref={ dropZoneRef }>
66+
<div>Drop Zone</div>
67+
</div>
68+
</div>
69+
);
70+
};
71+
72+
function renderWithDragCallbacks() {
5673
const onDragStart = vi.fn();
5774
const onDragEnd = vi.fn();
58-
const Component = () => {
59-
const dropZoneRef = useDropZone( { onDragStart, onDragEnd } );
75+
const { rerender } = render(
76+
<ComponentWithDragCallbacks
77+
onDragStart={ onDragStart }
78+
onDragEnd={ onDragEnd }
79+
/>
80+
);
81+
// Ensure `useEffect` has run.
82+
rerender(
83+
<ComponentWithDragCallbacks
84+
onDragStart={ onDragStart }
85+
onDragEnd={ onDragEnd }
86+
/>
87+
);
88+
return { onDragStart, onDragEnd };
89+
}
6090

61-
return (
62-
<div role="region" ref={ dropZoneRef }>
63-
Drop Zone
64-
</div>
65-
);
66-
};
67-
render( <Component /> );
91+
it( 'starts a drag on the first dragover in the document, without a dragenter', () => {
92+
const { onDragStart } = renderWithDragCallbacks();
93+
94+
fireEvent.dragOver( document.body );
95+
fireEvent.dragOver( document.body );
96+
97+
expect( onDragStart ).toHaveBeenCalledTimes( 1 );
98+
} );
99+
100+
it( 'ends a drag that reaches the drop zone without a dragenter on the document', () => {
101+
const { onDragStart, onDragEnd } = renderWithDragCallbacks();
68102

69-
// Chrome can send `dragover` to an iframe without a `dragenter`
70-
// first, after a previous drag left the iframe over an element of
71-
// the parent document.
72103
fireEvent.dragOver( screen.getByRole( 'region' ) );
73104
expect( onDragStart ).toHaveBeenCalledTimes( 1 );
74105

75106
fireEvent.dragEnd( document );
76107
expect( onDragEnd ).toHaveBeenCalledTimes( 1 );
77108
} );
109+
110+
it( 'does not end a drag on a mousemove while a button is held', () => {
111+
const { onDragEnd } = renderWithDragCallbacks();
112+
113+
fireEvent.dragEnter( document.body );
114+
fireEvent.mouseMove( document, { buttons: 1 } );
115+
expect( onDragEnd ).not.toHaveBeenCalled();
116+
117+
fireEvent.mouseMove( document, { buttons: 0 } );
118+
expect( onDragEnd ).toHaveBeenCalledTimes( 1 );
119+
} );
78120
} );

0 commit comments

Comments
 (0)