-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Allow dragging blocks from the inserter into the canvas #27669
Conversation
Size Change: +387 B (0%) Total Size: 1.28 MB
ℹ️ View Unchanged
|
This is awesome: I like this a lot. I think the blue line that appears in the canvas could be more responsive and precise, it seems to appears some of the time, but not always. I'm almost sure that's a separate issue and not to be solved by this PR, but nevertheless it becomes slightly more of a priority as this feature lands: The other part is that we really need the grabby-hand cursor to indicate draggability. Here's the block: Can we try so that hovering the block title, in this case "Heading", keeps the And one last thing, also potentially something to look at separately. The hover previews for blocks take a while to render, naturally: The effect is that the menu feels less responsive. Given how useful this drag and drop feature suddenly makes this, can we make the block preview appear, as a tooltip, after the cursor resting on the block for 0.2 or 0.4 seconds? Seems like that might make this feel a little better. |
The last time we tried this (movers), we had some push back from folks about the confusion it might create. I'm willing to give it another try though. |
I think our opportunity here is to make the icon itself, not the text, show the grabby hand. |
49c3d36
to
7722a7f
Compare
I added the icons and removed the dragging experience from the quick inserter. How does it feel? |
This is what I see: Nice! I think that works as well as I'd hoped. There's a clear drag cursor which helps discoverability, but there's also a clear click cursor. Dragging keeps the inserter open, which is great, clicking closes it upon insertion. I think this is worth trying. One thing — when dragging into the canvas, focus isn't moved, so if you set focus in a block at the bottom, then drag a block into the canvas and have it drag-scroll way to the top, once you release, you're scrolled way back to the block that has the caret. Can we prevent that? It's loosely seen in the GIF above. |
Right now when inserting blocks, we have an option called "updateSelection" which selects the inserter block. Selecting the inserted block focus the block as well and closes the inserter and focusing the block also scrolls the page to the right position. For drag and drop this boolean is set to "false" to avoid closing the inserter. I think ideally we'd split this boolean into two: "update selection" and "focus". Meaning the block get selected but the focus stays in the inseter avoiding the closing. This might also solve the scrolling issue. |
👍👍 |
Thinking more: if you insert a "columns" block and then the "columns" block get selected, the content of the inserter will empty and only keep "column" block. It seems this behavior can be confusing too and I wonder whether "selecting" the block is the right move. Maybe we should just "unselect everything"? |
If it's easy to try, seems worth it. I don't think the selected block is that sacred, but focus is. |
return; | ||
} | ||
// If the user is moving a block | ||
if ( dropType === 'block' ) { |
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.
Feels like it might be a good opportunity to rename these types. One thing is that it doesn't feel logical that the type block
doesn't have the blocks
property but inserter
does.
What do you think about moveBlocks
and insertBlocks
to more closely reflect the actions?
// If the user is inserting a block | ||
if ( dropType === 'inserter' ) { | ||
clearSelectedBlock(); | ||
insertBlocks( blocks, targetBlockIndex, targetRootClientId, false ); |
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.
The buttons block seems to have some kind of autofocus, which overrides the false
.
Because of this perhaps?
templateInsertUpdatesSelection: true, |
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.
yes, that block is acting weird, we should address it separately though because I'm not sure about the consequences of such change.
Just tested this. This seems to work well and isn't jarring. Seems totally fine to try for V1. Thank you Riad. |
7456cac
to
5de85ef
Compare
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.
This is working great, users will be very happy with this one 👍
I will the feature automatically be added to the Widgets screen Inserter as well? |
@paaljoachim yes |
This is worth highlighting in release logs :) |
closes #1511
This PR explores allowing users to drag blocks from the inserter (on the left) to the right position in the canvas.