-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Show alignment steps Tidy up and add labels Use a resize observer to handle resizes Set a max width to avoid visualizer step appearing beyond screen edges Only show visualizer while resizing Use availableAlignments instead of hard coded array Refactor away from block popover Adjust sizing to parent/root block list Move into separate component file Move more code into effect for optimization Remove export Hack together a version that dynamically uses the parent block list layout Bring back proper styles Fix popover positioning Tidy up css and naming - Rename step to zone - Use the unstyled variant Use text color as the contrast color Allow popover to render inline by overriding slot Show nearest alignment zone label - Register zones into a slot when they mount - use `getDistanceToNearestEdge` to detect nearest zone to mouse pointer - Make Popover render behind the image and receive mouse events - Add CSS to only show highlighted zone Show nearest zone to mouse by hooking into resizeable box events Allow popover to render inline Add unstyled variant to wrapping popover Rename props Rename prop Take padding and margin of layout into account Lessen opacity of background Rename from blockList to layout Reduce need for margin/padding on container Throttle zone detection Ensure popovers that reference iframe are positioned correctly by not applying padding to iframe Fix zone highlighting when layout has padding Remove spacing variable Remove unused dep Add a basic fade-in animation Animate label Update label style Refactor - Move ResizableImageControls to block editor package and rename to ResizableAlignmentControls - Export ResizableAlignmentControls and remove export for BlockAlignmentVisualizer - Move hardcoded alignments to image block Use context for zones and switch to a map instead of a set for storing zone data Add basic snapping Fix incorrect prop name Move nearest zone detection to event handler Update re-resizeable Detect snapping Document code Speed up iframe rendering Avoid removing iframe head and body and reattaching them Make iframe title a prop Use custom snapping algorithm over resizable box algorithm Reintroduce basic snapping while dragging Set alignment on release of mouse Allow snapping from left/right alignments Fix snapping to content width for images that are larger than content width Increase snap threshold Refactor content width snapping fix to image block code Try allowing resize handles on wide and full aligned images Revert "Try allowing resize handles on wide and full aligned images" This reverts commit f8402c2. Revert "Update re-resizeable" This reverts commit 7d7e231. Add some really dodgy animation code Revert "Add some really dodgy animation code" This reverts commit a60d775. Use a framer motion layout animation for snapping Refactor and rename things Remove unused resize observer Refactor - separate popover code from layout code Fix padding being applied to visualizer incorrectly Move styles to the content.scss Remove border from iframe Switch to using the shadow dom instead of an iframe to encapsulate styles Speed up animation Simplify guide styles Remove offset naming Adjust layout popopover to use correct element Try fixing layout block detection Remove non-existant stylesheet import Fix iframe offsetting Remove buggy padding on cover element Use a consistent value for no snapping Use package-lock from trunk Start implementation of new design Hide old guides Support content justifications for new visualization Simplify guides code, remove use of LayoutStyles Move snapping code into a hook Move getOffsetRect to dom package and rename Try: hide full and wide alignments at smaller container sizes Fix snapping Fix - show/hide block interface actions not usable outside dimensions panel Hide block toolbar when resizing Hide block border when using alignment visualizer
…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