Skip to content
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

Button: Add almost all missing typography supports #43649

Conversation

ndiego
Copy link
Member

@ndiego ndiego commented Aug 26, 2022

Related:

What?

Adds missing typography supports to the Quote block.

Why?

  • Improves consistency of our design tools across blocks.

How?

  • Opts into the missing line-height, font-weight, font-style, and letter-spacing support.

Note, that this PR does not include text-decoration. Text decoration is automatically set to none and cannot be overridden by the corresponding block support. How to best implement this, or if it should even be implemented, should be explored in a separate PR.

Testing Instructions

  1. Create a Buttons block and add a few Button blocks
  2. Check that the line-height, font-weight, font-style, and letter-spacing controls are now all available.
  3. Test various typography settings ensuring styles are applied in the editor.
  4. Save and confirm the application on the frontend.
  5. Switch to the site editor, and in a template create a Buttons block and add a few Button blocks
  6. Navigate to Global Styles > Blocks > Button > Typography
  7. Test applying a new typography controls and confirm it's applied in the preview and on the frontend.
  8. Test styling via theme.json

Screenshots

button-typography

@ndiego ndiego self-assigned this Aug 26, 2022
@ndiego ndiego added [Block] Buttons Affects the Buttons Block [Type] Enhancement A suggestion for improvement. [Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi [Feature] Typography Font and typography-related issues and PRs labels Aug 26, 2022
@ndiego ndiego changed the title Add missing typography supports to the Button block. Button: Add almost all missing typography supports Aug 26, 2022
Copy link
Contributor

@aaronrobertshaw aaronrobertshaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for getting the ball rolling on button typography @ndiego 👍

What's added so far works ok for me. I think we can add text-decoration as well though to keep the options here more consistent with other blocks.

The following diff might provide a basis for achieving that.

Example addition of text decoration
diff --git a/packages/block-library/src/button/block.json b/packages/block-library/src/button/block.json
index c13de8aa57..6076db9b8f 100644
--- a/packages/block-library/src/button/block.json
+++ b/packages/block-library/src/button/block.json
@@ -73,6 +73,7 @@
 			"__experimentalFontWeight": true,
 			"__experimentalFontStyle": true,
 			"__experimentalTextTransform": true,
+			"__experimentalTextDecoration": true,
 			"__experimentalLetterSpacing": true,
 			"__experimentalDefaultControls": {
 				"fontSize": true
diff --git a/packages/block-library/src/button/editor.scss b/packages/block-library/src/button/editor.scss
index 7ea7a64972..05c404a5d2 100644
--- a/packages/block-library/src/button/editor.scss
+++ b/packages/block-library/src/button/editor.scss
@@ -75,3 +75,7 @@
 div[data-type="core/button"] {
 	display: table;
 }
+
+.editor-styles-wrapper .wp-block-button[style*="text-decoration"] .wp-block-button__link {
+	text-decoration: inherit;
+}
diff --git a/packages/block-library/src/button/style.scss b/packages/block-library/src/button/style.scss
index 58fe01e12e..cc553625f8 100644
--- a/packages/block-library/src/button/style.scss
+++ b/packages/block-library/src/button/style.scss
@@ -34,6 +34,10 @@ $blocks-block__margin: 0.5em;
 	padding: calc(0.667em + 2px) calc(1.333em + 2px);
 }
 
+.wp-block-button[style*="text-decoration"] .wp-block-button__link {
+	text-decoration: inherit;
+}
+
 // Increased specificity needed to override margins.
 .wp-block-buttons > .wp-block-button {
 	&.has-custom-width {

The only catch I found in a quick test of this was that if a theme author adds text decoration via theme.json the end user won't be able to set text-decoration: none as the control wasn't designed to provide that option. I don't think that is a blocker for this here though.

@randhirexpresstech
Copy link
Contributor

@ndiego the same PR was already created long time ago and tagged as the [Type] Enhancement A suggestion for improvement.label on 9 Jun by you. Is there any specific reason to re create the same PR again?

@aaronrobertshaw
Copy link
Contributor

@ndiego & @randhirexpresstech, in the interests of getting typography support for Button and Buttons blocks into 6.1, and given they are tightly coupled together, I've created an alternate PR that opts into all the typography supports for both blocks and tweaks their styles and classes as needed.

It should be easier and quicker to test/review both the blocks together, particularly with some of the quirks of their overlapping styles and not wishing to remove styles that themes might rely on e.g. text-decoration: none on the Button block.

I'd also like to note that both your work in getting the ball rolling on button typography is appreciated. 🙇

In light of the new PR, would you have any objections to closing this PR in favour of #43934 @ndiego?

@aaronrobertshaw
Copy link
Contributor

#43934 has been merged which adds typography support to both Button and Buttons blocks. As a result, I'll close this.

Thanks everyone for the work, testing and reviews to get typography supports on buttons 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Buttons Affects the Buttons Block [Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi [Feature] Typography Font and typography-related issues and PRs [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants