[Button Block]: Add padding block support #31774

stacimc · 2021-05-12T23:38:47Z

Description

Adds padding support to the Button block using the spacing block support.
Issue #31731

Notes:
The block support currently allows arbitrarily large padding values which can cause blocks implementing it to overflow the page. See #31773

How has this been tested?

Manually.

Test Setup
Your theme will need to opt into padding support. You can do this by updating your theme.json to enable padding under the spacing control, in either your default settings or the block context for button:

"settings": {
    "spacing": {
        "customPadding": true,
        "units": [
            "px",
            "em",
            "rem",
            "vh",
            "vw"
        ]
    }
}

Test Instructions

Create a post, add a buttons block with a button.
Select the button block and confirm padding controls in the sidebar work, adjusting units/padding per side.
Add another button and confirm that its padding is default. Update its padding and verify that it does not affect the padding of the first button.
Save the post and confirm the padding is visually correct on the frontend.

Screenshots

Types of changes

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
I've tested my changes with keyboard and screen readers.
My code has proper inline documentation.
I've included developer documentation if appropriate.
I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

ramonjd · 2021-05-17T01:42:48Z

This is testing really well. ✅

Editor	Frontend

I think this PR requires a rebase to fix the failing PHP lint issues.

ramonjd · 2021-05-20T01:16:08Z

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

