Allow drag and drop to create Rows and Galleries #56186
Conversation
tellthemachines
added
[Feature] Drag and Drop
|
Size Change: +5.9 kB (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
|
@tellthemachines Please let Accessibility Team know when this is ready for early feedback. From personal memory, this kind of sounds like an accessibility nightmare. When I used to magnify the screen and have to drag and drop, overlapping used to be a common problem. I can just see this turning into a situation where users perform actions they do not expect. Especially on zoomed in views. With that said, this is still draft but wanted to leave this note while it's still early. Thanks. |
|
That's a great point @alexstine, thanks for the reminder! I'll have a play around with this in a magnified screen and see how it works. |
|
Very cool exploration! I wonder how far potentially unexpected behaviour could be mitigated by having a live preview of what's going to happen prior to a user letting go of the mouse button? So if a user goes to drag over a block and it'll turn it into a Row block, it shows what that'll look like before letting go. Another thing that could potentially help there is if only a particular part of the block you're dragging over activated the behaviour where it switches to a row — e.g. if you're hovering over the right edge of a paragraph block then it'll switch to Row and add to the right, but not if you're just dragging over the middle of the block. That's just a couple of quick thoughts / first impressions, though. Great idea playing around with this one! |
tellthemachines
added
the
Needs Accessibility Feedback
|
Added "Needs accessibility feedback" label as per @alexstine 's suggestion; this is an early experiment but would be great to know if there may be any major issues with the approach. Preliminary testing on zoomed in view shows functionality is not available on over 200% due to the block toolbar not displaying the drag handle when it's attached to the top of the canvas (which is its default mode of display on small screen/high zoom level). I'm not sure if the same will happen when using a magnifying app; would love some pointers regarding common apps to test with! I have both Mac and Windows so could test on either. |
|
Nice! I think this could make cases like "add two images side by side" a lot more intuitive to accomplish. I think how we communicate the result needs to be tweaked, though—not with an overlay on top of the entire block. I think we need some design options there. |
|
Note that the latest commit changes the inserter type to inbetween BUT I haven't tweaked its placement yet so it's popping up in all sorts of weird places 😅 Todo: try showing inserter to the left or right of the block being dragged over depending on current hover position. |
|
Catching up let me see if I can address the "Needs Design" label. Here's a mockup:
Shown in this mockup is a header layout with a site title. In the next slide, a photo is dragged on top of this site title block, as that happens, the site title gets highlighted with a 10% blue background, and a vertical blue line appears in front of the site title. The idea here is that the site title is in a Row container, so dragging any block into it will create a horizontal layout. Dragging on the left side of the site title will show this blue drop-indicator to the left. If you drag the image to the right side of the title, it will show the drop indicator to the right. In other words, this is a drag and drop equivalent of the "Add before" or "Add after" options, with a visual indicator created by the block you're dragging on top of. Hope that helps! |
|
I've updated this draft to use the drop indicator roughly as shown in @jasmussen 's designs. I say roughly because there is a slight obstacle to this behaviour when trying to drop over blocks that aren't already in a horizontal layout: most blocks are full-width by default, so when the inserter appears on the right hand side, it's to the right of the whole element box, and if the block content doesn't stretch to full width (as it won't with small images or some headings) then the indicator can be pretty far away from the block. I'm not sure how to deal with this as afaik there's no way of retrieving the width of just the content without changing the block style to inline. Here's a video showing current behaviour: dragdrop.mov(I haven't added the 10% blue background highlight yet) |
|
Nice! Thank you for working on this. I'm almost certainly missing nuance, so I'm going to list out some thoughts, then you can correct me or point out the misconceptions:
Following that train of thought, in this case:
In this case, the block is either "filling" the remaining part of a horizontal flex box, and the blue indicator all the way to the right would be correct, since that's where the block will end up. There's another behavior too. In this GIF are a bunch of paragraphs, and an image. When I drag the image on top of a preceding paragraph, dropzones left and right are surfaced, and when I drop on a zone left, the paragraph is turned into a row, with the image on the left:
In principle, that's a cool layout tool, but it's not actually what I would expect. I wouldn't expect any containers to be created as part of this effort, so if I drag an image onto a paragraph in a default flow-based post container, I would expect the dropzones to be highlighted before and after the paragraph, like this:
I realize I should've shared this instinct earlier on, apologies. One way I was thinking about this was that we actually output these drop-zones in the DOM, before and after the block hovered. The benefit would be, presumably, that those drop-zones in the DOM would both make the space available, but it would also intrinsically place itself where it needs to be as it would be subject to the container flex or flow rules. The main problem here is the layout shift this will cause, you might enter into a loop where the block you're hovering would layout-shift and you'd no longer be hovering, but I wonder if we can be smart there, and remember which block is hovered so long as the dropzones exist, and not blur it until you're also not hovering any of the dropzones? |
Apologies if I didn't make the intent fully clear 😅 this exploration is based on the ideas in #13202 which aim to create appropriate layout containers when users try to drag blocks next to other blocks. Essentially we're trying to guess when the desired outcome is to have the two blocks side by side, and wrap them in a Row. Do you think the visual indicators from your designs could be made to work for such a case? Regarding making the before and after dropzones visible while hovering over a block, @andrewserong has an experiment over in #56466. |
|
Oh, understood. As a prototype of that this PR certainly gets close. But I will also say in testing, that this didn't feel as useful in practice as it might sound. I think there's still room for the add-before suggestion as outlined. Perhaps they can even coexist — hover top or bottom halfs, it's before/after (like Add before), hover on the left or right halves, it's convert to row and put before or after, unless it's already in a Row in which case just put before or after. Make sense? |
tellthemachines
force-pushed
the
try/drag-into-row
branch
from
9a90c2d
to
e6ebdd0
Compare
tellthemachines
changed the title
|
Ok I think I've addressed all the feedback so far:
I don't think we need to check that the blocks being grouped are groupable, since grouping blocks should allow all blocks? Unless they can't be dropped in which case we won't see a drop indicator now that #56843 is in place. Or is there any other scenario I'm missing? We're already doing a ton of checks here so would be good to avoid unnecessary ones 😅 The failing tests will have to be overhauled because they were built on the assumption that dragging around the sides of a vertical block list would activate the vertical inbetween inserters, and that behaviour has now changed. Would be good to be sure this is the desired behaviour before going ahead with that! I'd especially appreciate design feedback. Would also be great to get accessibility feedback to make sure this isn't breaking anything! The only new behaviour here is the creation of a new wrapper block when blocks are dropped into certain positions, so I'd expect that to not be a huge deviation from the accessibility of the existing implementation, but I'm probably missing things here 😅 |
We might still want an 2024-01-17.16.26.24.mp4Otherwise, the threshold is feeling nice to me in testing! The main issue I ran into is this weird one when dragging a paragraph to the right of a Row block. It then wound up in a strange state where there's two instances of the one paragraph block, so they both appear to be selected at the same time, with duplicated block tools. I've never seen anything like it! Let me know if this is just an odd thing I've run into in my environment, and I can try to come up with some more detailed replication steps 😅 2024-01-17.16.30.45.mp4 |
|
This is working really well for me, nice one @tellthemachines ! The only feedback I have is could maybe increase the threshold ever so slightly but I think we can start with what we have here so its less intrusive and then bump it up based on feedback after release. layout-drag.mp4 |
|
Flaky tests detected in e3b729f. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7622150024
|
|
Thanks for the reviews!
Turns out the problem was the previous logic for avoiding double-wrapped Rows. I updated to instead prevent the drop indicator from appearing beside a Row (so you should only be able to drag into the Row, not beside it) and that fixed the issue for me. Also added the groupability check and updated the unit tests to take this new behaviour into account. I think this is ready for a final check! |
|
Nice, this is feeling a lot closer to me! The issue with nested groups and locked blocks appears to be resolved for me now 👍 I'm still getting a bit of weirdness when dragging to the very edge of a full-width block. An example is when dragging a heading block within the site editor in the TT4 blog home template where we've got a lot of quite tall Group blocks. It seems to flicker a bit with showing/hiding scrollbars for me: 2024-01-24.14.53.06.mp4In case it helps debug, my OS is set to always display scrollbars. I'm wondering if the vertical drop indicator line in this case is stretching off the edge of the container, and therefore creating an additional scrollbar? I wonder if we can constrain that drop indicator line so it never (technically) goes off the edge of the screen 🤔 To distil this use case: dragging and dropping to the left or right of a long block that extends beyond the height of the viewport and might be full width. Other than that, it looks like some of the e2e test failures might be real for |
tellthemachines
requested review from
kevin940726 and
Mamaduka
as code owners
|
Thanks for re-reviewing! I've fixed the e2e and added a new test for this case.
That's annoying, but not specific to this branch. I can reproduce it on trunk, in the site editor, with a very long block at the edge of a Row. Example markup:I think it might be a good candidate for exploring a fix for separately, especially given it's a bit of an edge case. |
There was a problem hiding this comment.
That's annoying, but not specific to this branch. I can reproduce it on trunk, in the site editor, with a very long block at the edge of a Row.
Oh, great catch! Yes, I can reproduce that on trunk now, but bizarrely only in the site editor and not in the post editor for some reason 🤔
In any case, it seems this PR just makes the issue more visible since we're using the vertical drop indicator more frequently, so I agree, that sounds like a good bug for us to look into fixing separately.
This looks good to try to me!
Nice work getting this one over the line ✨
Same! I wonder if there's some difference in how the iframe is setup between editors 🤔 Anyway, thanks for the thorough testing! |
|
Excellent work; I love the feature! |
|
@tellthemachines, I just noticed that one of the tests became flaky after this got merged - #58161. My attempt to fix it - #58208. |
|
It seems that this PR causes an error to occur when dragging and dropping multiple local images into the editor. See #58653 |
What?
Implements #13202
Enhances drag and drop so that, when hovering over or around blocks in a vertical layout, dropzones appear around the sides to allow for grouping blocks horizontally into Rows.
Additionally, if an Image block is dropped beside another Image block, they will transform into a Gallery.
Is this a good addition? Feedback appreciated!
Testing Instructions
Todo:
images-to-gallery.mov