-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Zoom out: Try vertical displacement when dragging a pattern between existing patterns/sections #63896
Conversation
Size Change: +1.7 kB (+0.1%) Total Size: 1.78 MB
ℹ️ View Unchanged
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
aba1f8f
to
4e1b5c5
Compare
a728b0d
to
92daad1
Compare
I like the exploration; there may be something here. A few notes to see if this can work better:
CleanShot.2024-07-25.at.12.20.05.mp4
CleanShot.2024-07-25.at.12.21.36.mp4
CleanShot.2024-07-25.at.12.24.33.mp4 |
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 looking good to me.
We need some sort of delay, or min time: CleanShot.2024-07-26.at.12.28.50.mp4 |
b5699a6
to
ca38b54
Compare
This comment was marked as outdated.
This comment was marked as outdated.
packages/block-editor/src/components/block-list/zoom-out-separator.js
Outdated
Show resolved
Hide resolved
I don't think this needs a transition delay, though I added one. I see a problem when we drag inside the inserter on top of the first element inside the root element (usually the first item right after the header) where the insertion point keeps being recalculated incorrectly. I haven't figured out yet why. I think once we fix that, this should be good enough to merge |
1cc8c84
to
89ee848
Compare
Attempt to define sectionClientIds Pass sectionClientIds to isSectionBlock Rename sectionClientIds to sectionRootClientIds check if it's zoom out mode, add some CSS to the separator expand separators when the insertion point is changed fix animation only open the first separator if the insertion point index is 0 conditionally render the separators and animate them using framer instead of on class change remove unused code refactor separators do displacement on drag
48a63ff
to
94a8a14
Compare
Rebased to pull in fix for Safari bug to allow testing on older versions of Safari. |
In relation to the jankyness of the separator animation in Safari. This is because the The info on the draglave targets is as follows:
As the The bug in Safari is - https://bugs.webkit.org/show_bug.cgi?id=66547 We now need to find a workaround. |
Thanks for digging into the jankiness @getdave! Just re-testing 2024-09-06.15.37.47.mp4So, I'm wondering if the source of the jankiness issue is partly something that already exists in I'm a bit on the fence about whether or not the jankiness is a blocker to landing — it'd be a great thing to fix and polish, but I also don't want to hold up this PR. Do feel free to go ahead and land this if you and others are happy enough with the current state of the PR! |
My vote is YES to land this and polish after. This is a much better experience than trunk already |
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 looking great, thanks for all the hard work
I added an issue to track the ongoing problems in Safari: #65125 |
What?
Part of #50739
Implements a way to separate blocks in zoom out mode vertically both when we drag in a new pattern in between sections and when we click on the inserters
Co-authored-by: Dave Smith [email protected]
Why?
We need a better visual aid to guide the user when building with patterns. Separating section makes it clear where the action is happening
How?
We are adding some conditional separators in between blocks of the blocklist that appear based on a couple of condition: if the block in question is a sectioning element, and if the insertion point is related to said block. The rendering of this separator is animated to push the sections apart.
This also required updating the drag handling logic to avoid triggering
dragend
events when moving in/out of the zoom out separators. This ensures the separator itself does not re-render if the mouse cursor inadvertently moves into/out of it whilst it is animating. This avoids the "stuttering" animations we were seeing early in the lifecycle of this PR.Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast
Screen.Capture.on.2024-08-22.at.14-25-52.mp4