Drop zone: rewrite without provider #30310
Conversation
Size Change: +293 B (0%) Total Size: 1.46 MB
|
Filename | Size | Change |
---|---|---|
build/a11y/index.js |
1.14 kB | 0 B |
build/blob/index.js |
664 B | 0 B |
build/block-directory/style-rtl.css |
1 kB | 0 B |
build/block-directory/style.css |
1.01 kB | 0 B |
build/block-library/blocks/archives/editor-rtl.css |
61 B | 0 B |
build/block-library/blocks/archives/editor.css |
60 B | 0 B |
build/block-library/blocks/audio/editor-rtl.css |
58 B | 0 B |
build/block-library/blocks/audio/editor.css |
58 B | 0 B |
build/block-library/blocks/audio/style-rtl.css |
112 B | 0 B |
build/block-library/blocks/audio/style.css |
112 B | 0 B |
build/block-library/blocks/block/editor-rtl.css |
161 B | 0 B |
build/block-library/blocks/block/editor.css |
161 B | 0 B |
build/block-library/blocks/button/editor-rtl.css |
475 B | 0 B |
build/block-library/blocks/button/editor.css |
474 B | 0 B |
build/block-library/blocks/button/style-rtl.css |
503 B | 0 B |
build/block-library/blocks/button/style.css |
503 B | 0 B |
build/block-library/blocks/buttons/editor-rtl.css |
315 B | 0 B |
build/block-library/blocks/buttons/editor.css |
315 B | 0 B |
build/block-library/blocks/buttons/style-rtl.css |
368 B | 0 B |
build/block-library/blocks/buttons/style.css |
368 B | 0 B |
build/block-library/blocks/calendar/style-rtl.css |
208 B | 0 B |
build/block-library/blocks/calendar/style.css |
208 B | 0 B |
build/block-library/blocks/categories/editor-rtl.css |
84 B | 0 B |
build/block-library/blocks/categories/editor.css |
83 B | 0 B |
build/block-library/blocks/categories/style-rtl.css |
79 B | 0 B |
build/block-library/blocks/categories/style.css |
79 B | 0 B |
build/block-library/blocks/code/style-rtl.css |
90 B | 0 B |
build/block-library/blocks/code/style.css |
90 B | 0 B |
build/block-library/blocks/columns/editor-rtl.css |
190 B | 0 B |
build/block-library/blocks/columns/editor.css |
190 B | 0 B |
build/block-library/blocks/columns/style-rtl.css |
436 B | 0 B |
build/block-library/blocks/columns/style.css |
435 B | 0 B |
build/block-library/blocks/cover/editor-rtl.css |
605 B | 0 B |
build/block-library/blocks/cover/editor.css |
605 B | 0 B |
build/block-library/blocks/cover/style-rtl.css |
1.23 kB | 0 B |
build/block-library/blocks/cover/style.css |
1.23 kB | 0 B |
build/block-library/blocks/embed/editor-rtl.css |
486 B | 0 B |
build/block-library/blocks/embed/editor.css |
486 B | 0 B |
build/block-library/blocks/embed/style-rtl.css |
401 B | 0 B |
build/block-library/blocks/embed/style.css |
400 B | 0 B |
build/block-library/blocks/file/editor-rtl.css |
175 B | 0 B |
build/block-library/blocks/file/editor.css |
174 B | 0 B |
build/block-library/blocks/file/style-rtl.css |
248 B | 0 B |
build/block-library/blocks/file/style.css |
248 B | 0 B |
build/block-library/blocks/freeform/editor-rtl.css |
2.44 kB | 0 B |
build/block-library/blocks/freeform/editor.css |
2.44 kB | 0 B |
build/block-library/blocks/gallery/editor-rtl.css |
704 B | 0 B |
build/block-library/blocks/gallery/editor.css |
705 B | 0 B |
build/block-library/blocks/gallery/style-rtl.css |
1.09 kB | 0 B |
build/block-library/blocks/gallery/style.css |
1.09 kB | 0 B |
build/block-library/blocks/group/editor-rtl.css |
160 B | 0 B |
build/block-library/blocks/group/editor.css |
160 B | 0 B |
build/block-library/blocks/group/style-rtl.css |
57 B | 0 B |
build/block-library/blocks/group/style.css |
57 B | 0 B |
build/block-library/blocks/heading/editor-rtl.css |
129 B | 0 B |
build/block-library/blocks/heading/editor.css |
129 B | 0 B |
build/block-library/blocks/heading/style-rtl.css |
76 B | 0 B |
build/block-library/blocks/heading/style.css |
76 B | 0 B |
build/block-library/blocks/html/editor-rtl.css |
281 B | 0 B |
build/block-library/blocks/html/editor.css |
281 B | 0 B |
build/block-library/blocks/image/editor-rtl.css |
717 B | 0 B |
build/block-library/blocks/image/editor.css |
716 B | 0 B |
build/block-library/blocks/image/style-rtl.css |
476 B | 0 B |
build/block-library/blocks/image/style.css |
478 B | 0 B |
build/block-library/blocks/latest-comments/style-rtl.css |
281 B | 0 B |
build/block-library/blocks/latest-comments/style.css |
282 B | 0 B |
build/block-library/blocks/latest-posts/editor-rtl.css |
137 B | 0 B |
build/block-library/blocks/latest-posts/editor.css |
137 B | 0 B |
build/block-library/blocks/latest-posts/style-rtl.css |
523 B | 0 B |
build/block-library/blocks/latest-posts/style.css |
522 B | 0 B |
build/block-library/blocks/legacy-widget/editor-rtl.css |
398 B | 0 B |
build/block-library/blocks/legacy-widget/editor.css |
399 B | 0 B |
build/block-library/blocks/list/style-rtl.css |
63 B | 0 B |
build/block-library/blocks/list/style.css |
63 B | 0 B |
build/block-library/blocks/media-text/editor-rtl.css |
191 B | 0 B |
build/block-library/blocks/media-text/editor.css |
191 B | 0 B |
build/block-library/blocks/media-text/style-rtl.css |
535 B | 0 B |
build/block-library/blocks/media-text/style.css |
532 B | 0 B |
build/block-library/blocks/more/editor-rtl.css |
434 B | 0 B |
build/block-library/blocks/more/editor.css |
434 B | 0 B |
build/block-library/blocks/navigation-link/editor-rtl.css |
597 B | 0 B |
build/block-library/blocks/navigation-link/editor.css |
597 B | 0 B |
build/block-library/blocks/navigation-link/style-rtl.css |
1.07 kB | 0 B |
build/block-library/blocks/navigation-link/style.css |
1.07 kB | 0 B |
build/block-library/blocks/navigation/editor-rtl.css |
1.24 kB | 0 B |
build/block-library/blocks/navigation/editor.css |
1.24 kB | 0 B |
build/block-library/blocks/navigation/style-rtl.css |
272 B | 0 B |
build/block-library/blocks/navigation/style.css |
271 B | 0 B |
build/block-library/blocks/nextpage/editor-rtl.css |
395 B | 0 B |
build/block-library/blocks/nextpage/editor.css |
395 B | 0 B |
build/block-library/blocks/page-list/editor-rtl.css |
239 B | 0 B |
build/block-library/blocks/page-list/editor.css |
240 B | 0 B |
build/block-library/blocks/page-list/style-rtl.css |
167 B | 0 B |
build/block-library/blocks/page-list/style.css |
167 B | 0 B |
build/block-library/blocks/paragraph/editor-rtl.css |
157 B | 0 B |
build/block-library/blocks/paragraph/editor.css |
157 B | 0 B |
build/block-library/blocks/paragraph/style-rtl.css |
247 B | 0 B |
build/block-library/blocks/paragraph/style.css |
248 B | 0 B |
build/block-library/blocks/post-author/editor-rtl.css |
209 B | 0 B |
build/block-library/blocks/post-author/editor.css |
209 B | 0 B |
build/block-library/blocks/post-author/style-rtl.css |
183 B | 0 B |
build/block-library/blocks/post-author/style.css |
184 B | 0 B |
build/block-library/blocks/post-comments-form/style-rtl.css |
250 B | 0 B |
build/block-library/blocks/post-comments-form/style.css |
250 B | 0 B |
build/block-library/blocks/post-content/editor-rtl.css |
139 B | 0 B |
build/block-library/blocks/post-content/editor.css |
139 B | 0 B |
build/block-library/blocks/post-excerpt/editor-rtl.css |
73 B | 0 B |
build/block-library/blocks/post-excerpt/editor.css |
73 B | 0 B |
build/block-library/blocks/post-excerpt/style-rtl.css |
69 B | 0 B |
build/block-library/blocks/post-excerpt/style.css |
69 B | 0 B |
build/block-library/blocks/post-featured-image/editor-rtl.css |
338 B | 0 B |
build/block-library/blocks/post-featured-image/editor.css |
338 B | 0 B |
build/block-library/blocks/post-featured-image/style-rtl.css |
100 B | 0 B |
build/block-library/blocks/post-featured-image/style.css |
100 B | 0 B |
build/block-library/blocks/post-title/style-rtl.css |
60 B | 0 B |
build/block-library/blocks/post-title/style.css |
60 B | 0 B |
build/block-library/blocks/preformatted/style-rtl.css |
103 B | 0 B |
build/block-library/blocks/preformatted/style.css |
103 B | 0 B |
build/block-library/blocks/pullquote/editor-rtl.css |
183 B | 0 B |
build/block-library/blocks/pullquote/editor.css |
183 B | 0 B |
build/block-library/blocks/pullquote/style-rtl.css |
318 B | 0 B |
build/block-library/blocks/pullquote/style.css |
318 B | 0 B |
build/block-library/blocks/query-loop/editor-rtl.css |
83 B | 0 B |
build/block-library/blocks/query-loop/editor.css |
82 B | 0 B |
build/block-library/blocks/query-loop/style-rtl.css |
315 B | 0 B |
build/block-library/blocks/query-loop/style.css |
317 B | 0 B |
build/block-library/blocks/query-pagination-numbers/editor-rtl.css |
122 B | 0 B |
build/block-library/blocks/query-pagination-numbers/editor.css |
121 B | 0 B |
build/block-library/blocks/query-pagination/editor-rtl.css |
270 B | 0 B |
build/block-library/blocks/query-pagination/editor.css |
262 B | 0 B |
build/block-library/blocks/query-pagination/style-rtl.css |
168 B | 0 B |
build/block-library/blocks/query-pagination/style.css |
168 B | 0 B |
build/block-library/blocks/query-title/editor-rtl.css |
86 B | 0 B |
build/block-library/blocks/query-title/editor.css |
86 B | 0 B |
build/block-library/blocks/query/editor-rtl.css |
810 B | 0 B |
build/block-library/blocks/query/editor.css |
809 B | 0 B |
build/block-library/blocks/quote/style-rtl.css |
169 B | 0 B |
build/block-library/blocks/quote/style.css |
169 B | 0 B |
build/block-library/blocks/rss/editor-rtl.css |
201 B | 0 B |
build/block-library/blocks/rss/editor.css |
202 B | 0 B |
build/block-library/blocks/rss/style-rtl.css |
290 B | 0 B |
build/block-library/blocks/rss/style.css |
290 B | 0 B |
build/block-library/blocks/search/editor-rtl.css |
189 B | 0 B |
build/block-library/blocks/search/editor.css |
189 B | 0 B |
build/block-library/blocks/search/style-rtl.css |
359 B | 0 B |
build/block-library/blocks/search/style.css |
362 B | 0 B |
build/block-library/blocks/separator/editor-rtl.css |
99 B | 0 B |
build/block-library/blocks/separator/editor.css |
99 B | 0 B |
build/block-library/blocks/separator/style-rtl.css |
251 B | 0 B |
build/block-library/blocks/separator/style.css |
251 B | 0 B |
build/block-library/blocks/shortcode/editor-rtl.css |
512 B | 0 B |
build/block-library/blocks/shortcode/editor.css |
512 B | 0 B |
build/block-library/blocks/site-logo/editor-rtl.css |
440 B | 0 B |
build/block-library/blocks/site-logo/editor.css |
441 B | 0 B |
build/block-library/blocks/site-logo/style-rtl.css |
154 B | 0 B |
build/block-library/blocks/site-logo/style.css |
154 B | 0 B |
build/block-library/blocks/social-link/editor-rtl.css |
164 B | 0 B |
build/block-library/blocks/social-link/editor.css |
165 B | 0 B |
build/block-library/blocks/social-links/editor-rtl.css |
796 B | 0 B |
build/block-library/blocks/social-links/editor.css |
795 B | 0 B |
build/block-library/blocks/social-links/style-rtl.css |
1.32 kB | 0 B |
build/block-library/blocks/social-links/style.css |
1.33 kB | 0 B |
build/block-library/blocks/spacer/editor-rtl.css |
308 B | 0 B |
build/block-library/blocks/spacer/editor.css |
308 B | 0 B |
build/block-library/blocks/spacer/style-rtl.css |
48 B | 0 B |
build/block-library/blocks/spacer/style.css |
48 B | 0 B |
build/block-library/blocks/table/editor-rtl.css |
478 B | 0 B |
build/block-library/blocks/table/editor.css |
478 B | 0 B |
build/block-library/blocks/table/style-rtl.css |
402 B | 0 B |
build/block-library/blocks/table/style.css |
402 B | 0 B |
build/block-library/blocks/tag-cloud/editor-rtl.css |
118 B | 0 B |
build/block-library/blocks/tag-cloud/editor.css |
118 B | 0 B |
build/block-library/blocks/tag-cloud/style-rtl.css |
94 B | 0 B |
build/block-library/blocks/tag-cloud/style.css |
94 B | 0 B |
build/block-library/blocks/template-part/editor-rtl.css |
552 B | 0 B |
build/block-library/blocks/template-part/editor.css |
551 B | 0 B |
build/block-library/blocks/term-description/editor-rtl.css |
90 B | 0 B |
build/block-library/blocks/term-description/editor.css |
90 B | 0 B |
build/block-library/blocks/text-columns/editor-rtl.css |
95 B | 0 B |
build/block-library/blocks/text-columns/editor.css |
95 B | 0 B |
build/block-library/blocks/text-columns/style-rtl.css |
166 B | 0 B |
build/block-library/blocks/text-columns/style.css |
166 B | 0 B |
build/block-library/blocks/verse/style-rtl.css |
87 B | 0 B |
build/block-library/blocks/verse/style.css |
87 B | 0 B |
build/block-library/blocks/video/editor-rtl.css |
568 B | 0 B |
build/block-library/blocks/video/editor.css |
569 B | 0 B |
build/block-library/blocks/video/style-rtl.css |
173 B | 0 B |
build/block-library/blocks/video/style.css |
173 B | 0 B |
build/block-library/common-rtl.css |
1.31 kB | 0 B |
build/block-library/common.css |
1.31 kB | 0 B |
build/block-library/editor-rtl.css |
9.79 kB | 0 B |
build/block-library/editor.css |
9.78 kB | 0 B |
build/block-library/reset-rtl.css |
502 B | 0 B |
build/block-library/reset.css |
503 B | 0 B |
build/block-library/style-rtl.css |
9.43 kB | 0 B |
build/block-library/style.css |
9.44 kB | 0 B |
build/block-library/theme-rtl.css |
692 B | 0 B |
build/block-library/theme.css |
693 B | 0 B |
build/block-serialization-spec-parser/index.js |
3.06 kB | 0 B |
build/customize-widgets/index.js |
8.01 kB | 0 B |
build/customize-widgets/style-rtl.css |
630 B | 0 B |
build/customize-widgets/style.css |
631 B | 0 B |
build/date/index.js |
31.9 kB | 0 B |
build/deprecated/index.js |
787 B | 0 B |
build/dom-ready/index.js |
576 B | 0 B |
build/edit-navigation/style-rtl.css |
2.86 kB | 0 B |
build/edit-navigation/style.css |
2.86 kB | 0 B |
build/edit-post/classic-rtl.css |
454 B | 0 B |
build/edit-post/classic.css |
454 B | 0 B |
build/edit-post/style-rtl.css |
6.99 kB | 0 B |
build/edit-post/style.css |
6.98 kB | 0 B |
build/edit-site/style-rtl.css |
4.9 kB | 0 B |
build/edit-site/style.css |
4.89 kB | 0 B |
build/edit-widgets/style-rtl.css |
2.97 kB | 0 B |
build/edit-widgets/style.css |
2.98 kB | 0 B |
build/editor/style-rtl.css |
3.92 kB | 0 B |
build/editor/style.css |
3.92 kB | 0 B |
build/escape-html/index.js |
735 B | 0 B |
build/format-library/style-rtl.css |
637 B | 0 B |
build/format-library/style.css |
639 B | 0 B |
build/hooks/index.js |
2.28 kB | 0 B |
build/html-entities/index.js |
623 B | 0 B |
build/is-shallow-equal/index.js |
699 B | 0 B |
build/list-reusable-blocks/style-rtl.css |
629 B | 0 B |
build/list-reusable-blocks/style.css |
628 B | 0 B |
build/nux/style-rtl.css |
731 B | 0 B |
build/nux/style.css |
727 B | 0 B |
build/priority-queue/index.js |
791 B | 0 B |
build/redux-routine/index.js |
2.83 kB | 0 B |
build/reusable-blocks/style-rtl.css |
225 B | 0 B |
build/reusable-blocks/style.css |
225 B | 0 B |
build/shortcode/index.js |
1.7 kB | 0 B |
build/token-list/index.js |
1.27 kB | 0 B |
build/warning/index.js |
1.14 kB | 0 B |
build/wordcount/index.js |
1.22 kB | 0 B |
This is very cool. I a have a few questions :) |
@@ -233,7 +233,9 @@ function InsertionPointPopover( { | |||
tabIndex={ -1 } | |||
onClick={ onClick } | |||
onFocus={ onFocus } | |||
className={ className } | |||
className={ classnames( className, { | |||
'is-with-inserter': showInsertionPointInserter, |
youknowriad
Mar 31, 2021
Contributor
Why is this needed?
Why is this needed?
ellatrix
Mar 31, 2021
Author
Member
Good question. :) It is needed because I removed pointer events on the inserter indicator, which will otherwise "catch" events when it appears between blocks. This worked fine previously because we were tracking dragover
events globally and we could still determine a position.
Now, the is-with-inserter
class is added so we can re-enable pointer events for the click area in the dead zone between blocks and the inserter button. This is only necessary when the insertion point includes an inserter button, which it does not in case of e.g. drag and drop.
Good question. :) It is needed because I removed pointer events on the inserter indicator, which will otherwise "catch" events when it appears between blocks. This worked fine previously because we were tracking dragover
events globally and we could still determine a position.
Now, the is-with-inserter
class is added so we can re-enable pointer events for the click area in the dead zone between blocks and the inserter button. This is only necessary when the insertion point includes an inserter button, which it does not in case of e.g. drag and drop.
...editor/src/components/block-navigation/use-block-navigation-drop-zone.js
Outdated
Show resolved
Hide resolved
The API is much nicer, awesome work. In testing I noticed dropping into navigation block submenus no longer works, the submenus don't become visible when dragging and hovering over a link block: gutenberg/packages/block-library/src/navigation-link/edit.js Lines 61 to 101 in f98f2d2 A quick look and I couldn't see why. |
if ( position ) { | ||
const blockElements = Array.from( element.current.children ); | ||
|
||
onDragOver( event ) { |
talldan
Apr 1, 2021
Contributor
Much nicer API 👍
Much nicer API
ellatrix
Apr 1, 2021
Author
Member
I'd still like to make some improvements, but I cannot do them without making the PR bigger to rewrite the drop handlers. I'm excited to do a follow-up, but let's see this through piece by piece. 😅
I'd still like to make some improvements, but I cannot do them without making the PR bigger to rewrite the drop handlers. I'm excited to do a follow-up, but let's see this through piece by piece.
}, 200 ); | ||
|
||
function onDragOver( event ) { | ||
// Only call onDragOver for the innermost hovered drag zones. |
talldan
Apr 1, 2021
Contributor
I was wondering how this would be replicated from the previous way it worked 😄
I was wondering how this would be replicated from the previous way it worked
ellatrix
Apr 1, 2021
Author
Member
Yeah, this is so much simpler than checking element rectangles. :)
Yeah, this is so much simpler than checking element rectangles. :)
I'll have a look. The drop event was previously also stopped from propagating, but I actually don't see why this is necessary if we check |
@talldan I fixed the issue. I had disabled pointer events on children of a drop zone, but we can do it without. |
Sorry it took so long for an approval. I tested again and this is working well Just some conflicts to sort out, but hopefully that's straightforward. |
Thanks a lot for reviewing everyone! |
* Drop zone: rewrite without provider * Remove providers * Use ref callback * Simplify * Fix drag start * Fix navigator * Fix dead zone writing flow * Fix e2e test * Address feedback * Avoid disabling pointer events and treat throttling as an implementation detail * Use dragenter for detecting the start of dragging * Move useDropZone to compose
ellatrix commentedMar 27, 2021
•
edited
Description
This PR is best reviewed without whitespace changes.
Currently, drag and drop is controlled by a provider that keeps track of all the drop zone, checks to see which one is being dragged over and updates state for that drop zone with position info.
This seems all rather complex (and a source of many headaches) and I see no good reason for each drop zone to not manage its own state and events properly.
I've also removed storing position coordinates to state and instead using a callback to signal the position to the drop zone implementor, which can set its own state based on the coordinates. This way, we don't need to continuously set state for position changes but only for data derived from it.
How has this been tested?
Test drag and drop of blocks in the editor, inner blocks, and placeholders.
Screenshots
Types of changes
Checklist:
*.native.js
files for terms that need renaming or removal).