-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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.
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
Drop zone: rewrite with hooks (useDropZone) #19514
Changes from all commits
b082ca6
413e2aa
66e6b91
464e660
4032990
af10ad6
3868c40
9d3667b
4011f05
5f1fc4f
a3f8278
b1f2474
77cd3da
257a093
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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'; | ||
|
@@ -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. | ||
|
@@ -394,10 +385,6 @@ function BlockListBlock( { | |
animationStyle | ||
} | ||
> | ||
{ shouldRenderDropzone && <BlockDropZone | ||
clientId={ clientId } | ||
rootClientId={ rootClientId } | ||
/> } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @youknowriad I fixed the issue entirely by rewriting There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is cool, I'll test a bit later. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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.