Cover block: Remove overflow hidden rule #49913
Merged
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
andrewserong
added
[Type] Bug
andrewserong
requested review from
talldan,
apeatling,
ramonjd,
tellthemachines and
aaronrobertshaw
andrewserong
changed the title
|
Size Change: -25 B (0%) Total Size: 1.37 MB
ℹ️ View Unchanged
|
aaronrobertshaw
approved these changes
Apr 19, 2023
There was a problem hiding this comment.
Thanks for the fix @andrewserong 👍
This tests as advertised for me.
✅ Works with a variety of cover blocks (image or color background, with or without borders, different content, duotone filters etc)
✅ No scrollbars or clipping occurs in either block or site editors
✅ Cover block displays fine on the front end
| Before | After |
|---|---|
 |
 |
|
Flaky tests detected in 731e3a5. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4740384154
|
|
Thanks for reviewing so quickly, much appreciated! |
andrewserong
deleted the
try/remove-overflow-hidden-rule-from-cover-block
branch
andrewserong
changed the title
|
@andrewserong @aaronrobertshaw removing cc @bph
|
|
Thanks for flagging this @ndiego! I'll look into this further, but if we can't come up with a short-term fix to preserve the removal of I'm wondering if we can somehow get the background element to inherit border-radius, rather than relying upon |
|
I've opened a quick revert PR over in #50209 — I'd be happy with us going with that in the interim, as it doesn't look like inheriting border-radius will quite work, as the background element would need a different value from its parent in order to not have a gap between the borders. Or, we'd want to apply the border to the child instead of the outer wrapper, which could also cause other issues 🤔. Example pic of setting the child to inherit the border from the parent:
For now, I think let's fix the regression, and we can look at other potential options in follow-ups. |
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?
Remove
overflow: hiddenrule from the Cover block's styling rules, and retainoverflow: visiblefor theblock-library-cover__resize-containerso that the drag handle is still visible outside the edge of the block. This is a follow-up to some of the changes landed in #41153.Why?
While working on re-enabling sticky positioning at non-root positions over in #49321, I noticed that the
sticky: positionrule does not work when the parent hasoverflow: hiddenset. Because the Cover block is a prominent container block, before landing #49321, it would be good to ensure that all the main container blocks can technically support sticky children.As far as I can tell, we no longer need an
overflow: hiddenrule because the resizeable box is using a block popover. From testing, I think the only place we need an overflow rule is for the resize container so that the handle is visible. Alsooverflow: visiblehides any scrollbars within the popover.This PR seems to resolve a subtle issue on
trunkwhere the drag handle is sometimes not visible, and there is sometimes a scrollbar within the resizeable box — I only ran into these issues in the site editor, not the post editor.How?
overflow: hiddenfrom the block's styling rulesoverflow: visiblefrom the placeholder state — sinceoverflow: hiddenhas been removed, we should no longer need this additional ruleoverflow: visibleto the resize containerTesting Instructions
A question for the reviewer — have I missed anything that was depending on the existing rules?
Testing Instructions for Keyboard
Screenshots or screencast
The below screenshots are from within the site editor.