Add/button block border support #41147

tellthemachines · 2025-01-17T05:09:05Z

Fixes #40696

Proposed changes:

Adds border support to the Buttons block.

WIP - perhaps should add shadow support too? It's not explicit in the original issue.

Not sure if tests are applicable for this change. Guidance appreciated! It's my first PR to this repo 😊

I tested these changes locally by building the Jetpack env as per these instructions.

Other information:

Have you written new tests for your changes, if applicable?
Have you checked the E2E test CI results, and verified that your changes do not break them?
Have you tested your changes on WordPress.com, if applicable (if so, you'll see a generated comment below with a script to run)?

Jetpack product discussion

Does this pull request change what data or activity we track or use?

Testing instructions:

Open the post editor and add a Form block. Choose any variation.
Select the submit button, and in the block sidebar switch to the "Styles" tab.
Check that the Border controls are there, and try adding some values.
Save and check that the border displays the same in editor and front end.

github-actions · 2025-01-17T05:09:50Z

Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.

To test on WoA, go to the Plugins menu on a WoA dev site. Click on the "Upload" button and follow the upgrade flow to be able to upload, install, and activate the Jetpack Beta plugin. Once the plugin is active, go to Jetpack > Jetpack Beta, select your plugin (Jetpack), and enable the add/button-block-border-support branch.
To test on Simple, run the following command on your sandbox:

bin/jetpack-downloader test jetpack add/button-block-border-support

Interested in more tips and information?

In your local development environment, use the jetpack rsync command to sync your changes to a WoA dev blog.
Read more about our development workflow here: PCYsg-eg0-p2
Figure out when your changes will be shipped to customers here: PCYsg-eg5-p2

github-actions · 2025-01-17T05:10:01Z

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

✅ Include a description of your PR changes.
✅ Add a "[Status]" label (In Progress, Needs Review, ...).
✅ Add a "[Type]" label (Bug, Enhancement, Janitorial, Task).
✅ Add testing instructions.
✅ Specify whether this PR includes any changes to data or privacy.
✅ Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.

Follow this PR Review Process:

Ensure all required checks appearing at the bottom of this PR are passing.
Make sure to test your changes on all platforms that it applies to. You're responsible for the quality of the code you ship.
You can use GitHub's Reviewers functionality to request a review.
When it's reviewed and merged, you will be pinged in Slack to deploy the changes to WordPress.com simple once the build is done.

If you have questions about anything, reach out in #jetpack-developers for guidance!

Jetpack plugin:

No scheduled milestone found for this plugin.

If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack.

projects/plugins/jetpack/extensions/blocks/button/button.php

tellthemachines · 2025-01-21T00:16:51Z

I can't get the Jetpack Beta tester plugin to recognise this branch, so I'm going to switch the PR to ready to review to see if it makes a difference.

tellthemachines · 2025-01-21T05:54:35Z

projects/plugins/jetpack/extensions/blocks/button/button.php

@@ -211,6 +217,36 @@ function get_button_styles( $attributes ) {
 		$styles[] = 'max-width: 100%';
 	}

+	if ( $has_named_border_color ) {
+		$border_styles['color'] = "var:preset|color|{$attributes['borderColor']}";


This doesn't seem to be working, although looking at the style engine output, $attributes['borderColor'] only contains the last bit of the preset string.
Looking at what's happening over in core, that particular style doesn't seem to be output by the border support either; for preset colors the front end styling depends on the preset classname and its corresponding CSS rule.

I think we'll have to ensure the classname is output on the front end instead.

Ok I've fixed it to output classname instead of styles.

tellthemachines · 2025-01-21T06:11:31Z

I've iterated on the feedback and tested the editor changes in wp.com, both on simple and atomic.

A design question about these changes: should I attempt to scrap the existing border radius control in the settings tab in favour of the native block support one? I'm not sure what back compat issues that might create.
Alternatively, it might be possible to move the existing border radius control into the styles tab. Or we could do nothing :)

ramonjd

This is generally working as described for me.

✅ controls appear in the editor
✅ style look good in editor and frontend
✅ i can overwrite the button styles in global styles

I see a jetpack button block peculiarity that overrides all border styles for buttons without the is-style-outline classname.

I was wondering why my styles wouldn't work!

projects/plugins/jetpack/extensions/blocks/button/button.php

jp-launch-control · 2025-03-04T02:43:41Z

Code Coverage Summary

Coverage changed in 3 files.

File	Coverage	Δ%	Δ Uncovered
projects/plugins/jetpack/extensions/blocks/button/button.php	0/145 (0.00%)	0.00%	29 💔
projects/plugins/jetpack/extensions/blocks/button/save.js	3/12 (25.00%)	-2.27%	1 ❤️‍🩹
projects/plugins/jetpack/extensions/blocks/button/edit.js	9/10 (90.00%)	1.11%	0 💚

Full summary · PHP report · JS report

_{If appropriate, add one of these labels to override the failing coverage check:

Covered by non-unit tests
Use to ignore the Code coverage requirement check when E2Es or other non-unit tests cover the code

Coverage tests to be added later
Use to ignore the Code coverage requirement check when tests will be added in a follow-up PR

I don't care about code coverage for this PR
Use this label to ignore the check for insufficient code coveage.}

Co-authored-by: Ramon <[email protected]>

tellthemachines · 2025-03-04T03:04:45Z

I see a jetpack button block peculiarity that overrides all border styles for buttons without the is-style-outline classname

Yeah might need to reduce specificity on that rule.

aaronrobertshaw

I've given this a further test after the recent conflicts were resolved.

All that's left I think is a minor tweak to the default Jetpack forms styles so they don't override the button support border-style defaults.

Editor	Frontend

Maybe something like:

diff --git a/projects/plugins/jetpack/extensions/blocks/button/view.scss b/projects/plugins/jetpack/extensions/blocks/button/view.scss
index e3bce484b9..2b5673303d 100644
--- a/projects/plugins/jetpack/extensions/blocks/button/view.scss
+++ b/projects/plugins/jetpack/extensions/blocks/button/view.scss
@@ -17,7 +17,7 @@
 		color: currentColor;
 	}
 
