Jetpack Button Block: Ensure block enqueues core Button block styles as a dependency #19595

andrewserong · 2021-04-21T07:46:13Z

As tracked in #19464 the Jetpack Button block depends on styles from the core Button block (via the class name .wp-block-button__link. In an FSE theme like TT1-blocks, no styles are provided to override the button block styling, so it becomes apparent that when no core Button block is inserted into a template part, then the Jetpack button block appears unstyled.

To address this, we can ensure that the core Button block styles are always enqueued when a Jetpack Button block is rendered on the server.

Possible fix for #19464 and related to #19504

Changes proposed in this Pull Request:

Always ensure the core Button Block styles are enqueued when rendering a Jetpack Button block
Provide a default for the Button border (inherit), to avoid the browser default button border from being added

Jetpack product discussion

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

No

Before	After

Ensuring no regressions on a non-FSE site, checking that outline style and custom colours still works:

Testing instructions:

In an FSE site with the TT1-blocks theme active, insert a block that uses the Jetpack Button block (e.g. Mailchimp or Contact Form) into a template part like the footer template part.
Save the template.
View the front end of the site — you should see the default styling of the Button block is applied to the Jetpack Button block
On a non-FSE theme and site, using a Varia theme like Barnsbury, insert one of the blocks that uses the Jetpack Button block, and ensure that on the front end it still renders as you'd expect using the theme's styles for the button

…as a dependency

matticbot · 2021-04-21T07:46:19Z

Caution: This PR has changes that must be merged to WordPress.com
Hello andrewserong! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer and confirm D60465-code works as expected before merging this PR. Once this PR is merged, please commit the changes to WP.com. Thank you!
This revision will be updated with each commit to this PR

github-actions · 2021-04-21T07:48:22Z

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.
✅ All commits were linted before commit.
✅ Add a "[Status]" label (In Progress, Needs Team Review, ...).
✅ 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 🤖

Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped.
Then, add the "[Status] Needs Team review" label and ask someone from your team review the code.
Once you’ve done so, switch to the "[Status] Needs Review" label; someone from Jetpack Crew will then review this PR and merge it to be included in the next Jetpack release.

Jetpack plugin:

Next scheduled release: May 4, 2021.
Scheduled code freeze: April 26, 2021.

andrewserong · 2021-04-21T10:34:02Z

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

