Try/add drag handle to select mode #28815
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
|
Size Change: +285 B (0%) Total Size: 1.4 MB
ℹ️ View Unchanged
|
|
Whoah. A Sunday PR, and an impressive one at that! This is what I see:
A few things come to mind, that don't necessarily have to be part of this PR, but are inspired by how good this feels:
Those are small nitpicks. But one thing I'd love to see, is that the entire block itself is the draggable area, in addition to the drag handle.
Right now when you click and drag on the block itself, nothing happens. So it seems like it could be the draggable handle instead. The primary tricky thing here is that the 2nd click on the selected block (i.e. click a selected block twice) should still go back to edit mode. So potentially we need to differentiate between ondrag and onclick, and only let the former actually drag. Nice work! 👌 |
|
The branch seems to have all the commits from the |
Yes @talldan I was testing the new nav screen design PR and branched from it. I'll fix it but I wanted it to be visible b/c it really feels nice and I was curious if I'm biased :) |
draganescu
force-pushed
the
try/add-drag-handle-to-select-mode
branch
from
3f1f494
to
e572cb5
Compare
|
@jasmussen @pablohoneyhoney would it make sense to use the dark background for the selection state the moment you press "Escape"? |
mtias
added
[Feature] Drag and Drop
Yes, that would be nice. I can probably push code directly to this branch to that effect? |
|
I made the handle dark, I think it works well:
But now that it's dark, should it be identical to the one you see when you're dragging? I.e. in this case, just show a paragraph icon and a drag handle? (I think yes) We could also increase the drop shadow while dragging, to better ambiguate between resting and lifting. What do you think Andrei? |
|
Nice!
Yes, it'd make sense to try. Could also make sense to have [icon] [drag] Label. With label disappearing when you start dragging but the icon and drag handle remaining exactly the same. |
Yes, that's probably best 👍 👍 |
|
Lovely additions @jasmussen |
|
Made the shadow droppier, and fixed a color issue with the grabby handle active state:
|
|
I am thinking of stealing from #23497 to see how that works in this slect mode context. |
draganescu
force-pushed
the
try/add-drag-handle-to-select-mode
branch
from
33debae
to
7433d19
Compare
|
My opinion is to merge this like it is and follow up with a secondary PR to explore long press drag for the blocks. Should we change the block popover (the handle in navigation mode) to look exactly like the 1st part of the block toolbar? Or is the block name better in this mode than an icon? |
If it's not a ton of work, that'd be nice. Either:
2 would be identical to lifted, 1 would probably be preferable and the label would simply disappear when you lift the block. |
|
@jasmussen I went for (1) [block-type-icon] [drag-handle] Label and also implemented the block selection button to be like the drag chip based on flex, bolted some CSS to make the transition when dragging look normal, and here it is: I updated the screen recording in the PRs description. What do you think? |
draganescu
commented
Mar 9, 2021
| z-index: z-index(".block-editor-block-list__block-selection-button"); | ||
| // Dark block UI appearance. | ||
| border-radius: $radius-block-ui; |
There was a problem hiding this comment.
The comment seems to be in the wrong place.
| .components-button.has-icon.block-selection-button_drag-handle { | ||
| cursor: grab; | ||
| padding: 0; | ||
| height: 24px; |
There was a problem hiding this comment.
I wonder why did I need to bolt these magic numbers here.
| @@ -614,28 +614,58 @@ | |||
| */ | |||
|
|
|||
| .block-editor-block-list__block-selection-button { | |||
| display: block; | |||
| display: inline-flex; | |||
| padding: 0 20px; | |||
There was a problem hiding this comment.
This was kinda too hard to get right, none of the $grid-unit options seemed to fit right "visually" with the chip once the drag started.
|
This is what I see:
Great. I think I need to push some tweaks to get some of the margin stuff right, it feels slightly off even if it isn't necessarily. I'll massage those pixels tomorrow! |
|
Alright, pushed a few things to get the metrics right:
I think this is good to land. |
gwwar
approved these changes
Mar 11, 2021
There was a problem hiding this comment.
This feels pretty nice and code looks reasonable. Was there anything in particular to focus on here for testing @draganescu ?
For other reviewers 👀 , a quick way of getting into select mode is by pressing esc while a block is focused.
selectmode.mp4
|
🔥 |
|
Nice! |
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.
Description
Advances #24092
Adds a drag handle to the block selector button when in navigation mode.
How has this been tested?
Screenshots
drag-chip-link.mp4
Types of changes
Checklist: