Increase CSS specificity of text alignment classes #62260
Conversation
t-hamano
added
[Type] Bug
t-hamano
added
the
Backport to WP 6.7 Beta/RC
|
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: -84 B (0%) Total Size: 1.76 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 99fe955. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/9361305050
|
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',
|
| @@ -32,16 +32,17 @@ | |||
| } | |||
|
|
|||
| // 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 😔 |
t-hamano
removed
the
Backport to WP 6.7 Beta/RC
|
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. |
|
@t-hamano @afercia I have added a warning in the Added Text alignment block support section of the Miscellaneous Editor changes in WordPress 6.6 DevNote, as per this comment |
|
This is only an issue for the media and text block I believe? |
This issue will likely occur with any block that supports For example, if the Heading block supported it, the same issue would occur.
Just to be clear, the Media & Text block was mentioned only to test this PR, and currently no core blocks support |
|
I don't have much time to follow up today, but can add that I did not test with a block that supports text align as a block support. when I found the issue I was testing with the heading block. The same class name is used with blocks that have the text align block attribute. |
For core blocks that support |
|
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 🤞 |
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