-	&:not(.is-style-outline) button {
+	:where(&:not(.is-style-outline) button) {
 		border: none;
 	}
 }
\ No newline at end of file

aaronrobertshaw · 2025-03-04T03:13:07Z

A design question about these changes: should I attempt to scrap the existing border radius control in the settings tab in favour of the native block support one? I'm not sure what back compat issues that might create.

To me, it makes sense to consolidate the border styles into a single location under the styles tab. Any backward compatibility issues should be able to be handled via block deprecations here.

aaronrobertshaw

One thing I missed in my earlier testing was Global Styles, sorry. It looks like we'll need a custom selector added to the block's selectors to apply the border to the inner element.

Also, the naming of the block makes the Global Styles > Blocks results a bit confusing, but that isn't something related to this PR in particular.

tellthemachines · 2025-03-04T06:00:32Z

It looks like we'll need a custom selector added to the block's selectors to apply the border to the inner element.

Added! Though it's not working for me on the front end 🤔

aaronrobertshaw · 2025-03-04T06:07:43Z

Added! Though it's not working for me on the front end

The server-side block registration will need updating as well. However, quickly testing that it still isn't working.

I'll have a little more of a dig to see if I can confirm an upstream bug 🕵️

aaronrobertshaw · 2025-03-04T07:14:38Z

The server-side block registration will need updating as well. However, quickly testing that it still isn't working.

🤦 After an embarrassing amount of time following the code, I realised that the global styles were being cached on the front end, and the server-side block registration tweak did in fact work.

I've pushed that change as it's working for me.

Screen.Recording.2025-03-04.at.5.12.35.pm.mp4

aaronrobertshaw · 2025-03-04T07:49:15Z

Pushed another tweak to fix border reset style when the button block is rendered as a button.

The earlier tweak in d471962 still had a specificity of 0-1-1 which would override global styles.

This is getting pretty close 🤞

✅ Flat borders work
✅ Individual borders per side work
✅ Consistent between editor and frontend
✅ Box sizing appears to work for parent enforced widths
✅ Global styles work in editor and frontend
✅ Block instance styles override Global Styles
✅ Default styles removing border still apply when no custom styling and outline block style isn't selected