@@ -39,10 +39,15 @@ function register_block() {
 function render_block( $attributes, $content ) {
 	$save_in_post_content = get_attribute( $attributes, 'saveInPostContent' );

-	if ( Blocks::is_amp_request() ) {


Note: this AMP is removed here. Previously we only loaded the block's style if it's an AMP request because the only styles we needed to load were targeted for AMP. We now have a style rule that should be applied when not in an AMP request, so I think the conditional is no longer needed.

stacimc

This tested mostly well for me, and looks like it will allow us to handle some of the related issues (Mailchimp for example) more easily!

I tested with Contact Form, Mailchimp, Donations, Payments, and Premium Content. I tried to test with Pay with Paypal but it is currently pretty broken/being worked on. A couple of things I noticed:

Button alignment does not appear to be working correctly. In FSE for example, I inserted a Contact Form and tried to make the button 50% width and center-aligned. The width applies but the alignment does not:

I also noticed that the Premium Content buttons are stacked on top of each other, and width/alignment do not function correctly:

Do you think it would be worth also enqueuing the Buttons block styles as well? I believe that would at least fix the vertical stacking on the Premium Content buttons, although I'm inclined to say most of the above should be separate issues from this PR -- it looks like the styles for alignment classes aren't all loaded, and some of the Premium Content issues appear to be block-specific.

The base styles should instead come from the jetpack button (#19595)

andrewserong · 2021-04-21T22:47:06Z

Thanks for testing this out @stacimc! I'll have a play around with enqueuing the Buttons block styles, too. I think if the Jetpack Button block was designed to depend on the styling from those blocks, it's fair to enqueue them as well.

…ock, include editor styles for right alignment

andrewserong · 2021-04-22T06:39:07Z

@stacimc thanks again for the thorough testing! I've updated this to also enqueue the core/buttons styles, which appears to fix the stacking buttons issue with the premium content block:

For the issue with center alignment, it looks like this has been fixed in Gutenberg and will land in GB 10.5 (I double-checked on wpcom with the Gutenberg Edge sticker activated, which is now running 10.5.0 RC1, and it looks like it's working). The PR that fixed it is: WordPress/gutenberg#30525

I've also added a CSS rule for right alignment for the Jetpack Button block within the editor (I noticed right alignment wasn't displaying correctly for e.g. the button block in the Mailchimp block).

There are a bunch of other little issues with button alignment, but like you mention, these are probably better tackled as separate issues/PRs for the individual blocks affected. After this PR lands, I'm happy to work through some of those to fix them up a bit. It might step slightly outside of scope for FSE strictly-speaking, but it'd be good to get some of them sorted.

Do you think this PR is looking okay now for a first pass, or do you think it needs tweaking / addressing other issues too?

stacimc

This looks great and will have a big impact 🥳 Tests well for me and I totally agree about the rest being handled in separate PRs. Thanks!

jeherve

This looks good to me. 🚢

github-actions · 2021-04-26T07:51:40Z

Great news! One last step: head over to your WordPress.com diff, D60465-code, and commit it.
Once you've done so, come back to this PR and add a comment with your changeset ID.

Thank you!

The base styles should instead come from the jetpack button (#19595)

andrewserong · 2021-04-27T04:46:29Z

r224816-wpcom

oskosk · 2022-03-31T14:14:34Z

projects/plugins/jetpack/extensions/blocks/button/view.scss

@@ -1,3 +1,7 @@
 .amp-wp-article .wp-block-jetpack-button {
 	color: #ffffff;
 }
+
+.wp-block-jetpack-button button {
+	border: inherit;


This change seems to be related to Automattic/themes#5776 (comment)

Jetpack Button Block: Ensure block enqueues core Button block styles …

52fbfb6

…as a dependency

matticbot added the Touches WP.com Files label Apr 21, 2021

andrewserong self-assigned this Apr 21, 2021

andrewserong added [Status] Needs Team Review [Block] Button labels Apr 21, 2021

github-actions bot added the [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ label Apr 21, 2021

andrewserong mentioned this pull request Apr 21, 2021

Site Editor Compatibility: Blocks that depend on the Button block styling #19464

Closed

7 tasks

Check that the button block is set before attempting to access its style

5a37213

andrewserong commented Apr 21, 2021

View reviewed changes

stacimc reviewed Apr 21, 2021

View reviewed changes

stacimc added a commit that referenced this pull request Apr 21, 2021

Remove base styles for button

41b27ad

The base styles should instead come from the jetpack button (#19595)

stacimc mentioned this pull request Apr 21, 2021

Mailchimp Block: Fix styling issue with FSE #19566

Merged

Add core/blocks style dependency to fix issue with premium content bl…

0098b53

…ock, include editor styles for right alignment

stacimc approved these changes Apr 22, 2021

View reviewed changes

This was referenced Apr 22, 2021

Site Editor Compatibility: Revue Block styling issues on front end #19485

Closed

Site Editor Compatibility: Tracking issue for remaining fixes for Jetpack block FSE support #19504

Closed

andrewserong added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Status] Needs Team Review labels Apr 23, 2021

jeherve added this to the jetpack/9.7 milestone Apr 23, 2021

jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Apr 23, 2021

jeherve approved these changes Apr 23, 2021

View reviewed changes

jeherve merged commit 8e23242 into master Apr 26, 2021

jeherve deleted the update/jetpack-button-block-to-enqueue-core-button-style branch April 26, 2021 07:50

github-actions bot removed the [Status] Ready to Merge Go ahead, you can push that green button! label Apr 26, 2021

stacimc added a commit that referenced this pull request Apr 26, 2021

[not verified] Remove base styles for button

86bbbdb

The base styles should instead come from the jetpack button (#19595)

oskosk reviewed Mar 31, 2022

View reviewed changes

jffng mentioned this pull request Mar 31, 2022

Remove border CSS from button block #23734

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Jetpack Button Block: Ensure block enqueues core Button block styles as a dependency #19595

Jetpack Button Block: Ensure block enqueues core Button block styles as a dependency #19595

andrewserong commented Apr 21, 2021

matticbot commented Apr 21, 2021

github-actions bot commented Apr 21, 2021 •

edited

Loading

andrewserong Apr 21, 2021

stacimc left a comment

andrewserong commented Apr 21, 2021

andrewserong commented Apr 22, 2021

stacimc left a comment

jeherve left a comment

github-actions bot commented Apr 26, 2021

andrewserong commented Apr 27, 2021

oskosk Mar 31, 2022

Jetpack Button Block: Ensure block enqueues core Button block styles as a dependency #19595

Jetpack Button Block: Ensure block enqueues core Button block styles as a dependency #19595

Conversation

andrewserong commented Apr 21, 2021

Changes proposed in this Pull Request:

Jetpack product discussion

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

Testing instructions:

matticbot commented Apr 21, 2021

github-actions bot commented Apr 21, 2021 • edited Loading

andrewserong Apr 21, 2021

Choose a reason for hiding this comment

stacimc left a comment

Choose a reason for hiding this comment

andrewserong commented Apr 21, 2021

andrewserong commented Apr 22, 2021

stacimc left a comment

Choose a reason for hiding this comment

jeherve left a comment

Choose a reason for hiding this comment

github-actions bot commented Apr 26, 2021

andrewserong commented Apr 27, 2021

oskosk Mar 31, 2022

Choose a reason for hiding this comment

github-actions bot commented Apr 21, 2021 •

edited

Loading