Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Drop zone: rewrite with hooks (useDropZone) #19514

Merged
merged 14 commits into from
Jan 10, 2020
Merged

Drop zone: rewrite with hooks (useDropZone) #19514

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
b082ca6
Drop zone: rewrite with hooks
ellatrix Jan 8, 2020
413e2aa
Add missing drop zones
ellatrix Jan 8, 2020
66e6b91
Use hook for component
ellatrix Jan 8, 2020
464e660
Oops
ellatrix Jan 8, 2020
4032990
fix styling
ellatrix Jan 8, 2020
af10ad6
fix unit tests
ellatrix Jan 8, 2020
3868c40
Simplify
ellatrix Jan 8, 2020
9d3667b
Add style for appender drop target
ellatrix Jan 9, 2020
4011f05
Readd className prop
ellatrix Jan 10, 2020
5f1fc4f
Add dropzone to block list instead of block
ellatrix Jan 10, 2020
a3f8278
Add appender style
ellatrix Jan 10, 2020
b1f2474
Clean up
ellatrix Jan 10, 2020
77cd3da
Mark useDropZone unstable
ellatrix Jan 10, 2020
257a093
Remove position classes
ellatrix Jan 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
185 changes: 84 additions & 101 deletions packages/block-editor/src/components/block-drop-zone/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,23 +1,14 @@
/**
* External dependencies
*/
import classnames from 'classnames';

/**
* WordPress dependencies
*/
import {
DropZone,
withFilters,
} from '@wordpress/components';
import { __unstableUseDropZone as useDropZone } from '@wordpress/components';
import {
pasteHandler,
getBlockTransforms,
findTransform,
} from '@wordpress/blocks';
import { Component } from '@wordpress/element';
import { withDispatch, withSelect } from '@wordpress/data';
import { compose } from '@wordpress/compose';
import { useDispatch, useSelect } from '@wordpress/data';
import { useEffect, useState, useCallback } from '@wordpress/element';

