Button Block: Set proper typography for inner elements #68023

shimotmk · 2024-12-16T11:49:47Z

What?

The current typography does not work correctly with the given button styles. This fix forces inner elements to inherit the styles.

Fixes #64662
related #64869

Why?

How?

Testing Instructions

Set Twenty Twenty-Four theme
Place Button block
Set the Appearance to Bold, etc.
You can see that the Appearance has changed on either the editor screen or the front screen.

Testing Instructions for Keyboard

Screenshots or screencast

Before	After

github-actions · 2024-12-16T11:49:58Z

Warning: Type of PR label mismatch

To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.

Type-related labels to choose from: [Type] Automated Testing, [Type] Breaking Change, [Type] Bug, [Type] Build Tooling, [Type] Code Quality, [Type] Copy, [Type] Developer Documentation, [Type] Enhancement, [Type] Experimental, [Type] Feature, [Type] New API, [Type] Task, [Type] Technical Prototype, [Type] Performance, [Type] Project Management, [Type] Regression, [Type] Security, [Type] WP Core Ticket, Backport from WordPress Core, Gutenberg Plugin.
Labels found: .

Read more about Type labels in Gutenberg. Don't worry if you don't have the required permissions to add labels; the PR reviewer should be able to help with the task.

github-actions · 2024-12-16T11:49:59Z

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 props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: shimotmk <[email protected]>
Co-authored-by: ramonjd <[email protected]>
Co-authored-by: aaronrobertshaw <[email protected]>
Co-authored-by: talldan <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

ramonjd · 2024-12-17T00:14:00Z

packages/block-library/src/button/edit.js