❓ The only remaining issue I ran into was some default styles overriding global styles when the is-style-outline block style is applied. Their specificity would also override any block style variation styles generated by global styles as well.

I think the Jetpack button block might have more issues regarding supporting block style variations through global styles. Perhaps that could be done as a followup?

In case it's of use, here's a diff lowering specificity of defaults to allow global styles borders to apply regardless of outline block style.

CSS Diff (Note: we need a better solution than this)

diff --git a/projects/plugins/jetpack/extensions/blocks/button/editor.scss b/projects/plugins/jetpack/extensions/blocks/button/editor.scss
index 124d209d52..034a127fa3 100644
--- a/projects/plugins/jetpack/extensions/blocks/button/editor.scss
+++ b/projects/plugins/jetpack/extensions/blocks/button/editor.scss
@@ -9,11 +9,14 @@
 
 	&.is-style-outline > .wp-block-button__link {
 		background-color: transparent;
-		border: solid 1px currentColor;
 		color: currentColor;
 	}
 }
 
+:root :where(.wp-block-jetpack-button.is-style-outline .wp-block-button__link ) {
+	border: solid 1px currentColor;
+}
+
 // Support align feature for older themes
 .wp-block[data-align]:has(> .wp-block-jetpack-button) {
 	margin-left: 0;
diff --git a/projects/plugins/jetpack/extensions/blocks/button/view.scss b/projects/plugins/jetpack/extensions/blocks/button/view.scss
index 09cd0b8d4b..9dd4b582f2 100644
--- a/projects/plugins/jetpack/extensions/blocks/button/view.scss
+++ b/projects/plugins/jetpack/extensions/blocks/button/view.scss
@@ -13,10 +13,13 @@
 
 	&.is-style-outline > .wp-block-button__link {
 		background-color: transparent;
-		border: solid 1px currentColor;
 		color: currentColor;
 	}
 	
+	:where(&.is-style-outline > .wp-block-button__link) {
+		border: solid 1px currentColor;
+	}
+	
 	:where(&:not(.is-style-outline) button) {
  		border: none;
  	}

Also, these default outline styles are just broken for some buttons in TT5 (and probably more themes).

Screen.Recording.2025-03-04.at.5.46.58.pm.mp4

Steps to replicate:

Activate TT5
Add Calendly block
Switch Calendly block to button style via block inspector controls
Select block and apply outline block style

tellthemachines · 2025-03-04T23:36:54Z

Thanks @aaronrobertshaw !

I think the Jetpack button block might have more issues regarding supporting block style variations through global styles. Perhaps that could be done as a followup?

I wouldn't worry too much about that, it should be fine to add them as a follow-up. Would those issues be specific to border support or would they affect other styling options?

aaronrobertshaw · 2025-03-05T07:21:31Z

Would those issues be specific to border support or would they affect other styling options?

Without diving into the code, I suspect they'll prevent any global styles for block style variations to apply. Then there's the fact that the default styles for the outline block style can result in invisible buttons e.g. on TT5.

A dedicated follow-up to fix or improve the button block's default styles would make a lot of sense as all these styles are pretty interconnected.

aaronrobertshaw

If we're happy to address block style variations (and default styling) in a follow-up, I think this PR is good to go.

In that spirit, I'm happy to give this an approval ✅. Feel free to get a second opinion if you feel it is warranted.

tellthemachines · 2025-03-05T23:56:48Z

If we're happy to address block style variations (and default styling) in a follow-up,

Yes, I think that makes sense.

github-actions bot added [Block] Button [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ [Status] In Progress labels Jan 17, 2025

github-actions bot added the [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. label Jan 17, 2025

aaronrobertshaw reviewed Jan 17, 2025

View reviewed changes

projects/plugins/jetpack/extensions/blocks/button/button.php Outdated Show resolved Hide resolved

tellthemachines added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Status] Needs Team Review Obsolete. Use Needs Review instead. labels Jan 20, 2025

tellthemachines marked this pull request as ready for review January 21, 2025 00:16

tellthemachines commented Jan 21, 2025

View reviewed changes

ramonjd assigned tellthemachines Feb 4, 2025

ramonjd reviewed Mar 3, 2025

View reviewed changes

projects/plugins/jetpack/extensions/blocks/button/button.php Outdated Show resolved Hide resolved

tellthemachines added 6 commits March 4, 2025 12:18

Button block: add border support

38edd38

changelog

1219cd2

Add per-side support and remove duplicate radius

5732c97

Fix shorthand properties output.

9d7fdd1

Fix preset colors

f88b908

make PHPs more nicer

099816c

tellthemachines force-pushed the add/button-block-border-support branch from 1b98857 to 099816c Compare March 4, 2025 02:35

tellthemachines and others added 2 commits March 4, 2025 13:54

Remove extra prop left over from rebase

3b7cd53

dedupe style engine call

d18c17a

Co-authored-by: Ramon <[email protected]>

aaronrobertshaw reviewed Mar 4, 2025

View reviewed changes

Default rule shouldn't override border styles

d471962

aaronrobertshaw reviewed Mar 4, 2025

View reviewed changes

Add selector for global styles

7930503

Add selectors config to server-side button block registration

df21708

Prevent border reset overriding global styles

4a4eae8

aaronrobertshaw approved these changes Mar 5, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add/button block border support #41147

Add/button block border support #41147

tellthemachines commented Jan 17, 2025 •

edited

Loading

github-actions bot commented Jan 17, 2025 •

edited

Loading

github-actions bot commented Jan 17, 2025 •

edited

Loading

tellthemachines commented Jan 21, 2025

tellthemachines Jan 21, 2025

tellthemachines Jan 21, 2025

tellthemachines commented Jan 21, 2025

ramonjd left a comment

jp-launch-control bot commented Mar 4, 2025 •

edited

Loading

tellthemachines commented Mar 4, 2025

aaronrobertshaw left a comment

aaronrobertshaw commented Mar 4, 2025

aaronrobertshaw left a comment •

edited

Loading

tellthemachines commented Mar 4, 2025

aaronrobertshaw commented Mar 4, 2025

aaronrobertshaw commented Mar 4, 2025

aaronrobertshaw commented Mar 4, 2025

tellthemachines commented Mar 4, 2025

aaronrobertshaw commented Mar 5, 2025

aaronrobertshaw left a comment

tellthemachines commented Mar 5, 2025

Add/button block border support #41147

Are you sure you want to change the base?

Add/button block border support #41147

Conversation

tellthemachines commented Jan 17, 2025 • edited Loading

Proposed changes:

Other information:

Jetpack product discussion

Does this pull request change what data or activity we track or use?

Testing instructions:

github-actions bot commented Jan 17, 2025 • edited Loading

github-actions bot commented Jan 17, 2025 • edited Loading

tellthemachines commented Jan 21, 2025

tellthemachines Jan 21, 2025

Choose a reason for hiding this comment

tellthemachines Jan 21, 2025

Choose a reason for hiding this comment

tellthemachines commented Jan 21, 2025

ramonjd left a comment

Choose a reason for hiding this comment

jp-launch-control bot commented Mar 4, 2025 • edited Loading

Code Coverage Summary

tellthemachines commented Mar 4, 2025

aaronrobertshaw left a comment

Choose a reason for hiding this comment

aaronrobertshaw commented Mar 4, 2025

aaronrobertshaw left a comment • edited Loading

Choose a reason for hiding this comment

tellthemachines commented Mar 4, 2025

aaronrobertshaw commented Mar 4, 2025

aaronrobertshaw commented Mar 4, 2025

aaronrobertshaw commented Mar 4, 2025

tellthemachines commented Mar 4, 2025

aaronrobertshaw commented Mar 5, 2025

aaronrobertshaw left a comment

Choose a reason for hiding this comment

tellthemachines commented Mar 5, 2025

tellthemachines commented Jan 17, 2025 •

edited

Loading

github-actions bot commented Jan 17, 2025 •

edited

Loading

github-actions bot commented Jan 17, 2025 •

edited

Loading

jp-launch-control bot commented Mar 4, 2025 •

edited

Loading

aaronrobertshaw left a comment •

edited

Loading