Show movers next to block switcher #22673
Conversation
|
Size Change: +497 B (0%) Total Size: 1.13 MB
ℹ️ View Unchanged
|
|
The recently-introduced parent-selector button now feels a bit more awkward since the switcher + movers are a different shape. It also feels a bit weirder since the movers are no longer sticking out on the left. That could be just a matter of needing to get used to it, though.
The arrows appear off-center when focused, which feels just a bit weird. Either the arrows need to be moved a bit to the right, or the buttons need to become a bit wider; I'd prefer the latter. It's now really odd that you can drag the block from either of the mover buttons, but not the block switcher/icon. I think the drag area needs to be extended to include that button. Overall, I like how the movers are more clearly surfaced, and the button order makes more sense. I think there needs to be a bit more space between the block icon and the movers, though. |
I didn't have that thought until you mentioned it, and even now I don't think too much of it, because just like popovers extending from the toolbar it's made of the same material. I did notice it's 50x50 instead of 48x48 though, which I'll probably patch right up :)
Good thoughts, I'll try and push some polish today. |
|
Pushed polish to the parent selector, and to focus, and re-balanced the icons a little bit:
|
|
Pushed a bunch of polish. It's feeling good:
I still need to polish drag and drop, and to retire unused functions. But I'm marking this as ready for review to get some feedback. |
jasmussen
requested review from
ellatrix,
ajitbohra,
chrisvanpatten and
jorgefilipecosta
as code owners
|
In 571e077 I polished drag and drop:
I think it works well, but I expect you'll have thoughts on this. Lay them on me. |
|
The size of the block switcher in the DOM is a bit weird: (Since the movers partially overlap it, the actual clickable area is smaller like you would expect.) Also, I still think you should also be able to drag from the switcher. It's still weird that you can drag from only the right half of the rectangle that makes up the switcher + mover area. Additionally, I'm not sure how I feel about the black drag handle thing. I don't dislike it, but I wonder if maybe it's a bit too heavy? Anyway, I don't think it should be a blocker for this PR. The spacing adjustments really helped a lot, and I'm already used to how it feels with the parent-selector. Functionality-wise, I think this is very close to ready! |
|
Thank you Zeb, good feedback. The changes to the button size and such I agree need to happen but they require a little tinkering that I thought I'd postpone until there's some agreement on a path forward (which appears to be forming!) Also agree on the draggable area. The new dark handle, I know it's a big change, but if you're able to compare this with master or even 5.4, I'm actually liking it a fair bit. It helps indicate where you need to drop the block in order to actually move it. It's also not "muddy" in that it has a solid background, whereas the block you're moving does not (because we can't know which color to apply there). I'm definitely open to more thoughts on that feeling, so I hear you, and it's why I opened the PR even while not totally baked yet. |
ZebulanStanphill
added
the
General Interface
|
When there is only one block on the page the arrow icons are missing leaving only an empty gap. |
jasmussen
force-pushed
the
update/movers-to-toolbar
branch
from
571e077
to
bcc2798
Compare
|
Good catch, Paal. I'll show the movers as disabled when they can't be moved, that feels appropriate. |
|
Pushed a fix for the "just one block" case. As it turns out, there are a few heuristics for when the block mover should not be shown, including when blocks can't be moved. So instead of unhiding, I just restored the border in that case:
|
jasmussen
force-pushed
the
update/movers-to-toolbar
branch
2 times, most recently
from
f859ff1
to
6924547
Compare
Props @zebulandstanphill Co-authored-by: Zebulan Stanphill <zebulanstanphill@protonmail.com>
jasmussen
force-pushed
the
update/movers-to-toolbar
branch
from
ac74d0b
to
27e548a
Compare
| @@ -12,7 +12,7 @@ import { withSafeTimeout } from '@wordpress/compose'; | |||
| const dragImageClass = 'components-draggable__invisible-drag-image'; | |||
| const cloneWrapperClass = 'components-draggable__clone'; | |||
| const cloneHeightTransformationBreakpoint = 700; | |||
| const clonePadding = 20; | |||
There was a problem hiding this comment.
These 20 appear to have been picked at random, back in the day, and almost certainly due to there being side UI and margins. Because there is neither anymore, I've now set this to zero, instead of cmopensating for it.
| background: transparent; | ||
| pointer-events: none; | ||
| z-index: z-index(".components-draggable__clone"); | ||
|
|
||
| > * { | ||
| opacity: 0.8; | ||
| // This needs specificity as a theme is meant to define these by default. | ||
| margin-top: 0 !important; |
There was a problem hiding this comment.
I know that you must never use !important. Except in this case. Themes can define block margins, and when they do, the draggable clone will be as offset from the mouse cursor position as the theme margin decides. By explicitly and with high specificity setting these, this is unlikely to break.
|
In ffe28be I simplifid the on-drag handle positioning math. There was some old stuff we don't need anymore that I was able to reduce and convert to variables. Shaun I believe this fixes the discrepancies you saw. 2019:
2020:
|
|
Uh. I commented on the wrong post.. |
|
Let's get this in and I will address any followups. |
|
Like perhaps drag & drop hot zones...:) |
Can you clarify? |
|
Thinking this: There are multiple issues intertwined into each other. |
|
Ah. There's lots to explore for drag and drop still, yes! |
Fixes #21935.
One of the most important things the new UI (as detailed in #18667) did, was to unify on a single toolbar, meaning the mover control was included. This afforded a number of benefits:
The downside was a wider block toolbar, which we compensated for by hiding the mover control until you hover the block switcher. However the feedback was clear, that the basic mover control was used often enough that it should not be hidden.
This PR unhides it:
The PR takes the most simple approach discussed to unhiding it. This approach does not preclude tweaking the design further — but it can serve as a good baseline, because it is so trivial to implement. And instead of discussing what the best appraoch is ad nauseum, a PR is an excellent way to try this out, and please do try it out, don't just look at the GIFs. As I was working on this, it struck me as feeling intuitive and natural, and not in need of any more visual affordances.
Still todo: