Refactor Draggable to decouple drag handle from the DOM node being dr…

…agged (#9311)

docs/reference/deprecated.md

@@ -3,6 +3,7 @@ Gutenberg's deprecation policy is intended to support backwards-compatibility fo
 ## 4.0.0
 
 - `wp.components.RichTextProvider` has been removed. Please use `wp.data.select( 'core/editor' )` methods instead.
+- `wp.components.Draggable` as a DOM node drag handler has been removed. Please, use `wp.components.Draggable` as a wrap component for your DOM node drag handler.
 
 ## 3.9.0
 

packages/components/CHANGELOG.md

@@ -2,4 +2,5 @@
 
 ### Breaking Change
 
-- `withAPIData` has been removed. Please use the Core Data module or `@wordpress/api-fetch` directly instead.
+- `withAPIData` has been removed. Please use the Core Data module or `@wordpress/api-fetch` directly instead.
+- `wp.components.Draggable` as a DOM node drag handler has been deprecated. Please, use `wp.components.Draggable` as a wrap component for your DOM node drag handler.

packages/components/src/draggable/README.md

@@ -1,6 +1,8 @@
 # Draggable
 
-`Draggable` is a Component that can wrap any element to make it draggable. When used, a cross-browser (including IE) customisable drag image is created. The component clones the specified element on drag-start and uses the clone as a drag image during drag-over. Discards the clone on drag-end.
+`Draggable` is a Component that provides a way to set up a a cross-browser (including IE) customisable drag image and the transfer data for the drag event. It decouples the drag handle and the element to drag: use it by wrapping the component that will become the drag handle and providing the DOM ID of the element to drag.
+
+Note that the drag handle needs to declare the `draggable="true"` property and bind the `Draggable`s `onDraggableStart` and `onDraggableEnd` event handlers to its own `onDragStart` and `onDragEnd` respectively. `Draggable` takes care of the logic to setup the drag image and the transfer data, but is not concerned with creating an actual DOM element that is draggable.
 
 ## Props
 
@@ -22,15 +24,15 @@ Arbitrary data object attached to the drag and drop event.
 
 ### onDragStart
 
-The function called when dragging starts.
+A function to be called when dragging starts.
 
 - Type: `Function`
 - Required: No
 - Default: `noop`
 
 ### onDragEnd
 
-The function called when dragging ends.
+A function to be called when dragging ends.
 
 - Type: `Function`
 - Required: No
@@ -49,10 +51,54 @@ const MyDraggable = () => (
 					elementId="draggable-panel"
 					transferData={ { } }
 				>
-					<Dashicon icon="move" />
+				{
+					( { onDraggableStart, onDraggableEnd } ) => (
+						<Dashicon
+							icon="move"
+							onDragStart={ onDraggableStart }
+							onDragEnd={ onDraggableEnd }
+							draggable
+						/>
+					)
+				}
+				</Draggable>
+			</PanelBody>
+		</Panel>
+	</div>
+);
+export default MyDraggable;
+```
+
+In case you want to call your own `dragstart` / `dragend` event handlers as well, you can pass them to `Draggable` and it'll take care of calling them after their own:
+
+```jsx
+import { Dashicon, Draggable, Panel, PanelBody } from '@wordpress/components';
+
+const MyDraggable = ( { onDragStart, onDragEnd } ) => (
+	<div id="draggable-panel">
+		<Panel header="Draggable panel" >
+			<PanelBody>
+				<Draggable
+					elementId="draggable-panel"
+					transferData={ { } }
+					onDragStart={ onDragStart }
+					onDragEnd={ onDragEnd }
+				>
+				{
+					( { onDraggableStart, onDraggableEnd } ) => (
+						<Dashicon
+							icon="move"
+							onDragStart={ onDraggableStart }
+							onDragEnd={ onDraggableEnd }
+							draggable
+						/>
+					)
+				}
 				</Draggable>
 			</PanelBody>
 		</Panel>
 	</div>
 );
+
+export default MyDraggable;
 ```

packages/components/src/draggable/index.js

@@ -9,6 +9,7 @@ import classnames from 'classnames';
  */
 import { Component } from '@wordpress/element';
 import { withSafeTimeout } from '@wordpress/compose';
+import deprecated from '@wordpress/deprecated';
 
 const dragImageClass = 'components-draggable__invisible-drag-image';
 const cloneWrapperClass = 'components-draggable__clone';
@@ -148,6 +149,19 @@ class Draggable extends Component {
 
 	render() {
 		const { children, className } = this.props;
+		if ( typeof children === 'function' ) {
+			return children( {
+				onDraggableStart: this.onDragStart,
+				onDraggableEnd: this.onDragEnd,
+			} );
+		}
+
+		deprecated( 'wp.components.Draggable as a DOM node drag handle', {
+			version: 4.0,
+			alternative: 'wp.components.Draggable as a wrapper component for a DOM node',
+			plugin: 'Gutenberg',
+		} );
+
 		return (
 			<div
 				className={ classnames( 'components-draggable', className ) }

packages/editor/src/components/block-list/block-draggable.js

@@ -8,24 +8,39 @@ import classnames from 'classnames';
  */
 import { Draggable } from '@wordpress/components';
 
-function BlockDraggable( { rootClientId, index, clientId, layout, isDragging, ...props } ) {
+const BlockDraggable = ( { clientId, rootClientId, blockElementId, layout, order, isDragging, onDragStart, onDragEnd } ) => {
 	const className = classnames( 'editor-block-list__block-draggable', {
 		'is-visible': isDragging,
 	} );
 
 	const transferData = {
 		type: 'block',
-		fromIndex: index,
+		fromIndex: order,
 		rootClientId,
 		clientId,
 		layout,
 	};
 
 	return (
-		<Draggable className={ className } transferData={ transferData } { ...props }>
-			<div className="editor-block-list__block-draggable-inner"></div>
+		<Draggable
+			elementId={ blockElementId }
+			transferData={ transferData }
+			onDragStart={ onDragStart }
+			onDragEnd={ onDragEnd }
+		>
+			{
+				( { onDraggableStart, onDraggableEnd } ) => (
+					<div
+						className={ className }
+						onDragStart={ onDraggableStart }
+						onDragEnd={ onDraggableEnd }
+						draggable
+					>
+						<div className="editor-block-list__block-draggable-inner"></div>
+					</div> )
+			}
 		</Draggable>
 	);
-}
+};
 
 export default BlockDraggable;

packages/editor/src/components/block-list/block.js

@@ -483,14 +483,14 @@ export class BlockListBlock extends Component {
 			>
 				{ ! isPartOfMultiSelection && isMovable && (
 					<BlockDraggable
-						rootClientId={ rootClientId }
-						index={ order }
 						clientId={ clientId }
+						rootClientId={ rootClientId }
+						blockElementId={ blockElementId }
 						layout={ layout }
+						order={ order }
 						onDragStart={ this.onDragStart }
 						onDragEnd={ this.onDragEnd }
 						isDragging={ dragging }
-						elementId={ blockElementId }
 					/>
 				) }
 				{ shouldShowInsertionPoint && (