const parseDropEvent = ( event ) => {
let result = {
Expand All @@ -40,49 +31,63 @@ const parseDropEvent = ( event ) => {
return result;
};

class BlockDropZone extends Component {
constructor() {
super( ...arguments );

this.onFilesDrop = this.onFilesDrop.bind( this );
this.onHTMLDrop = this.onHTMLDrop.bind( this );
this.onDrop = this.onDrop.bind( this );
}
export default function useBlockDropZone( { element, rootClientId } ) {
const [ clientId, setClientId ] = useState( null );

getInsertIndex( position ) {
const { clientId, rootClientId, getBlockIndex } = this.props;
if ( clientId !== undefined ) {
const index = getBlockIndex( clientId, rootClientId );
return ( position && position.y === 'top' ) ? index : index + 1;
}
function selector( select ) {
const {
getBlockIndex,
getClientIdsOfDescendants,
getSettings,
getTemplateLock,
} = select( 'core/block-editor' );
return {
getBlockIndex,
blockIndex: getBlockIndex( clientId, rootClientId ),
getClientIdsOfDescendants,
hasUploadPermissions: !! getSettings().mediaUpload,
isLockedAll: getTemplateLock( rootClientId ) === 'all',
};
}

onFilesDrop( files, position ) {
if ( ! this.props.hasUploadPermissions ) {
const {
getBlockIndex,
blockIndex,
getClientIdsOfDescendants,
hasUploadPermissions,
isLockedAll,
} = useSelect( selector, [ rootClientId, clientId ] );
const {
insertBlocks,
updateBlockAttributes,
moveBlockToPosition,
} = useDispatch( 'core/block-editor' );

const onFilesDrop = useCallback( ( files ) => {
if ( ! hasUploadPermissions ) {
return;
}

const transformation = findTransform(
getBlockTransforms( 'from' ),
( transform ) => transform.type === 'files' && transform.isMatch( files )
);

if ( transformation ) {
const insertIndex = this.getInsertIndex( position );
const blocks = transformation.transform( files, this.props.updateBlockAttributes );
this.props.insertBlocks( blocks, insertIndex );
const blocks = transformation.transform( files, updateBlockAttributes );
insertBlocks( blocks, blockIndex, rootClientId );
}
}
}, [ hasUploadPermissions, updateBlockAttributes, insertBlocks, blockIndex, rootClientId ] );

onHTMLDrop( HTML, position ) {
const onHTMLDrop = useCallback( ( HTML ) => {
const blocks = pasteHandler( { HTML, mode: 'BLOCKS' } );

if ( blocks.length ) {
this.props.insertBlocks( blocks, this.getInsertIndex( position ) );
insertBlocks( blocks, blockIndex, rootClientId );
}
}
}, [ insertBlocks, blockIndex, rootClientId ] );

onDrop( event, position ) {
const { rootClientId: dstRootClientId, clientId: dstClientId, getClientIdsOfDescendants, getBlockIndex } = this.props;
const onDrop = useCallback( ( event ) => {
const { srcRootClientId, srcClientId, srcIndex, type } = parseDropEvent( event );

const isBlockDropType = ( dropType ) => dropType === 'block';
Expand All @@ -95,75 +100,53 @@ class BlockDropZone extends Component {
const isSrcBlockAnAncestorOfDstBlock = ( src, dst ) => getClientIdsOfDescendants( [ src ] ).some( ( id ) => id === dst );

if ( ! isBlockDropType( type ) ||
isSameBlock( srcClientId, dstClientId ) ||
isSrcBlockAnAncestorOfDstBlock( srcClientId, dstClientId || dstRootClientId ) ) {
isSameBlock( srcClientId, clientId ) ||
isSrcBlockAnAncestorOfDstBlock( srcClientId, clientId || rootClientId ) ) {
return;
}

const dstIndex = dstClientId ? getBlockIndex( dstClientId, dstRootClientId ) : undefined;
const positionIndex = this.getInsertIndex( position );
const dstIndex = clientId ? getBlockIndex( clientId, rootClientId ) : undefined;
const positionIndex = blockIndex;
// If the block is kept at the same level and moved downwards,
// subtract to account for blocks shifting upward to occupy its old position.
const insertIndex = dstIndex && srcIndex < dstIndex && isSameLevel( srcRootClientId, dstRootClientId ) ? positionIndex - 1 : positionIndex;
this.props.moveBlockToPosition( srcClientId, srcRootClientId, insertIndex );
}

render() {
const { hasUploadPermissions, isLockedAll } = this.props;
if ( isLockedAll ) {
return null;
const insertIndex = dstIndex && srcIndex < dstIndex && isSameLevel( srcRootClientId, rootClientId ) ? positionIndex - 1 : positionIndex;
moveBlockToPosition( srcClientId, srcRootClientId, rootClientId, insertIndex );
}, [ getClientIdsOfDescendants, getBlockIndex, clientId, blockIndex, moveBlockToPosition, rootClientId ] );

const { position } = useDropZone( {
element,
onFilesDrop,
onHTMLDrop,
onDrop,
isDisabled: isLockedAll,
withPosition: true,
} );

useEffect( () => {
if ( position ) {
const { y } = position;
const rect = element.current.getBoundingClientRect();

const offset = y - rect.top;
const target = Array.from( element.current.children ).find( ( blockEl ) => {
return blockEl.offsetTop + ( blockEl.offsetHeight / 2 ) > offset;
} );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to take into consideration the blocks that are laid out horizontally (I'm not sure it was the case before)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not possible in master right now, so I'd prefer to tackle this in a separate PR to keep this small and contained.


if ( ! target ) {
return;
}

const targetClientId = target.id.slice( 'block-'.length );

if ( ! targetClientId ) {
return;
}

setClientId( targetClientId );
}
const index = this.getInsertIndex();
const isAppender = index === undefined;
return (
<DropZone
className={ classnames( 'block-editor-block-drop-zone', {
'is-appender': isAppender,
} ) }
onHTMLDrop={ this.onHTMLDrop }
onDrop={ this.onDrop }
onFilesDrop={ hasUploadPermissions ? this.onFilesDrop : undefined }
/>
);
}, [ position ] );

if ( position ) {
return clientId;
}
}

export default compose(
withDispatch( ( dispatch, ownProps ) => {
const {
insertBlocks,
updateBlockAttributes,
moveBlockToPosition,
} = dispatch( 'core/block-editor' );

return {
insertBlocks( blocks, index ) {
const { rootClientId } = ownProps;

insertBlocks( blocks, index, rootClientId );
},
updateBlockAttributes( ...args ) {
updateBlockAttributes( ...args );
},
moveBlockToPosition( srcClientId, srcRootClientId, dstIndex ) {
const { rootClientId: dstRootClientId } = ownProps;
moveBlockToPosition( srcClientId, srcRootClientId, dstRootClientId, dstIndex );
},
};
} ),
withSelect( ( select, { rootClientId } ) => {
const {
getBlockIndex,
getClientIdsOfDescendants,
getSettings,
getTemplateLock,
} = select( 'core/block-editor' );
return {
getBlockIndex,
getClientIdsOfDescendants,
hasUploadPermissions: !! getSettings().mediaUpload,
isLockedAll: getTemplateLock( rootClientId ) === 'all',
};
} ),
withFilters( 'editor.BlockDropZone' )
)( BlockDropZone );
27 changes: 0 additions & 27 deletions packages/block-editor/src/components/block-drop-zone/style.scss
View file
Open in desktop

This file was deleted.

4 changes: 3 additions & 1 deletion packages/block-editor/src/components/block-list-appender/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* External dependencies
*/
import { last } from 'lodash';
import classnames from 'classnames';

/**
* WordPress dependencies
Expand All @@ -25,6 +26,7 @@ function BlockListAppender( {
canInsertDefaultBlock,
isLocked,
renderAppender: CustomAppender,
className,
} ) {
if ( isLocked || CustomAppender === false ) {
return null;
Expand Down Expand Up @@ -68,7 +70,7 @@ function BlockListAppender( {
// Prevent the block from being selected when the appender is
// clicked.
onFocus={ stopPropagation }
className="block-list-appender"
className={ classnames( 'block-list-appender', className ) }
>
{ appender }
</div>
Expand Down
10 changes: 10 additions & 0 deletions packages/block-editor/src/components/block-list-appender/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,16 @@
}
}

.block-list-appender.is-drop-target > div::before {
content: "";
position: absolute;
right: -$block-padding;
left: -$block-padding;
top: -$block-padding;
bottom: -$block-padding;
border: 3px solid theme(primary);
}

.block-list-appender > .block-editor-inserter {
display: block;
}
13 changes: 0 additions & 13 deletions packages/block-editor/src/components/block-list/block.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import { useShortcut } from '@wordpress/keyboard-shortcuts';
* Internal dependencies
*/
import BlockEdit from '../block-edit';
import BlockDropZone from '../block-drop-zone';
import BlockInvalidWarning from './block-invalid-warning';
import BlockCrashWarning from './block-crash-warning';
import BlockCrashBoundary from './block-crash-boundary';
Expand Down Expand Up @@ -284,14 +283,6 @@ function BlockListBlock( {
isFirstMultiSelected
);

// Insertion point can only be made visible if the block is at the
// the extent of a multi-selection, or not in a multi-selection.
const shouldShowInsertionPoint = ! isMultiSelecting && (
( isPartOfMultiSelection && isFirstMultiSelected ) ||
! isPartOfMultiSelection
);

const shouldRenderDropzone = shouldShowInsertionPoint;
const isDragging = isDraggingBlocks && ( isSelected || isPartOfMultiSelection );

// The wp-block className is important for editor styles.
Expand Down Expand Up @@ -394,10 +385,6 @@ function BlockListBlock( {
animationStyle
}
>
{ shouldRenderDropzone && <BlockDropZone
clientId={ clientId }
rootClientId={ rootClientId }
/> }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main problem with this PR I think is that the dropzone is smaller than before. In master for paragraphs, its size is "37px" with this pr it is "30px". They both suffer from the same issue, there's a space (margin) between blocks where you can't drop blocks. It's is more visible in this PR because of the smaller size though.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@youknowriad I fixed the issue entirely by rewriting useBlockDropZone to work on BlockList instead of BlockListBlock. Now there are no longer "dead zones" between the blocks where you can't drop.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is cool, I'll test a bit later.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I think the error might be gone too, but let me know if it isn't. :)

{ hasAncestorCapturingToolbars && ( shouldShowContextualToolbar || isToolbarForced ) && (
// If the parent Block is set to consume toolbars of the child Blocks
// then render the child Block's toolbar into the Slot provided
Expand Down
12 changes: 12 additions & 0 deletions packages/block-editor/src/components/block-list/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import classnames from 'classnames';
* WordPress dependencies
*/
import { AsyncModeProvider, useSelect } from '@wordpress/data';
import { useRef } from '@wordpress/element';

/**
* Internal dependencies
Expand All @@ -15,6 +16,7 @@ import BlockListBlock from './block';
import BlockListAppender from '../block-list-appender';
import __experimentalBlockListFooter from '../block-list-footer';
import RootContainer from './root-container';
import useBlockDropZone from '../block-drop-zone';

/**
* If the block count exceeds the threshold, we disable the reordering animation
Expand Down Expand Up @@ -79,8 +81,16 @@ function BlockList( {

const Container = rootClientId ? 'div' : RootContainer;

const ref = useRef();

const targetClientId = useBlockDropZone( {
element: ref,
rootClientId,
} );

return (
<Container
ref={ ref }
className={ classnames(
'block-editor-block-list__layout',
className
Expand All @@ -106,13 +116,15 @@ function BlockList( {
enableAnimation={ enableAnimation }
hasSelectedUI={ uiParts.hasSelectedUI }
hasMovers={ uiParts.hasMovers }
className={ clientId === targetClientId ? 'is-drop-target' : undefined }
/>
</AsyncModeProvider>
);
} ) }
<BlockListAppender
rootClientId={ rootClientId }
renderAppender={ renderAppender }
className={ targetClientId === null ? 'is-drop-target' : undefined }
/>
<__experimentalBlockListFooter.Slot />
</Container>
Expand Down
Loading