Block editor: try direct drag (outside text editable) #67305
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ellatrix
added
[Type] Enhancement
ellatrix
changed the title
|
Size Change: +853 B (+0.05%) Total Size: 1.83 MB
ℹ️ View Unchanged
|
|
In my testing this doesn't work in Safari for the moment. It works on Chrome. A couple of notes:
|
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Another thing I noticed is that for image block placeholders", it only works after you select the block. It would be better if we can trigger it without having to select the block first. |
|
@youknowriad Fixed the issue in Safari, which also fixes the placeholder issue I think. |
|
Wow, this is feeling so great :) |
youknowriad
reviewed
Nov 26, 2024
There was a problem hiding this comment.
I noticed that when you have a multi-selection and start dragging, it only drags the image that you clicked on and not the full multi-selection.
I'm happy for this to be considered a follow-up but here are my expectations for multi-selected blocks:
- If I click on a block (regardless of the block itself, text or not), Ideally the whole multi-selection moves.
- If I click on an image outside the multi selection, only that image is dragged.
packages/block-editor/src/components/block-draggable/content.scss
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/block-list/use-block-props/index.js
Outdated
Show resolved
Hide resolved
.../block-editor/src/components/block-list/use-block-props/use-selected-block-event-handlers.js
Show resolved
Hide resolved
|
Do you think this drag/drop behavior is e2e testable. It would be good to solidify it. |
|
This is grea!! Sometimes it's very hard to figure out what is going on. In the vide below I try to move things between columns and it's unclear why eventually works and then does not work again. drag-direct.mp4Also images can be dragged and dropped on top of other images and nothing happens. Also:
|
|
@draganescu Given how this is implemented, my guess is that all of the "dropping" issues you're having in your videos have nothing to do with this PR and may already be present in trunk. Did you try there using the chip? |
|
Yes, so I would compare what happens when you drag using the block toolbar, it may be that nothing happens then too. The drag transfer from this should be 100% the same, so would be good to know. |
I had not considered multi block selection yet. I might disable that for now |
|
There are issues not caused by the implementation but by the new UX
|
Can you show it? What do you have selected? |
both are in the video above. |
|
@draganescu I don't see where it's dragging a whole column? The column that the image was in remained in place? |
This is fixed, changed it to the drag chip. |
ellatrix
force-pushed
the
try/direct-drag
branch
from
0ff13ca to
d89e774
Compare
ellatrix
commented
Nov 27, 2024
| @@ -5,10 +5,6 @@ | |||
| opacity: 0.05 !important; | |||
| border-radius: $radius-small !important; | |||
|
|
|||
| // Disabling pointer events during the drag event is necessary, | |||
| // lest the block might affect your drag operation. | |||
| pointer-events: none !important; | |||
There was a problem hiding this comment.
@jasmussen You added this a while back in #33950, but not sure why. I'm guessing to not allow dragging within the dragged block's inner blocks. I've fixed this in the PR.
There was a problem hiding this comment.
We can't do this anymore because pointer-events will end drag immediately.
There was a problem hiding this comment.
When you drag a block from A to B, you don't actually move that block. A copy of the block is made, and shown near your cursor, the block you're intending to move becomes a "gray slot", and only when you release the mouse button, is the actual block moved, and the copy you were dragging destroyed.
I'm pretty sure that's the reason I set pointer-events to none in that PR: if the block you are moving has other inner blocks, or other interactive elements (I guess ondragover related ones), then even though it shows up as a gray slot, it's still I guess a bit interactive.
If you've solved that, then certainly go forward. Pointer events is always a poor CSS propety to tinker with.
There was a problem hiding this comment.
has other inner blocks, or other interactive elements
Yes, I added a different solution for inner blocks. Also left it for frames, which would completely capture pointer event into another document.
More testing:drag-direct2.mp4In the video:
|
|
@draganescu Yeah, there's a few issues in FF I need to fix |
youknowriad
reviewed
Nov 27, 2024
youknowriad
reviewed
Nov 27, 2024
|
This should be fixed btw
|
Fixed |
ellatrix
changed the title
ellatrix
force-pushed
the
try/direct-drag
branch
from
8d8b40b to
76f61df
Compare
draganescu
approved these changes
Nov 29, 2024
|
Re-tested Chrome, Safari and Firefox too. Let's merge. I'm sure issues will pop up, but I'm confident that we can fix them quickly. |
|
Thanks for your work on this. This is really great. |
im3dabasia
pushed a commit
to im3dabasia/gutenberg
that referenced
this pull request
Dec 4, 2024
Co-authored-by: ellatrix <ellatrix@git.wordpress.org> Co-authored-by: youknowriad <youknowriad@git.wordpress.org> Co-authored-by: jasmussen <joen@git.wordpress.org> Co-authored-by: draganescu <andraganescu@git.wordpress.org>
|
@ellatrix It's really great to see this shipped 👏 I think some non-text blocks might need to opt out of this behavior. For example this Sketch block can't be used anymore because the block is dragged when a user is attempting to draw something. It's probably a very small proportion of blocks that will have this kind of issue. I'm not sure if there would be any reliable way to automatically detect them the way contenteditable is being detected. |
|
@talldan that is usually because the block is doing something wrong. The block will only be dragged if focus is on the outer block wrapper, so if a block doesn't appropriately work and is accessible, it will be dragged yes. So you can disable dragging simply by ensuring the wrapper is not focused 🙂 |
michalczaplinski
pushed a commit
that referenced
this pull request
Dec 5, 2024
Co-authored-by: ellatrix <ellatrix@git.wordpress.org> Co-authored-by: youknowriad <youknowriad@git.wordpress.org> Co-authored-by: jasmussen <joen@git.wordpress.org> Co-authored-by: draganescu <andraganescu@git.wordpress.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Why?
Because it's more intuitive.
How?
Enable built-in drag support and set the correct data.
We might want to use the current drag chip, instead of the default preview that the browser provides, not sure.
Testing Instructions
Drag an image by click and holding it directly instead of using the toolbar.
Testing Instructions for Keyboard
Screenshots or screencast