-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Try showing an alignment visualizer when using the drag handle to resize an image block #45056
Conversation
Size Change: +2.77 kB (0%) Total Size: 1.37 MB
ℹ️ View Unchanged
|
Exciting! I know it's incomplete, but here's what I'm seeing currently: image.mp4Aside from the points outlined in the OP, I noticed:
In these cases Outline mode automatically swaps the default blue outline on selected blocks is replaced with a white one, perhaps we could do something similar here? |
Oh, my mistake, I hadn't pushed a commit, it should be better now.
Right, at the moment it'll work where content is centered, I'll need to take offset content into account. Thanks for testing. |
It might help if we can add the option to the BlockPopover, in this case, not to be constrained to the current viewport. At least vertically. This is also an issue for the PR adding borders to the Cover block. In that PR, the resizable box for the Cover block is moved to a |
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 a great start so far @talldan 👌
It definitely appears to have some curly challenges.
I initially tested with a dark theme and some of the alignment guides looked out of whack. It seemed like there was a border to the left but not the right as well as the Full Width alignment almost completely overlapping the Wide alignment label.
It turned out the Pitch style variant for TT3 doesn't have all alignments set in the theme.json but those alignments were still displayed by the visualizer. Switching to the default style for TT3 and everything looked very nice.
TT3 - Pitch | TT3 - Default |
---|---|
Screen.Recording.2022-10-19.at.4.19.41.pm.mp4 |
Screen.Recording.2022-10-19.at.4.19.04.pm.mp4 |
Find a way to hide the block border when resizing
While this might be a bit of a hack, would it be possible to lift the isResizingImage
state up to the edit component and then use that to override the class name or style that applies the block's outline?
This is preeettty wild: Nice. If code-wise you're feeling this one, this expanded grid can be pretty potent. As Jay notes, it's best if it appears (perhaps fades in quickly) when you scale above content width, but not when you're scaling down. Can we use |
Sounds good! I'll try to get to those improvements soon. I noticed a few more cases that need to be handled. The entire post content can be justified in a template, and that changes how alignments work. I'm wondering what the best way of handling all those situations is. If the visualizer could use the same styling as the block list (replicate a block in a block list), that might be an interesting solution that would just work. At the moment I've only really handled centered content. I've also been working on a PR adjacent to this to improve how popovers work for these kind of components (#45137). |
a8ba2dd
to
17bcfe6
Compare
This should be working better in terms of different layouts and justifications. For example, you can put an image in a full width group, adjust the justification and content/wide sizes on the group, and when resizing the image it should show the alignment zones in the correct position. I notice there are still some things to solve, like padding isn't taken into account, alignments in groups that are content width are a bit unusual, and all the other review feedback still needs to be worked on. |
Took it for a quick spin, this is fast coming together, impressive: In smaller viewports such as for this example, the labels are clashing a bit. I wonder if we could make the labels appear as your resize handle nears the snap-point, so we only show one label at a time? What do you think? That might also make it clearer that a "snap" is about to happen if you release when the tooltip appears. 🤔 It might also make sense to use the Tooltip for these after all. Nice progress! |
I think that's worth at try. Another option could be to simply omit 'width' from the labels. This has the benefit of clarifying the guides before you drag to reach them, but it's not bullet-proof and there will still be rare overlaps. |
I think one issue this may face is an "agressive jump" when the resizing goes from being just by one corner to filling up the left half as well and the alignwide class being added. Obviously this won't be an issue when the image is align center, but most of the time they're unset and therefore scale from the left and out. Could it be an idea if the sizing was how it currently is (scaling by the corner) up until one hits the content width, and then after that have it scale from the center and then snap to alignwide and full? I think thay would provide a bit less "jumpy" experience. Any thoughts? |
cb5967f
to
97105cd
Compare
Quick update on today's changes:
Kapture.2022-10-31.at.12.52.12.mp4
@victorberland I also anticipate that this might be an issue. I don't think we'll know how bad it is until we get to the point where it's implemented, and then we can try a few things. Another idea is that we could show a kind of ghost version of the image. The other potential issue that comes to mind is that sometimes the user might not want the image center aligned when resizing. |
Nice, coming together fast!
Nice! It if the all-caps label style is the one Jay is going for, it looks like the font is slightly too bold and large compared to what's in the sidebar:
Feels great! |
95c44e0
to
5e5f924
Compare
…ser before snapping initiates
Thanks for all the feedback everyone. I'll respond to some of it. I think we also need to determine what the MVP is for this feature, especially with it now behind an experiment toggle.
Unfortunately it's not just a matter of removing that code. The image would also first have to be unsnapped when the user starts dragging from full/wide alignments, otherwise the image will be resized from the wrong origin (full/wide position instead of content position). It's not trivial to implement, so I thought I'd leave it as a follow-up task to keep this PR as small as possible.
I agree on all points (especially that it is a big ask 😄). I think it'd be good from a UX perspective to introduce more canvas tools like this. I'd definitely be interested in trying it out on a few other blocks, and looking at making it part of the
I couldn't reproduce any performance issues in Safari (using an M1 Macbook Pro). I think I have an older computer I can try it on.
I've noticed it too. The position of full alignment is dependent on the viewport width. Perhaps it'd be best to hide 'wide' when it's too close to 'full'. At the moment I only hide it when they completely converge 🤔
It seems in I'm also very certain it won't work in all cases—groups with padding has been a difficult one to solve, the layout system doesn't offer any clue of the layout bounding box, and I think that's needed. I would quite like to work through this in a separate PR before hacking around with the layout system—scoping the PR will make it much easier to review.
It seems unusual that this happens with freeform resizing. I can look into it, but if it happens in trunk it'll be a separate PR.
I think @jasmussen feels differently, so you two might need to discuss it 😄
So when you drag over the entire segment, it snaps? I'll try it, I think that should be fairly easy to experiment with. |
e91d8d4
to
7290e20
Compare
Personaly, allowing to drag and "unsnap" when you're already in full/wide images feels like the biggest issue right now. As a user, it's not clear why you can't just continue resizing after you're done with your "first attempt". |
I'll start exploring it in a separate PR that we can either merge into this one or merge to trunk after this one - #49888 |
That could work as a plan B, but if I can do it from the button in the toolbar feels worth also doing it with resizing if at all possible. Could we set a minimum "snap hit area" that, if need be, pushes the preceding snap point inwards? To be clear this might be best explored in a followup, I don't think of this as a blocker.
I do indeed :D — I think the tooltips that appear when you are near a snap point are perfectly timed and contextually useful. I love them. But I also would not object to starting this experiment without them and adding them later. Or indeed the reverse, starting with them and removing later. Overall, it's not a strong opinion, and I'm happy to defer to someone who has. |
Perhaps as a follow-up we can try this UX. It would help prevent the awkward resizing past the content area issue. |
I haven't found a way to do this that isn't buggy. If the alignment is unset from 'full' or 'wide' as the user starts dragging, the My worry is that the only solution is to build a new |
Still struggling with this one technically. I've been slowly working on an alternative implementation to |
What?
See #44357
Implements a new feature to help with resizing image to the same widths as alignments.
Given the number of known issues and that some of them are very challenging, I've decided to make this an experiment (it needs to be toggled on via the experiments page), so that this PR can be review/merged without encompassing too many changes.
Follow-up work / Known issues:
Testing Instructions
wp-admin/admin.php?page=gutenberg-experiments
)Technical details
BlockAlignmentVisualizer
component has been introduced which does the bulk of the work, it's responsible for showing the alignment visualization and has a helpfuluseDetectSnapping
hook that can be used by other components. It should be possible to use this component in other places when alignments need to be visualized.ResizableAlignmentControls
is introduced which composesBlockAlignmentVisualizer
together with theResizableBox
component. It mostly has the same interface asResizableBox
.ResizableAlignmentControls
component instead ofResizableBox
, a few extra lines of code are added to handle snapping events.The
BlockAlignmentVisualizer
component also has a few sub components:Underlay
- like aPopover
, but it renders inline under the block. This PR did initially use a Popover, but introducing this component is less code and means no changes toPopover
(which doesn't support inline rendering right now).ShadowDOMContainer
- since the whole visualization is rendered inline, the code is nested in a shadow dom container to prevent styles leaking. It does unfortunately mean that scss can't be used for the styles.Visualization
- this is the component that renders visualization. Props to @jasmussen for the grid implementation.Guides
- this component is not visible on the screen, but it renders a div for each alignment, and then the positions of divs are used to provide the snapping functionality.GuideContext
- stores refs to the DOM nodes rendered by the guides component. Also exports theuseDetectSnapping
hook, which detects snapping by calculating intersections between the resizable block and the guides.Screenshots or screencast
Kapture.2023-03-28.at.13.13.11.mp4