-
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
Remove the default block margins from themes with theme.json file #30375
Conversation
Size Change: +79 B (0%) Total Size: 1.42 MB
ℹ️ View Unchanged
|
Thanks so much for trying this. As noted, those default margin values, but top and bottom, left and right, have caused headaches for a long time. This PR is a step in the right direction for block themes, but the increased specificity of the .wp-block selector as you note, is causing some headaches even just in this branch, as they now override theme specificity in all the themes I tried: If we were able to output the old block styles in a separate stylesheet that was not loaded for block themes, essentially a back-compat style, that seems like it would be able to keep the legacy specificity intact, while not burdening block themes. Is this what you'd prefer to avoid (based on the second comment)? One alternative I can think of is to invert the styles, to something like this:
But that puts the extra specificity burden on block themes. A 3rd option is to load a separate additional stylesheet just for block themes, one that includes only these styles:
The only option of those three that I like, is to output the legacy styles in a separate stylesheet, loaded conditionally. What's in trunk currently is already a bit of a headache (see again #30337). |
Thanks for the feedback @jasmussen I implemented your first suggestion, let me know what you think. |
Thanks so much Riad, this is working really well for me, and the split seems like it will provide us with a way to support both paths forward in as best a possible way. I think this'll work well, though I'd encourage additional thoughts. @mcsf? I'm seeing the same issue as Carolina, even with the demo content: The rules are output correctly it appears: The new lower specificity (which, amazing, by the way), now gets overridden by this: Perhaps we can lower the specificity on that reset as well? |
That's not possible, these are there to override wp-admin default styles. |
The bug should be fixed now, I restored the specificity that was applied to these rules before #29335 and in trunk as well. |
Big fan of this approach. It could use a good bit of testing, but 👍 👍 from here. Thank you Riad! |
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.
LGTM! 👍 - I tested with various themes and in general this feels so much better! Thanks Riad!
@@ -360,6 +360,11 @@ function gutenberg_register_packages_styles( $styles ) { | |||
'wp-reusable-blocks', | |||
); | |||
|
|||
// Only load the default layout and margin styles for themes without theme.json file. | |||
if ( ! WP_Theme_JSON_Resolver::theme_has_support() ) { | |||
$wp_edit_blocks_dependencies[] = 'wp-editor-classic-layout-styles'; |
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 such a nice way to ensure backward compatibility. ❤️
Looks like this PR fixes some issues in 10.3.1. If we do a 10.3.2 we could consider including this one. |
@WunderBart, it looks like we have 2 candidates for 10.3.2, including your #30540. |
Tries to implement the idea expressed by @jasmussen here #27315 (comment)
This PR removes the default block margins applied in the editor from themes with experimenta-theme.json file..
This ensures that these themes are more WSIWYG