Site Editor: Ensure ResizableFrame does not force Cover blocks within the editor to show drag handles #59262
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… the editor to always display drag handle
andrewserong
added
[Type] Bug
|
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. |
|
Size Change: +5 B (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in ae62676. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7999668258
|
getdave
approved these changes
Feb 28, 2024
There was a problem hiding this comment.
Screen.Capture.on.2024-02-28.at.14-43-17.1.mp4
I tested this and the drag handle is not available once you select an aspect ratio. The only issue is that even if I go back to Original the drag handle is still not displayed.
The only way to get it back is to set a minimum height.
This might not be an issue because when I reset the aspect ratio control it seems to have a minimum height set by default so perhaps the behaviour of Original is intentional?
The fix seems good for the original bug though.
getdave
added
the
Backport to WP 6.7 Beta/RC
|
Thanks for the review @getdave!
Yes, that's a separate issue in regards to how the aspect ratio controls work, which is that selecting "Original" sets an explicit value rather than clearing out the value. I think we can look at that separately if it winds up being an issue for folks. |
andrewserong
deleted the
fix/resizable-frame-from-overriding-inner-resizable-boxes-used-by-cover-block
branch
youknowriad
pushed a commit
that referenced
this pull request
Mar 4, 2024
… the editor to always display drag handle (#59262) Co-authored-by: andrewserong <[email protected]> Co-authored-by: getdave <[email protected]>
|
I just cherry-picked this PR to the update/packages-6.5-rc1 branch to get it included in the next release: 82a4572 |
youknowriad
added
Backported to WP Core
youknowriad
pushed a commit
that referenced
this pull request
Mar 4, 2024
… the editor to always display drag handle (#59262) Co-authored-by: andrewserong <[email protected]> Co-authored-by: getdave <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
In the site editor's
ResizableFramecomponent, set theResizableBox'sshowHandleprop tofalseso that CSS rules that show the handles don't flow down to nested resizable boxes that shouldn't display their handles.This resolves an issue in the site editor where the Cover block will show the handles on its resizable box even when it shouldn't be (i.e. when an aspect ratio is set), because the Cover block is rendered further down the tree from this
ResizableFrame.Why?
By default,
ResizableBoxtreatsshowHandleastrueand adds a classname that means some CSS rules to reveal the handle will apply to any nested resizable box.To work around this, we can set the
ResizableFrameto pass infalsetoshowHandle. Because the site editor'sResizableFrameuses its own customhandleComponentwith the class name ofedit-site-resizable-frame__handle, it never received theResizableBox's CSS rules for showing and hiding the drag handle, so this PR shouldn't affect the resizable frame in any way.While we could potentially resolve this issue over in the resizable box, for 6.5, I think this is likely the safer fix as it's a small change to one component used in one place, where the impact is hopefully fairly well known.
How?
ResizableFramecomponent, set theResizableBox'sshowHandleprop tofalse.Testing Instructions
trunk, note that the drag handle of the Cover block will still be shown, but it'll be in the wrong position, and dragging it will do nothingTesting Instructions for Keyboard
To check for regressions, from the site editor browse mode, make sure that you can still tab all the way over to the drag handle for the resizable frame.
Screenshots or screencast
Before
Note the drag handle that doesn't do anything at the end of this video:
2024-02-22.15.44.05.mp4
After
Drag handle is hidden when aspect ratio is applied (same behaviour as in the post editor):
2024-02-22.15.47.05.mp4
Drag handle that should be unaffected
This is the drag handle for the resizable frame that should be unaffected, and still be able to be tabbed over to by keyboard.