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

Jetpack button: fix width and alignment settings #41139

Draft
wants to merge 3 commits into
base: trunk
Choose a base branch
from

Conversation

monsieur-z
Copy link
Contributor

@monsieur-z monsieur-z commented Jan 16, 2025

Setting the width and alignment of a jetpack/button block doesn't work consistently, depending on how it is integrated with its parent block and whether we're in the editor or site frontend.

For instance, in a Form block, setting the button width to 50% in the editor shrinks it instead of making it half the container's width.
Screenshot 2025-01-16 at 3 32 57 PM

In a Mailchimp block, trying to align the button (to the right in the capture below) doesn't work.
Screenshot 2025-01-16 at 3 33 08 PM

The following blocks use the jetpack/button block and show one or both faulty behaviors we've highlighted: Form, Premium Content, Mailchimp, Calendly, Eventbrite, Recipe, and Recurring Payments. The AI Assistant might as well, though I haven't been able to experience it.

Proposed changes:

To address that, this PR does two things in the jetpack/button block:

  1. It removes a class-less div from the markup generated by the block
Screenshot 2025-01-16 at 3 37 38 PM

This div prevents the flexbox layout applied to .block-editor-block-list__layout from working as intended.

  1. It applies the selected width to .wp-block-button instead of .wp-block-button__link.

Indeed, this Gutenberg PR forces .wp-block-button__link to have a width of 100%, breaking the width settings behavior.

Furthermore, the PR also updates the blocks consuming jetpack/button so the width and alignment settings work as expected. Here are some caveats:

  • Setting the width or alignment of jetpack/button in the Recipe block doesn't work as one would expect, given the width is relative to the grid items of the block. Solving this should be part of a broader discussion.
  • Similarly, aligning the jetpack/button of a Form is confusing when it's not in its row. It deserves a separate discussion too.
  • The alignment of buttons in a similar row is not implemented (Premium Content and Recurring Payments blocks). Again, this should be a separate concern.
  • The styles of the Eventbrite block are not loaded in the frontend. This should be addressed in a separate PR.

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

n/a

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

No.

Testing instructions:

  • Spin up a test Jetpack site
  • Create a new post
  • Add the following blocks: Form, Premium Content, Mailchimp, Calendly, Eventbrite, and Recurring Payments. You'll need to connect various accounts. It's not worth adding the Recipe block (see caveat above)
  • For Calendly and Eventbrite, select the view that renders a submit button (in the sidebar)
  • Set a width on all submit buttons
  • Verify they're rendered with the proper width in both the editor and front end
Screenshot 2025-01-17 at 4 03 09 PM Screenshot 2025-01-17 at 4 03 13 PM Screenshot 2025-01-17 at 4 03 17 PM Screenshot 2025-01-17 at 4 03 20 PM Screenshot 2025-01-17 at 4 03 22 PM Screenshot 2025-01-17 at 4 03 27 PM
  • Play with the buttons' alignment
  • Verify the alignment is correct in both the editor and front end, except for the Premium Content and Recurring blocks (see caveat above). With the Form block, the button should be aligned in the front end but not the editor (see caveat above).
  • Verify this works with a simple site too

Copy link
Contributor

github-actions bot commented Jan 16, 2025

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 WordPress.com Simple 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, and enable the fix/jetpack-button-styling branch.

  • To test on Simple, run the following command on your sandbox:

    bin/jetpack-downloader test jetpack fix/jetpack-button-styling
    

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 github-actions bot added [Block] Button [Block] Contact Form Form block (also see Contact Form label) [Feature] Contact Form [Package] Forms [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ [Status] In Progress labels Jan 16, 2025
Copy link
Contributor

github-actions bot commented Jan 16, 2025

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 Team 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:

  1. Ensure all required checks appearing at the bottom of this PR are passing.
  2. Choose a review path based on your changes:
    • A. Team Review: add the "[Status] Needs Team Review" label
      • For most changes, including minor cross-team impacts.
      • Example: Updating a team-specific component or a small change to a shared library.
    • B. Crew Review: add the "[Status] Needs Review" label
      • For significant changes to core functionality.
      • Example: Major updates to a shared library or complex features.
    • C. Both: Start with Team, then request Crew
      • For complex changes or when you need extra confidence.
      • Example: Refactor affecting multiple systems.
  3. Get at least one approval before merging.

Still unsure? Reach out in #jetpack-developers for guidance!


Jetpack plugin:

The Jetpack plugin has different release cadences depending on the platform:

  • WordPress.com Simple releases happen semi-continuously (PCYsg-Jjm-p2).
  • WoA releases happen weekly.
  • Releases to self-hosted sites happen monthly. The next release is scheduled for February 4, 2025 (scheduled code freeze on February 4, 2025).

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

@github-actions github-actions bot added the [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! label Jan 16, 2025
@@ -90,6 +84,5 @@ export function ButtonEdit( props ) {
}

export default compose(
withColors( { backgroundColor: 'background-color' }, { textColor: 'color' } ),
applyFallbackStyles
Copy link
Contributor Author

Choose a reason for hiding this comment

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

applyFallbackStyles uses the withFallbackStyles high order component from @wordpress/components, which renders the unwanted extra div (at this line). The code suggests we could get rid of that extra div by specifying a node prop, but it's unclear to me what node we could pass at that stage.

In the following discussion, not using applyFallbackStyles was hinted by several persons: p1736773641027939/1736360651.812989-slack-C052XEUUBL4. I do think it's the way to go as well. As far as I can tell, what applyFallbackStyles does is make the default background and text colors available to ButtonEdit. Yet eventually, those two colors are passed to the ContrastChecker component, which doesn't render a warning when the contrast ratio is too low to be deemed accessible.

@@ -52,6 +52,10 @@
flex: 0 0 100%;
margin: 0;

&.wp-block-jetpack-button {
flex-basis: auto;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Overwrite the 100% value. We want to be able to set the width through the button styling options in the editor.

@@ -226,7 +220,7 @@ function get_button_wrapper_styles( $attributes ) {
$has_width = array_key_exists( 'width', $attributes );

if ( $has_width ) {
$styles[] = 'max-width: 100%';
$styles[] = sprintf( 'width: %s;', $attributes['width'] );
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Apply the width to the root of the jetpack/button, instead of its __link element that's now 100% wide.


usePassthroughAttributes( { attributes, clientId, setAttributes } );
useWidth( { attributes, disableEffects: isWidthSetOnParentBlock, setAttributes } );
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Calling this hook results in resetting the button width when it is aligned left or right. I don't get the rationale behind that decision. In the current UI, from a user standpoint, this doesn't seem like something we'd want.

@@ -39,6 +34,7 @@ export function ButtonEdit( props ) {

const blockProps = useBlockProps( {
className: clsx( 'wp-block-button', className ),
style: { width },
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Set the width to the root instead of the __link element.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Button [Block] Calendly [Block] Contact Form Form block (also see Contact Form label) [Block] Eventbrite [Block] Mailchimp [Block] Paid Content aka Premium Content [Block] Payments aka Unified Intro [Feature] Contact Form [Package] Forms [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ [Status] In Progress [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant