Skip to content

Commit

Permalink
Do not pass dragover to children
Browse files Browse the repository at this point in the history
It seems to be more performant.

API-wise it makes sense that withDraggable takes care of calling
children's dragstart/dragend event handlers as it needs them
to be executed after its own. The reason for this is that the
children's event handlers could modify the DOM element
withDraggable clones, messing up with its behavior.
For example, if the children's event handler change the visibility of
the element that takes the event, in turn the element won't be able
to process any further events.

We don't have the same restrictions with the dragover event.
At that point, it can work independently from the children's.
  • Loading branch information
oandregal committed Aug 29, 2018
1 parent 41fe72e commit 1185662
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 3 deletions.
7 changes: 6 additions & 1 deletion packages/components/src/higher-order/with-draggable/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ const withDraggable = createHigherOrderComponent(
return;
}

// Connect event listeners
document.addEventListener( 'dragover', this.onDragOver );

event.dataTransfer.setData( 'text', JSON.stringify( transferData ) );

// Set a fake drag image to avoid browser defaults. Remove from DOM
Expand Down Expand Up @@ -124,6 +127,9 @@ const withDraggable = createHigherOrderComponent(
* while dragging.
*/
resetDragState() {
// Remove event listeners
document.removeEventListener( 'dragover', this.onDragOver );

// Remove drag clone
if ( this.cloneWrapper && this.cloneWrapper.parentNode ) {
this.cloneWrapper.parentNode.removeChild( this.cloneWrapper );
Expand All @@ -139,7 +145,6 @@ const withDraggable = createHigherOrderComponent(
<OriginalComponent
{ ...this.props }
onDragStart={ this.onDragStart }
onDragOver={ this.onDragOver }
onDragEnd={ this.onDragEnd }
/>
);
Expand Down
3 changes: 1 addition & 2 deletions packages/editor/src/components/block-list/block-draggable.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { withDraggable } from '@wordpress/components';

class BlockDraggable extends Component {
render() {
const { isDragging, onDragStart, onDragOver, onDragEnd } = this.props;
const { isDragging, onDragStart, onDragEnd } = this.props;
const className = classnames( 'editor-block-list__block-draggable', {
'is-visible': isDragging,
} );
Expand All @@ -20,7 +20,6 @@ class BlockDraggable extends Component {
<div
className={ className }
onDragStart={ onDragStart }
onDragOver={ onDragOver }
onDragEnd={ onDragEnd }
draggable
>
Expand Down

0 comments on commit 1185662

Please sign in to comment.