@@ -225,6 +227,7 @@ function ButtonEdit( props ) {
 							? borderRadius + 'px'
 							: undefined,
 						...colorProps.style,
+						...spacingProps.style,


Should this be ported to edit.native.js as well? Asking because I don't know :) and I can't yet get the native mobile test env running locally.

So I got it running and saw that we do have button props. Color and spacing aren't there. Maybe for a follow up PR?

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

packages/block-editor/src/index.js

ramonjd

This is looking great.

We need to import the new hook getSpacingClassesAndStyles in index.native.js

Since this PR doesn't add the button padding controls to native there's no change. Should we add it in another PR?

stacimc · 2021-05-20T19:53:01Z

This is looking great.

We need to import the new hook getSpacingClassesAndStyles in index.native.js

Since this PR doesn't add the button padding controls to native there's no change. Should we add it in another PR?

Thanks for the thorough testing @ramonjd -- I think the controls should be added to native in this PR as well. I'll get that added!

stacimc · 2021-05-27T21:26:24Z

I took a stab at adding native support for the button padding here: #32218. Until the BoxControl is supported natively, it won't be possible to match the UI exactly without essentially re-implementing that component. There's more information on that PR.

I updated this PR with the native exports so that it does not cause errors in the app, but think it might be better to continue work on the mobile side in a separate PR.

ramonjd

Nice!

I retested this PR on desktop and native mobile.

Desktop button padding support works as described in the test steps:

Buttons controls on native work as expected in the editor and frontend after controls extraction:

I updated this PR with the native exports so that it does not cause errors in the app, but think it might be better to continue work on the mobile side in a separate PR.

Good point. 👍 Will be easier to test in a new PR too.

jasmussen · 2021-06-08T08:11:50Z

Thanks for working on this. Per the improvements that @aaronrobertshaw has been exploring with regards to a specific "Dimensions" panel, can we leverage some of that work for this one? See also #32499 for an example of that panel.

aaronrobertshaw · 2021-06-08T08:28:56Z

can we leverage some of that work for this one?

The changes in #32392 will be picked up by all blocks using spacing support once it lands.

The "Spacing" panel and padding control shown in the gif in the PR description are injected by block supports. #32392 updates the panel injected by the spacing support to use the new progressive disclosure block support panel.

aaronrobertshaw

Nice work @stacimc ✨

This tests well for me.

Individual button padding gets updated appropriately in the block editor and frontend
Setting padding for buttons through the Global Styles works
Theme.json generated styles also get correctly applied to the button both in editor and frontend

It might be worth raising here that there is a discussion around how we can allow blocks to possibly apply styles from block supports to inner elements such as table rows or nav menus.

If a solution to that is found it might allow the skipping of spacing support serialization in this block to be removed. I don't believe it should block this however.

Before merging we'll need to rebase this PR and get some design feedback that we can land this under the provision that the width and spacing panels will ultimately be unified once block support is updated to provide only a dimensions panel.

Being able to control button padding appears pretty important to me and really helps out block pattern designs.

cc/ @jasmussen Any objections to merging this now?

This reverts commit 7235697

ramonjd · 2021-07-28T01:53:00Z

I retested this today and it all looks good in the editor and frontends. Also rechecked on iOS.

I can't see why we couldn't merge this one and wait for the Dimensions UI panel changes to be merged. As I understand it the Dimensions panel only updates the UI and doesn't affect this block's attributes, or requires writing deprecations.

Following @aaronrobertshaw's advice and pinging @geriux just to make sure the changes to the web button has no negative effects on native buttons.

geriux · 2021-07-28T08:13:32Z

Following @aaronrobertshaw's advice and pinging @geriux just to make sure the changes to the web button has no negative effects on native buttons.

Thanks for the ping! I'll check it out 👍

geriux · 2021-07-28T09:34:18Z

I tested this by creating a post on the web editor with a button block and custom padding (following this PR testing instructions) and there are no negative effects while opening/modifying it on the mobile editor 👍

aaronrobertshaw · 2021-07-29T01:54:42Z

I've given this another quick double check as well. LGTM.

youknowriad · 2021-07-30T10:22:29Z

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

@@ -199,6 +200,7 @@ function ButtonEdit( props ) {

 	const borderProps = useBorderProps( attributes );
 	const colorProps = useColorProps( attributes );
+	const spacingProps = useSpacingProps( attributes );


Can you clarify why this is needed here? Ideally padding support is enabled for blocks by just adding a block support flag without touching the block itself at all.

I guess this is because of the extra wrapper of the button block? That wrapper is hurting us a lot, ideally we should find ways to remove it without breaking changes. We tried this in the past but reverted. I wonder if we could make it opt-in somehow.

It seems for now we're just accepting the less than ideal button markup.

cc @mtias

Definitely worth considering and exploring. I'm not too sure of what was the case with the extra wrapper.

mtias · 2021-07-30T10:49:23Z

@stacimc @ramonjd can we try disabling the granular padding for each edge and only keep general padding and/or axial padding and see if there are any concrete requests for the other?

ramonjd · 2021-08-02T06:34:32Z

can we try disabling the granular padding for each edge and only keep general padding and/or axial padding and see if there are any concrete requests for the other?

I've been chatting to @aaronrobertshaw about this, and working out the best way to represent various configurations in the block supports JSON and/or what we need to do to get the existing control hooks to recognize them to display axial padding controls.

I think support for axial spacing can be achieved on top of work he's already done in #30607 and by @andrewserong in #32610.

I'll take a closer look. 👍

I can't find any desperate pleas for allowing padding controls on individual edges so far. Still it sounds reasonable to able to specify between axial/individal and so on.

Merged enums in margin and padding since they can be used together according to WordPress/gutenberg#31774

* Fix margin and padding in block.json Merged enums in margin and padding since they can be used together according to WordPress/gutenberg#31774 * Update block.json

stacimc marked this pull request as ready for review May 12, 2021 23:40

stacimc requested review from ajitbohra, ellatrix, spacedmonkey and TimothyBJacobs as code owners May 12, 2021 23:40

skorasaurus added the [Block] Buttons Affects the Buttons Block label May 13, 2021

stacimc force-pushed the add/padding-support-button-block branch 2 times, most recently from cb4fb84 to e85cfce Compare May 19, 2021 23:30

ramonjd reviewed May 20, 2021

View reviewed changes

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

ramonjd reviewed May 20, 2021

View reviewed changes

packages/block-editor/src/index.js Show resolved Hide resolved

ramonjd requested changes May 20, 2021

View reviewed changes

stacimc changed the title ~~Button Block: Add padding block support~~ [WIP] Button Block: Add padding block support May 20, 2021

stacimc changed the title ~~[WIP] Button Block: Add padding block support~~ Button Block: Add padding block support May 20, 2021

stacimc mentioned this pull request May 25, 2021

[Button Block]: Add native mobile support for padding (WIP) #32218

Closed

7 tasks

stacimc force-pushed the add/padding-support-button-block branch from 62fd58d to 51b7ef2 Compare May 27, 2021 20:48

stacimc changed the title ~~Button Block: Add padding block support~~ [Button Block]: Add padding block support May 27, 2021

ramonjd approved these changes May 28, 2021

View reviewed changes

aaronrobertshaw mentioned this pull request Jul 13, 2021

Buttons Block: Add a way to control button padding #31731

Closed

aaronrobertshaw self-requested a review July 13, 2021 06:37

aaronrobertshaw reviewed Jul 13, 2021

View reviewed changes

stacimc force-pushed the add/padding-support-button-block branch from 35c1561 to 44da455 Compare July 13, 2021 17:52

stacimc added 3 commits July 13, 2021 12:01

Refactor Button to functional component

1136357

Add a utility for getting the spacing inline styles

4d93ddd

Add padding support to the Button block

0fa1556

stacimc added 3 commits July 13, 2021 12:01

Reflect style selection on frontend

0c298fc

Fix error on mobile from missing export

1286e6b

Revert "Refactor Button to functional component"

ef6fb3a

This reverts commit 7235697

stacimc force-pushed the add/padding-support-button-block branch from 44da455 to ef6fb3a Compare July 13, 2021 19:03

aaronrobertshaw merged commit 4c59c1b into WordPress:trunk Jul 29, 2021

github-actions bot added this to the Gutenberg 11.3 milestone Jul 29, 2021

youknowriad reviewed Jul 30, 2021

View reviewed changes

stacimc mentioned this pull request Aug 4, 2021

Button: update spacing support to use axial padding #33859

Merged

7 tasks

ajlende added a commit to ajlende/schemastore that referenced this pull request Oct 25, 2021

Fix margin and padding in block.json

c9914c7

Merged enums in margin and padding since they can be used together according to WordPress/gutenberg#31774

ajlende mentioned this pull request Oct 25, 2021

Fix margin and padding in block.json SchemaStore/schemastore#1903

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Button Block]: Add padding block support #31774

[Button Block]: Add padding block support #31774

stacimc commented May 12, 2021 •

edited

Loading

ramonjd commented May 17, 2021

ramonjd May 20, 2021

ramonjd May 20, 2021

ramonjd left a comment

stacimc commented May 20, 2021

stacimc commented May 27, 2021

ramonjd left a comment

jasmussen commented Jun 8, 2021

aaronrobertshaw commented Jun 8, 2021

aaronrobertshaw left a comment

ramonjd commented Jul 28, 2021 •

edited

Loading

geriux commented Jul 28, 2021

geriux commented Jul 28, 2021

aaronrobertshaw commented Jul 29, 2021

youknowriad Jul 30, 2021

youknowriad Jul 30, 2021

mtias Jul 30, 2021

mtias commented Jul 30, 2021

ramonjd commented Aug 2, 2021 •

edited

Loading

[Button Block]: Add padding block support #31774

[Button Block]: Add padding block support #31774

Conversation

stacimc commented May 12, 2021 • edited Loading

Description

How has this been tested?

Screenshots

Types of changes

Checklist:

ramonjd commented May 17, 2021

ramonjd May 20, 2021

Choose a reason for hiding this comment

ramonjd May 20, 2021

Choose a reason for hiding this comment

ramonjd left a comment

Choose a reason for hiding this comment

stacimc commented May 20, 2021

stacimc commented May 27, 2021

ramonjd left a comment

Choose a reason for hiding this comment

jasmussen commented Jun 8, 2021

aaronrobertshaw commented Jun 8, 2021

aaronrobertshaw left a comment

Choose a reason for hiding this comment

ramonjd commented Jul 28, 2021 • edited Loading

geriux commented Jul 28, 2021

geriux commented Jul 28, 2021

aaronrobertshaw commented Jul 29, 2021

youknowriad Jul 30, 2021

Choose a reason for hiding this comment

youknowriad Jul 30, 2021

Choose a reason for hiding this comment

mtias Jul 30, 2021

Choose a reason for hiding this comment

mtias commented Jul 30, 2021

ramonjd commented Aug 2, 2021 • edited Loading

stacimc commented May 12, 2021 •

edited

Loading

ramonjd commented Jul 28, 2021 •

edited

Loading

ramonjd commented Aug 2, 2021 •

edited

Loading