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.

Sign up for GitHub

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

Jump to bottom

[Button Block]: Add padding block support #31774

Merged
merged 6 commits into from
Jul 29, 2021
+41 −0
Merged

[Button Block]: Add padding block support #31774

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1136357
Refactor Button to functional component
stacimc May 24, 2021
4d93ddd
Add a utility for getting the spacing inline styles
stacimc May 12, 2021
0fa1556
Add padding support to the Button block
stacimc May 12, 2021
0c298fc
Reflect style selection on frontend
stacimc May 12, 2021
1286e6b
Fix error on mobile from missing export
stacimc May 20, 2021
ef6fb3a
Revert "Refactor Button to functional component"
stacimc Jul 13, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/block-editor/src/hooks/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,4 @@ import './layout';
export { useCustomSides } from './spacing';
export { getBorderClassesAndStyles, useBorderProps } from './use-border-props';
export { getColorClassesAndStyles, useColorProps } from './use-color-props';
export { getSpacingClassesAndStyles } from './use-spacing-props';
1 change: 1 addition & 0 deletions packages/block-editor/src/hooks/index.native.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ import './font-size';

export { getBorderClassesAndStyles, useBorderProps } from './use-border-props';
export { getColorClassesAndStyles, useColorProps } from './use-color-props';
export { getSpacingClassesAndStyles } from './use-spacing-props';
28 changes: 28 additions & 0 deletions packages/block-editor/src/hooks/use-spacing-props.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/**
* Internal dependencies
*/
import { getInlineStyles } from './style';

// This utility is intended to assist where the serialization of the spacing
// block support is being skipped for a block but the spacing related CSS
// styles still need to be generated so they can be applied to inner elements.

/**
* Provides the CSS class names and inline styles for a block's spacing support
* attributes.
*
* @param {Object} attributes Block attributes.
*
* @return {Object} Spacing block support derived CSS classes & styles.
*/
export function getSpacingClassesAndStyles( attributes ) {
const { style } = attributes;

// Collect inline styles for spacing.
const spacingStyles = style?.spacing || {};
const styleProp = getInlineStyles( { spacing: spacingStyles } );

return {
style: styleProp,
};
}
1 change: 1 addition & 0 deletions packages/block-editor/src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export {
getColorClassesAndStyles as __experimentalGetColorClassesAndStyles,
useColorProps as __experimentalUseColorProps,
useCustomSides as __experimentalUseCustomSides,
getSpacingClassesAndStyles as __experimentalGetSpacingClassesAndStyles,
stacimc marked this conversation as resolved.
Show resolved Hide resolved
} from './hooks';
export * from './components';
export * from './utils';
Expand Down
4 changes: 4 additions & 0 deletions packages/block-library/src/button/block.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@
"__experimentalFontFamily": true
},
"reusable": false,
"spacing": {
"__experimentalSkipSerialization": true,
"padding": true
},
"__experimentalBorder": {
"radius": true,
"__experimentalSkipSerialization": true
Expand Down
3 changes: 3 additions & 0 deletions packages/block-library/src/button/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
useBlockProps,
__experimentalUseBorderProps as useBorderProps,
__experimentalUseColorProps as useColorProps,
__experimentalGetSpacingClassesAndStyles as useSpacingProps,
__experimentalLinkControl as LinkControl,
} from '@wordpress/block-editor';
import { rawShortcut, displayShortcut } from '@wordpress/keycodes';
Expand Down Expand Up @@ -199,6 +200,7 @@ function ButtonEdit( props ) {

const borderProps = useBorderProps( attributes );
const colorProps = useColorProps( attributes );
const spacingProps = useSpacingProps( attributes );
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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

const ref = useRef();
const blockProps = useBlockProps( { ref } );

Expand Down Expand Up @@ -231,6 +233,7 @@ function ButtonEdit( props ) {
style={ {
...borderProps.style,
...colorProps.style,
...spacingProps.style,
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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?

Screen Shot 2021-05-20 at 1 54 04 pm

} }
onSplit={ ( value ) =>
createBlock( 'core/button', {
Expand Down
3 changes: 3 additions & 0 deletions packages/block-library/src/button/save.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
useBlockProps,
__experimentalGetBorderClassesAndStyles as getBorderClassesAndStyles,
__experimentalGetColorClassesAndStyles as getColorClassesAndStyles,
__experimentalGetSpacingClassesAndStyles as getSpacingClassesAndStyles,
stacimc marked this conversation as resolved.
Show resolved Hide resolved
} from '@wordpress/block-editor';

export default function save( { attributes, className } ) {
Expand All @@ -31,6 +32,7 @@ export default function save( { attributes, className } ) {

const borderProps = getBorderClassesAndStyles( attributes );
const colorProps = getColorClassesAndStyles( attributes );
const spacingProps = getSpacingClassesAndStyles( attributes );
const buttonClasses = classnames(
'wp-block-button__link',
colorProps.className,
Expand All @@ -44,6 +46,7 @@ export default function save( { attributes, className } ) {
const buttonStyle = {
...borderProps.style,
...colorProps.style,
...spacingProps.style,
};

// The use of a `title` attribute here is soft-deprecated, but still applied
Expand Down