Increase CSS specificity of text alignment classes#62260
Conversation
|
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: +3 B (0%) Total Size: 2.42 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 367acb0. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/19289324768
|
It sounds to me like block supports styles should load after global styles? |
I have tried adding text alignment-related styles to I think this approach will probably work too? diffdiff --git a/lib/block-supports/typography.php b/lib/block-supports/typography.php
index fa2a7b81e9..e90291a74b 100644
--- a/lib/block-supports/typography.php
+++ b/lib/block-supports/typography.php
@@ -621,3 +621,20 @@ if ( function_exists( 'wp_render_typography_support' ) ) {
remove_filter( 'render_block', 'wp_render_typography_support' );
}
add_filter( 'render_block', 'gutenberg_render_typography_support', 10, 2 );
+
+function gutenberg_poutput_text_align_styles() {
+ foreach ( array( 'left', 'center', 'right' ) as $alignment ) {
+ gutenberg_style_engine_get_styles(
+ array(
+ 'typography' => array(
+ 'textAlign' => $alignment,
+ ),
+ ),
+ array(
+ 'selector' => ".has-text-align-{$alignment}",
+ 'context' => 'block-supports',
+ )
+ );
+ }
+}
+add_action( 'init', 'gutenberg_poutput_text_align_styles' );
diff --git a/packages/style-engine/class-wp-style-engine.php b/packages/style-engine/class-wp-style-engine.php
index 272c12705b..e519fbaf56 100644
--- a/packages/style-engine/class-wp-style-engine.php
+++ b/packages/style-engine/class-wp-style-engine.php
@@ -267,6 +267,12 @@ if ( ! class_exists( 'WP_Style_Engine' ) ) {
),
'path' => array( 'typography', 'lineHeight' ),
),
+ 'textAlign' => array(
+ 'property_keys' => array(
+ 'default' => 'text-align',
+ ),
+ 'path' => array( 'typography', 'textAlign' ),
+ ),
'textColumns' => array(
'property_keys' => array(
'default' => 'column-count',
|
| // Text alignments. | ||
| .has-text-align-center { | ||
| // Increase specificity so that styles applied via the global styles can be overridden in block instances. | ||
| :root .has-text-align-center { |
There was a problem hiding this comment.
Do we not need the same for .has-larger-font-size etc. above?
There was a problem hiding this comment.
Honestly, it sounds to me like these types of selectors should move after theme.json styles. These are not default styles to me.
|
In 3b093ca, I tried injecting text alignment-related styles after the global styles. For example, if <style id='global-styles-inline-css'>
:root :where(.wp-block-media-text){ text-align: right; }
</style>
<style id='core-block-supports-inline-css'>
/**
* Core styles: block-supports
*/
.has-text-align-left {
text-align: left;
}
.has-text-align-center {
text-align: center;
}
.has-text-align-right {
text-align: right;
}
</style>However, I couldn't figure out how to apply a similar approach on the editor side. If you have any implementation advice, I'd love to hear it 🙏 |
|
While looking into this, I saw that no block currently supports text align? Is that right? |
Yes, that's right. You can find more details on what's coming in WP6.6 in the draft dev note on this support. |
|
Is it a high priority for 6.6 to fix text alignment for the media and text block, if it's not supported by core? |
|
This issue isn't an issue unless developers explicitly opt-in to So, while it would be ideal to fix this in WP6.6, I don't think it's a high priority. |
|
@t-hamano Are you ok with punting this? Sorry 😔 |
|
I have a question. However, I don't see any mention to the fact that this feature is, in a way, still in the works and when developers will play with it they will be very likely surprised the block toolbar alignment setting won't work as they expect. I'd tend to think it would be important to mention this issue in the dev note. Pinging @juanmaguitar as author of the dev note. |
That's certainly true. I'll add it to this dev note and ping @juanmaguitar. |
|
I understand that is the plan, and I fully agree with it. But it is not implemented right now, what is the expected test result for blocks, including third party blocks, that uses the attribute? |
The UI will be displayed twice. But isn't this the same for all other block support? For example, if you add Developers who want to migrate from attribute to block support will need to add explicit migrations, but I believe this applies to all block support. |
|
I am confused, I meant for this PR, which I thought was intended to make the block toolbar option work again, not the conversion. |
That's right.
My explanation for this was not accurate. For example, the Heading block currently supports text alignment as an attribute, not as a block support. If you define a default text alignment via {
"version": 3,
"styles": {
"blocks": {
"core/heading": {
"typography": {
"textAlign": "center"
}
}
}
}
}This PR is an attempt to address these issues as well. However, |
|
As a theme developer, I do want to explicitly opt-in though 😅 because it means that default text alignments can be removed from the template and pattern code, making them much more flexible. |
|
If anyone has any good ideas for moving this forward, please let me know 🙏 Here's a summary of this PR and the latest status:
|
At a glance, it appears in 3b093ca you are bundling these text alignment styles in with the others created for block supports. The equivalent of that in the editor is probably I'm not sure Could the block support in the editor, not only add the This is all just scraping the bottom of the barrel for ideas though. Everything else I can think of ends up requiring any block adopting the support to need deprecations which to date has been deemed undesirable. Hopefully others have some better ideas 🤞 |
|
The main issue is that We have only two ways to make these nested styles work:
In my experience relying on the order is harder than controlling the specificity. I feel like 99fe955 is the best route to go since it maintains backwards compatibility and provides the smallest changeset. |
|
@t-hamano Should we resurface this again. I think it's important to switch to the block support for paragraph to allow global styles support (justification...) |
|
I think the loading order is correct. block library is "core" styles that should be overridable by themes (theme.json) which should be overridable by instance block styles. The issue is that these classnames are "user instance block styles" but defined by the block library (it can happen) but in this case, we need to ensure the specificity is higher than the theme.json styles. Assuming 99fe955 doesn't cause any unintended consequences, I think I'd be ok with this fix. |
|
@youknowriad Sorry for the late reply.
I tried this approach again. As far as I know, there is no problem with this approach. |
youknowriad
left a comment
There was a problem hiding this comment.
I'd appreciate more testing but I think this is good. It would be great to migrate all the blocks to use the block support after this. (I have a PR for paragraph already)
Follow up #59531
See this discussion: #61717
What?
This PR increases the CSS specificity of the
.has-text-align-*selector so that text alignment support can be overridden by block instances.Why?
When text alignment styles are defined in the global styles or theme.json, the latest Gutenberg and WP6.6 generate styles like the following:
On the other hand, if you change text alignment on a block instance, the
.has-text-align-*class is added. This style is part of the block library styles:Both selectors have the same specificity (
0-1-0), and global styles are loaded after block library styles, so overriding styles on block instances doesn't work.How?
There are several possible approaches, but I added
:rootto increase the specificity of the selector by one (0-2-0). The reason I used:rootis to make the format match the selector output by the global styles.We could have added
!important, but I didn't add it to avoid overriding corner cases like the following:I also considered two other possible approaches:
Enqueue text alignment-related selectors after the global styles
As far as I can tell, the only core inline CSS that is loaded after the global styles inline CSS is the style with the
core-block-supportshandle.I could have moved the text alignment-related selectors there, but this style seems to be for generating dynamic selectors depending on the context, so I didn't think it was appropriate:
Add a new selector for block support (such as
has-block-text-align-*)If we take this approach, we will convert the core text alignment controls to this new class when migrating to block support (See #60763).
However, some themes define additional styles that rely on the
.has-text-align-*class, so migrating to this new class will break their styles.Search results in WordPress core: https://github.com/search?q=repo%3AWordPress%2Fwordpress-develop%20.has-text-align-&type=code
Testing Instructions
Add textAlign support to the Media & Text block:
Details
Enable Emptytheme and change the default text alignment of the Media & Text block via theme.json:
Details
{ "$schema": "../../schemas/json/theme.json", "version": 3, "settings": { "appearanceTools": true, "layout": { "contentSize": "840px", "wideSize": "1100px" } }, "styles": { "blocks": { "core/media-text": { "typography": { "textAlign": "center" } } } } }Screenshots or screencast