@@ -301,6 +317,7 @@ function ButtonEdit( props ) {
 						...colorProps.style,
 						...spacingProps.style,
 						...shadowProps.style,
+						...typographyProps.style,


Hmm I think the .wp-block-button__link height of 100% is interfering with the typography setting writing-mode: vertical-rl;

I wonder if changing it to auto would break anything else 🤔

If writing-mode is best placed still on the wrapper, we could just change the __experimentalSkipSerialization value for typography to be an array of all the other typography features except writing mode.

Good call! I didn't think of that, thanks!

aaronrobertshaw

Thanks for wrangling this fix @shimotmk 👍

While reading through the changes I've spotted a few issues and left inline comments for those.

There also looks to be a possible opportunity to clean up the .has-custom-font-size class. It appears that was added to allow the inner element to be forced to inherit the parent font size.

Regarding the e2e failures, there are two issues;

The new deprecation fixture for the v12 button needs to have a Button block as the base block rather than a Buttons block
The v10 fixture has typography styles in it so the final .serialized.html result for the fixture needs to have the typography classes and styles moved to the inner element.

The last point there would be cover by running npm run fixtures:regenerate. Doing so will also leave a few other unrelated fixtures updated due to the fact their attributes have been reordered. Those changes can be discarded for this PR.

The following diff shows an example of how then fixture could be. It isn't suitable for simply committing to this PR as the fixture should cover all the typography styles (with maybe the exception of writing mode as @ramonjd highlighted).

Example diff fixing fixtures

diff --git a/test/integration/fixtures/blocks/core__button__deprecated-v10.serialized.html b/test/integration/fixtures/blocks/core__button__deprecated-v10.serialized.html
index e4c7b89c794..0bebe131629 100644
--- a/test/integration/fixtures/blocks/core__button__deprecated-v10.serialized.html
+++ b/test/integration/fixtures/blocks/core__button__deprecated-v10.serialized.html
@@ -1,3 +1,3 @@
 <!-- wp:button {"fontFamily":"cambria-georgia"} -->
-<div class="wp-block-button has-cambria-georgia-font-family"><a class="wp-block-button__link wp-element-button">My button</a></div>
+<div class="wp-block-button"><a class="wp-block-button__link has-cambria-georgia-font-family wp-element-button">My button</a></div>
 <!-- /wp:button -->
diff --git a/test/integration/fixtures/blocks/core__button__deprecated-v12.html b/test/integration/fixtures/blocks/core__button__deprecated-v12.html
index 03ac733e5c3..355f1c2019d 100644
--- a/test/integration/fixtures/blocks/core__button__deprecated-v12.html
+++ b/test/integration/fixtures/blocks/core__button__deprecated-v12.html
@@ -1,17 +1,3 @@
-<!-- wp:buttons {"align":"wide","layout":{"type":"flex","justifyContent":"center"}} -->
-<div class="wp-block-buttons alignwide"><!-- wp:button {"fontSize":"xx-large"} -->
-<div class="wp-block-button has-custom-font-size has-xx-large-font-size"><a class="wp-block-button__link wp-element-button">My button 1</a></div>
-<!-- /wp:button -->
-
-<!-- wp:button {"style":{"typography":{"fontStyle":"normal","fontWeight":"800"}}} -->
-<div class="wp-block-button" style="font-style:normal;font-weight:800"><a class="wp-block-button__link wp-element-button">My button 2</a></div>
-<!-- /wp:button -->
-
-<!-- wp:button {"style":{"typography":{"letterSpacing":"39px"}}} -->
-<div class="wp-block-button" style="letter-spacing:39px"><a class="wp-block-button__link wp-element-button">My button 3</a></div>
-<!-- /wp:button -->
-
-<!-- wp:button {"tagName":"button","style":{"typography":{"letterSpacing":"39px"}}} -->
-<div class="wp-block-button" style="letter-spacing:39px"><button type="button" class="wp-block-button__link wp-element-button">My button 4</button></div>
-<!-- /wp:button --></div>
-<!-- /wp:buttons -->
+<!-- wp:button {"fontSize":"xx-large","style":{"typography":{"fontStyle":"normal","fontWeight":"800","letterSpacing":"39px"}}} -->
+<div class="wp-block-button has-custom-font-size has-xx-large-font-size" style="font-style:normal;font-weight:800;letter-spacing:39px"><a class="wp-block-button__link wp-element-button">My Button</a></div>
+<!-- /wp:button -->
\ No newline at end of file
diff --git a/test/integration/fixtures/blocks/core__button__deprecated-v12.json b/test/integration/fixtures/blocks/core__button__deprecated-v12.json
index b6cab91f0d5..7fd537b6575 100644
--- a/test/integration/fixtures/blocks/core__button__deprecated-v12.json
+++ b/test/integration/fixtures/blocks/core__button__deprecated-v12.json
@@ -1,72 +1,20 @@
 [
 	{
-		"name": "core/buttons",
+		"name": "core/button",
 		"isValid": true,
 		"attributes": {
-			"align": "wide",
-			"layout": {
-				"type": "flex",
-				"justifyContent": "center"
+			"tagName": "a",
+			"type": "button",
+			"text": "My Button",
+			"fontSize": "xx-large",
+			"style": {
+				"typography": {
+					"fontStyle": "normal",
+					"fontWeight": "800",
+					"letterSpacing": "39px"
+				}
 			}
 		},
-		"innerBlocks": [
-			{
-				"name": "core/button",
-				"isValid": true,
-				"attributes": {
-					"tagName": "a",
-					"type": "button",
-					"text": "My button 1",
-					"fontSize": "xx-large"
-				},
-				"innerBlocks": []
-			},
-			{
-				"name": "core/button",
-				"isValid": true,
-				"attributes": {
-					"tagName": "a",
-					"type": "button",
-					"text": "My button 2",
-					"style": {
-						"typography": {
-							"fontStyle": "normal",
-							"fontWeight": "800"
-						}
-					}
-				},
-				"innerBlocks": []
-			},
-			{
-				"name": "core/button",
-				"isValid": true,
-				"attributes": {
-					"tagName": "a",
-					"type": "button",
-					"text": "My button 3",
-					"style": {
-						"typography": {
-							"letterSpacing": "39px"
-						}
-					}
-				},
-				"innerBlocks": []
-			},
-			{
-				"name": "core/button",
-				"isValid": false,
-				"attributes": {
-					"tagName": "button",
-					"type": "button",
-					"text": "My button 4",
-					"style": {
-						"typography": {
-							"letterSpacing": "39px"
-						}
-					}
-				},
-				"innerBlocks": []
-			}
-		]
+		"innerBlocks": []
 	}
 ]
diff --git a/test/integration/fixtures/blocks/core__button__deprecated-v12.parsed.json b/test/integration/fixtures/blocks/core__button__deprecated-v12.parsed.json
index 466b53ae00e..542817c8428 100644
--- a/test/integration/fixtures/blocks/core__button__deprecated-v12.parsed.json
+++ b/test/integration/fixtures/blocks/core__button__deprecated-v12.parsed.json
@@ -1,84 +1,20 @@
 [
 	{
-		"blockName": "core/buttons",
+		"blockName": "core/button",
 		"attrs": {
-			"align": "wide",
-			"layout": {
-				"type": "flex",
-				"justifyContent": "center"
+			"fontSize": "xx-large",
+			"style": {
+				"typography": {
+					"fontStyle": "normal",
+					"fontWeight": "800",
+					"letterSpacing": "39px"
+				}
 			}
 		},
-		"innerBlocks": [
-			{
-				"blockName": "core/button",
-				"attrs": {
-					"fontSize": "xx-large"
-				},
-				"innerBlocks": [],
-				"innerHTML": "\n<div class=\"wp-block-button has-custom-font-size has-xx-large-font-size\"><a class=\"wp-block-button__link wp-element-button\">My button 1</a></div>\n",
-				"innerContent": [
-					"\n<div class=\"wp-block-button has-custom-font-size has-xx-large-font-size\"><a class=\"wp-block-button__link wp-element-button\">My button 1</a></div>\n"
-				]
-			},
-			{
-				"blockName": "core/button",
-				"attrs": {
-					"style": {
-						"typography": {
-							"fontStyle": "normal",
-							"fontWeight": "800"
-						}
-					}
-				},
-				"innerBlocks": [],
-				"innerHTML": "\n<div class=\"wp-block-button\" style=\"font-style:normal;font-weight:800\"><a class=\"wp-block-button__link wp-element-button\">My button 2</a></div>\n",
-				"innerContent": [
-					"\n<div class=\"wp-block-button\" style=\"font-style:normal;font-weight:800\"><a class=\"wp-block-button__link wp-element-button\">My button 2</a></div>\n"
-				]
-			},
-			{
-				"blockName": "core/button",
-				"attrs": {
-					"style": {
-						"typography": {
-							"letterSpacing": "39px"
-						}
-					}
-				},
-				"innerBlocks": [],
-				"innerHTML": "\n<div class=\"wp-block-button\" style=\"letter-spacing:39px\"><a class=\"wp-block-button__link wp-element-button\">My button 3</a></div>\n",
-				"innerContent": [
-					"\n<div class=\"wp-block-button\" style=\"letter-spacing:39px\"><a class=\"wp-block-button__link wp-element-button\">My button 3</a></div>\n"
-				]
-			},
-			{
-				"blockName": "core/button",
-				"attrs": {
-					"tagName": "button",
-					"style": {
-						"typography": {
-							"letterSpacing": "39px"
-						}
-					}
-				},
-				"innerBlocks": [],
-				"innerHTML": "\n<div class=\"wp-block-button\" style=\"letter-spacing:39px\"><button type=\"button\" class=\"wp-block-button__link wp-element-button\">My button 4</button></div>\n",
-				"innerContent": [
-					"\n<div class=\"wp-block-button\" style=\"letter-spacing:39px\"><button type=\"button\" class=\"wp-block-button__link wp-element-button\">My button 4</button></div>\n"
-				]
-			}
-		],
-		"innerHTML": "\n<div class=\"wp-block-buttons alignwide\">\n\n\n\n\n\n</div>\n",
+		"innerBlocks": [],
+		"innerHTML": "\n<div class=\"wp-block-button has-custom-font-size has-xx-large-font-size\" style=\"font-style:normal;font-weight:800;letter-spacing:39px\"><a class=\"wp-block-button__link wp-element-button\">My Button</a></div>\n",
 		"innerContent": [
-			"\n<div class=\"wp-block-buttons alignwide\">",
-			null,
-			"\n\n",
-			null,
-			"\n\n",
-			null,
-			"\n\n",
-			null,
-			"</div>\n"
+			"\n<div class=\"wp-block-button has-custom-font-size has-xx-large-font-size\" style=\"font-style:normal;font-weight:800;letter-spacing:39px\"><a class=\"wp-block-button__link wp-element-button\">My Button</a></div>\n"
 		]
 	}
 ]
diff --git a/test/integration/fixtures/blocks/core__button__deprecated-v12.serialized.html b/test/integration/fixtures/blocks/core__button__deprecated-v12.serialized.html
index 2df6789245e..1b18b70facc 100644
--- a/test/integration/fixtures/blocks/core__button__deprecated-v12.serialized.html
+++ b/test/integration/fixtures/blocks/core__button__deprecated-v12.serialized.html
@@ -1,17 +1,3 @@
-<!-- wp:buttons {"align":"wide","layout":{"type":"flex","justifyContent":"center"}} -->
-<div class="wp-block-buttons alignwide"><!-- wp:button {"fontSize":"xx-large"} -->
-<div class="wp-block-button has-custom-font-size"><a class="wp-block-button__link has-xx-large-font-size wp-element-button">My button 1</a></div>
+<!-- wp:button {"fontSize":"xx-large","style":{"typography":{"fontStyle":"normal","fontWeight":"800","letterSpacing":"39px"}}} -->
+<div class="wp-block-button has-custom-font-size"><a class="wp-block-button__link has-xx-large-font-size wp-element-button" style="font-style:normal;font-weight:800;letter-spacing:39px">My Button</a></div>
 <!-- /wp:button -->
-
-<!-- wp:button {"style":{"typography":{"fontStyle":"normal","fontWeight":"800"}}} -->
-<div class="wp-block-button"><a class="wp-block-button__link wp-element-button" style="font-style:normal;font-weight:800">My button 2</a></div>
-<!-- /wp:button -->
-
-<!-- wp:button {"style":{"typography":{"letterSpacing":"39px"}}} -->
-<div class="wp-block-button"><a class="wp-block-button__link wp-element-button" style="letter-spacing:39px">My button 3</a></div>
-<!-- /wp:button -->
-
-<!-- wp:button {"tagName":"button","style":{"typography":{"letterSpacing":"39px"}}} -->
-<div class="wp-block-button" style="letter-spacing:39px"><button type="button" class="wp-block-button__link wp-element-button">My button 4</button></div>
-<!-- /wp:button --></div>
-<!-- /wp:buttons -->

I hope that helps 🤞

packages/block-library/src/button/deprecated.js

aaronrobertshaw · 2024-12-17T01:38:33Z

packages/block-library/src/button/edit.js

@@ -301,6 +317,7 @@ function ButtonEdit( props ) {
 						...colorProps.style,
 						...spacingProps.style,
 						...shadowProps.style,
+						...typographyProps.style,


If writing-mode is best placed still on the wrapper, we could just change the __experimentalSkipSerialization value for typography to be an array of all the other typography features except writing mode.

packages/block-library/src/button/save.js

test/integration/fixtures/blocks/core__button__deprecated-v12.html

shimotmk · 2024-12-17T04:53:47Z

@ramonjd @aaronrobertshaw
Thanks for reviewing!

I adjusted the attributes and made core__button__deprecated-v12.html into a single button block.

However, the data for the fourth letterSpacing setting in core__button__deprecated-v12.html cannot be migrated.

Why is that?🤔

aaronrobertshaw · 2024-12-17T05:20:06Z

However, the data for the fourth letterSpacing setting in core__button__deprecated-v12.html cannot be migrated.

I'm not following this comment sorry. It looks fine to me in the lastest commit.

The current e2e failures relate to full post content fixture › core__button__deprecated-v10. As I noted in my review this fixture needing updating. The required change was also in the diff I provided.

After updating the v10 fixture, all the fixture tests pass for me.

Once that is sorted, there's still the issue with writing mode to fix up.

shimotmk · 2024-12-17T05:31:43Z

@aaronrobertshaw
Sorry. I was mistaken. The transition was going well.

WordPress#68023 (comment)

shimotmk · 2024-12-17T09:27:06Z

@ramonjd @aaronrobertshaw
I adjusted the writing-mode to add inline styles to the outer div 🙂

packages/block-library/src/button/edit.js

packages/block-library/src/button/block.json

shimotmk · 2024-12-20T05:20:06Z

@ramonjd @aaronrobertshaw
Any other things I need to fix?

aaronrobertshaw · 2024-12-20T06:23:09Z

Any other things I need to fix?

@shimotmk the issue I flagged in my last two reviews still needs fixing.

The current e2e failures relate to full post content fixture › core__button__deprecated-v10. As I noted in my #68023 (review) this fixture needing updating. The required change was also in the diff I provided.

WordPress#68023 (review)

shimotmk · 2024-12-20T10:58:08Z

@aaronrobertshaw
Thanks for the reply

Moved the has-custom-font-size class internally.
And updated test/integration/fixtures/blocks/core__button__deprecated-v10.serialized.html
Is this OK?

ramonjd · 2024-12-22T07:38:23Z

Thanks for the continued work on this PR @shimotmk 🙇🏻

It looks like the mobile unit tests fails are related to the button block: https://github.com/WordPress/gutenberg/actions/runs/12429816613/job/34704026313?pr=68023#step:6:2906

shimotmk · 2024-12-22T08:52:40Z

@ramonjd
Thanks for the reply
I don't know how to fix the mobile unit.
How do I fix it?

Fixes WordPress#64662

ed06ca9

shimotmk requested review from ajitbohra and fabiankaegy as code owners December 16, 2024 11:49

shimotmk mentioned this pull request Dec 16, 2024

Button Block: Set proper typography for inner elements #64869

Open

ramonjd reviewed Dec 17, 2024

View reviewed changes

aaronrobertshaw reviewed Dec 17, 2024

View reviewed changes

fix

aaaa452

fix writing-mode

c5b5dee

WordPress#68023 (comment)

ramonjd reviewed Dec 17, 2024

View reviewed changes

packages/block-library/src/button/edit.js Show resolved Hide resolved

packages/block-library/src/button/block.json Outdated Show resolved Hide resolved

fix WordPress#68023 (comment)

e0fbfd4

fix

0f0013d

WordPress#68023 (review)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Button Block: Set proper typography for inner elements #68023

Button Block: Set proper typography for inner elements #68023

shimotmk commented Dec 16, 2024 •

edited

Loading

github-actions bot commented Dec 16, 2024

github-actions bot commented Dec 16, 2024 •

edited

Loading

ramonjd Dec 17, 2024

aaronrobertshaw Dec 17, 2024

ramonjd Dec 17, 2024

aaronrobertshaw left a comment

aaronrobertshaw Dec 17, 2024

shimotmk commented Dec 17, 2024

aaronrobertshaw commented Dec 17, 2024

shimotmk commented Dec 17, 2024

shimotmk commented Dec 17, 2024

shimotmk commented Dec 20, 2024

aaronrobertshaw commented Dec 20, 2024

shimotmk commented Dec 20, 2024

ramonjd commented Dec 22, 2024

shimotmk commented Dec 22, 2024

Button Block: Set proper typography for inner elements #68023

Are you sure you want to change the base?

Button Block: Set proper typography for inner elements #68023

Conversation

shimotmk commented Dec 16, 2024 • edited Loading

What?

Why?

How?

Testing Instructions

Testing Instructions for Keyboard

Screenshots or screencast

github-actions bot commented Dec 16, 2024

github-actions bot commented Dec 16, 2024 • edited Loading

ramonjd Dec 17, 2024

Choose a reason for hiding this comment

aaronrobertshaw Dec 17, 2024

Choose a reason for hiding this comment

ramonjd Dec 17, 2024

Choose a reason for hiding this comment

aaronrobertshaw left a comment

Choose a reason for hiding this comment

aaronrobertshaw Dec 17, 2024

Choose a reason for hiding this comment

shimotmk commented Dec 17, 2024

aaronrobertshaw commented Dec 17, 2024

shimotmk commented Dec 17, 2024

shimotmk commented Dec 17, 2024

shimotmk commented Dec 20, 2024

aaronrobertshaw commented Dec 20, 2024

shimotmk commented Dec 20, 2024

ramonjd commented Dec 22, 2024

shimotmk commented Dec 22, 2024

shimotmk commented Dec 16, 2024 •

edited

Loading

github-actions bot commented Dec 16, 2024 •

edited

Loading