From 8f389d257ae5437475b6f8af646164406b3624b1 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Fri, 9 Oct 2020 17:31:27 +0800 Subject: [PATCH 1/4] Remove isDraggingBlocks check --- packages/block-editor/src/components/block-list/index.js | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/packages/block-editor/src/components/block-list/index.js b/packages/block-editor/src/components/block-list/index.js index dbc3fe538d3e7b..c44e570316c22f 100644 --- a/packages/block-editor/src/components/block-list/index.js +++ b/packages/block-editor/src/components/block-list/index.js @@ -70,7 +70,6 @@ function Items( { hasMultiSelection, getGlobalBlockCount, isTyping, - isDraggingBlocks, __experimentalGetActiveBlockIdByBlockNames, } = select( 'core/block-editor' ); @@ -88,7 +87,6 @@ function Items( { enableAnimation: ! isTyping() && getGlobalBlockCount() <= BLOCK_ANIMATION_THRESHOLD, - isDraggingBlocks: isDraggingBlocks(), activeEntityBlockId, }; } @@ -100,7 +98,6 @@ function Items( { orientation, hasMultiSelection, enableAnimation, - isDraggingBlocks, activeEntityBlockId, } = useSelect( selector, [ rootClientId ] ); @@ -109,8 +106,7 @@ function Items( { rootClientId, } ); - const isAppenderDropTarget = - dropTargetIndex === blockClientIds.length && isDraggingBlocks; + const isAppenderDropTarget = dropTargetIndex === blockClientIds.length; return ( <> @@ -119,8 +115,7 @@ function Items( { ? multiSelectedBlockClientIds.includes( clientId ) : selectedBlockClientId === clientId; - const isDropTarget = - dropTargetIndex === index && isDraggingBlocks; + const isDropTarget = dropTargetIndex === index; return ( Date: Fri, 9 Oct 2020 17:40:35 +0800 Subject: [PATCH 2/4] Clean up drag and drop data if the user presses escape when dragging --- packages/components/src/drop-zone/provider.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/packages/components/src/drop-zone/provider.js b/packages/components/src/drop-zone/provider.js index 5496ddf4d9165e..7330f9eabd4023 100644 --- a/packages/components/src/drop-zone/provider.js +++ b/packages/components/src/drop-zone/provider.js @@ -6,6 +6,7 @@ import { isEqual, find, some, filter, throttle, includes } from 'lodash'; /** * WordPress dependencies */ +import { ESCAPE } from '@wordpress/keycodes'; import { Component, createContext, createRef } from '@wordpress/element'; import isShallowEqual from '@wordpress/is-shallow-equal'; @@ -60,6 +61,7 @@ class DropZoneProvider extends Component { // Event listeners this.onDragOver = this.onDragOver.bind( this ); this.onDrop = this.onDrop.bind( this ); + this.onKeyUp = this.onKeyUp.bind( this ); // Context methods so this component can receive data from consumers this.addDropZone = this.addDropZone.bind( this ); this.removeDropZone = this.removeDropZone.bind( this ); @@ -88,6 +90,7 @@ class DropZoneProvider extends Component { const { defaultView } = ownerDocument; defaultView.addEventListener( 'dragover', this.onDragOver ); defaultView.addEventListener( 'mouseup', this.resetDragState ); + defaultView.addEventListener( 'keyup', this.onKeyUp ); } componentWillUnmount() { @@ -95,6 +98,7 @@ class DropZoneProvider extends Component { const { defaultView } = ownerDocument; defaultView.removeEventListener( 'dragover', this.onDragOver ); defaultView.removeEventListener( 'mouseup', this.resetDragState ); + defaultView.removeEventListener( 'keyup', this.onKeyUp ); } addDropZone( dropZone ) { @@ -226,6 +230,14 @@ class DropZoneProvider extends Component { event.preventDefault(); } + onKeyUp( event ) { + const { isDraggingOverDocument } = this.state; + // If the user presses escape while dragging, cancel the drag. + if ( isDraggingOverDocument && event.keyCode === ESCAPE ) { + this.resetDragState(); + } + } + onDrop( event ) { // This seemingly useless line has been shown to resolve a Safari issue // where files dragged directly from the dock are not recognized From dc90bb46ace8a484b5069a1926f09de51044b845 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Fri, 9 Oct 2020 17:53:58 +0800 Subject: [PATCH 3/4] Add comment to ensure avoidance of dragend --- packages/components/src/drop-zone/provider.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/components/src/drop-zone/provider.js b/packages/components/src/drop-zone/provider.js index 7330f9eabd4023..ed81a6902c576f 100644 --- a/packages/components/src/drop-zone/provider.js +++ b/packages/components/src/drop-zone/provider.js @@ -90,6 +90,8 @@ class DropZoneProvider extends Component { const { defaultView } = ownerDocument; defaultView.addEventListener( 'dragover', this.onDragOver ); defaultView.addEventListener( 'mouseup', this.resetDragState ); + // The `dragend` event could be used for this, but only has partial + // support in firefox. defaultView.addEventListener( 'keyup', this.onKeyUp ); } From 9d1a55e4631372152df7c0a82ed72eaa839c0074 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Mon, 12 Oct 2020 13:11:06 +0800 Subject: [PATCH 4/4] Switch to using dragend --- packages/components/src/drop-zone/provider.js | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/packages/components/src/drop-zone/provider.js b/packages/components/src/drop-zone/provider.js index ed81a6902c576f..364f5491bb4afc 100644 --- a/packages/components/src/drop-zone/provider.js +++ b/packages/components/src/drop-zone/provider.js @@ -6,7 +6,6 @@ import { isEqual, find, some, filter, throttle, includes } from 'lodash'; /** * WordPress dependencies */ -import { ESCAPE } from '@wordpress/keycodes'; import { Component, createContext, createRef } from '@wordpress/element'; import isShallowEqual from '@wordpress/is-shallow-equal'; @@ -61,7 +60,6 @@ class DropZoneProvider extends Component { // Event listeners this.onDragOver = this.onDragOver.bind( this ); this.onDrop = this.onDrop.bind( this ); - this.onKeyUp = this.onKeyUp.bind( this ); // Context methods so this component can receive data from consumers this.addDropZone = this.addDropZone.bind( this ); this.removeDropZone = this.removeDropZone.bind( this ); @@ -90,9 +88,10 @@ class DropZoneProvider extends Component { const { defaultView } = ownerDocument; defaultView.addEventListener( 'dragover', this.onDragOver ); defaultView.addEventListener( 'mouseup', this.resetDragState ); - // The `dragend` event could be used for this, but only has partial - // support in firefox. - defaultView.addEventListener( 'keyup', this.onKeyUp ); + // Note that `dragend` doesn't fire consistently for file and HTML drag + // events where the drag origin is outside the browser window. + // In Firefox it may also not fire if the originating node is removed. + defaultView.addEventListener( 'dragend', this.resetDragState ); } componentWillUnmount() { @@ -100,7 +99,7 @@ class DropZoneProvider extends Component { const { defaultView } = ownerDocument; defaultView.removeEventListener( 'dragover', this.onDragOver ); defaultView.removeEventListener( 'mouseup', this.resetDragState ); - defaultView.removeEventListener( 'keyup', this.onKeyUp ); + defaultView.removeEventListener( 'dragend', this.resetDragState ); } addDropZone( dropZone ) { @@ -232,14 +231,6 @@ class DropZoneProvider extends Component { event.preventDefault(); } - onKeyUp( event ) { - const { isDraggingOverDocument } = this.state; - // If the user presses escape while dragging, cancel the drag. - if ( isDraggingOverDocument && event.keyCode === ESCAPE ) { - this.resetDragState(); - } - } - onDrop( event ) { // This seemingly useless line has been shown to resolve a Safari issue // where files dragged directly from the dock are not recognized