Layout: Reduce specificity of flex and grid margin overrides #50825
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
the
[Feature] Layout
|
Size Change: 0 B Total Size: 1.4 MB ℹ️ View Unchanged
|
|
This appears to mostly be working, except for a conflict with |
|
Flaky tests detected in cbf2d0c. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5042499887
|
|
Thanks for spinning up this PR so quickly @andrewserong 🚀 I can confirm it does fix the original issue flagged via #45196 (review).
The catch with the If we can't lower the specificity of the classic margin auto styles, could we reintroduce the layout styles at a higher specificity in Something to explore perhaps, what do you think? |
|
Thanks for taking a look, Aaron! 🙇
Worth trying, especially since this is all about just a single rule! I have a few other tasks to focus on this week, so might not have much time to immediately work on this PR, but I'll come back to it 👍 |
Understood, I'm in the same boat. FWIW I've given the following snippet a quick run by appending it to I don't think I have a full grasp on all the ins and outs of the layout support styles so there's every chance I've missed something. I'll happily wait until you have a chance to revisit this, then I can test and review again. /**
* Layout Styles
*/
.editor-styles-wrapper {
.is-layout-grid > *,
.is-layout-flex > * {
margin: 0;
}
} |
|
Related: Trac 60080 |
|
This PR is pretty old now, and there are specificity reduction efforts happening elsewhere, so I'll close this PR out for now. |
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.
In Classic themes, the
classic.cssfile includesmargin-left: autoandmargin-right: autorules. Reducing the specificity of the margin overrides for flex rules causes an issue in the block editor for the social icons block — the individual social icons blocks with this PR applied have margin rules applied that override the now reduced rule. Here's a screenshot of how it currently looks:What?
Reduce the specificity of the
flexandgridlayout type margin overrides, for consistency with theflowandconstrainedlayout types.This resolves an issue where flex and grid layout types have margin override specificity that interferes with setting margins at the block level in global styles.
Why?
As reported in #45196 (review) the flow and constrained layouts currently allow block-level-in-global-styles rules to override general layout styles. However,
flexandgriddon't have that lower level of specificity for theirmargin: 0rules. From a little digging, it appears that the solution back in #47858 was to reduce specificity for thespacingStylesrules for each layout type. However, theflexandgridlayouts had their overrides inbaseStyles, which doesn't include the extra:where()rule to lower the specificity. The fix is to move the margin overrides for these two layout types over tospacingStyles.How?
margin: 0override rules forflexandgridlayout types over tospacingStylesso that they receive the lower specificity rules introduced in Try reducing specificity of layout margin rules #47858.The difference in output of rules is:
Becomes:
Testing Instructions
50pxA question / note to the reviewer: the
flexandgridrules, in contrast to flow/constrained, do not do anything about resetting margins for first/last items. I think that's likely preferable due to how these layout types work, but I'm curious what folks think. Is the change in this PR the desired approach for these layout types?Testing Instructions for Keyboard
Screenshots or screencast
Prior to this PR, the margins would only look correct in the site editor, but not on the site frontend. So for these before/after screenshots, we're looking at the site front end: