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

Blocks: Refactor blocks to use supports align #5099

Closed
wants to merge 7 commits into from
Closed

Blocks: Refactor blocks to use supports align #5099

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b4ebda2
Blocks: Refactor blocks to use supports align
gziolo Feb 16, 2018
baaa079
Block: Code cleanup after review for supports align feature
gziolo Feb 19, 2018
9b1d5ae
Blocks: Rename wideAlign option to alignWide for consistency
gziolo Feb 19, 2018
e19d12b
Blocks: Remove obsolete key props in embed block
gziolo Feb 19, 2018
ddad84d
Blocks: Refactor tests to work with new supports align option
gziolo Feb 20, 2018
3a32e1c
Blocks: More refactoring for blocks to work with supports align
gziolo Feb 22, 2018
f903bb0
Blocks: Revert supports align changes for the latest posts block
gziolo Feb 22, 2018
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
2 changes: 1 addition & 1 deletion blocks/hooks/align.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export function getBlockValidAlignments( blockName ) {
validAlignments.push( 'left', 'center', 'right' );

// ...including wide alignments unless explicitly `false`.
if ( hasBlockSupport( blockName, 'wideAlign', true ) ) {
if ( hasBlockSupport( blockName, 'alignWide', true ) ) {
validAlignments.push( 'wide', 'full' );
}
}
Expand Down
8 changes: 4 additions & 4 deletions blocks/hooks/test/align.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ describe( 'align', () => {
...blockSettings,
supports: {
align: true,
wideAlign: false,
alignWide: false,
},
} );
const validAlignments = getBlockValidAlignments( 'core/foo' );
Expand Down Expand Up @@ -128,7 +128,7 @@ describe( 'align', () => {
...blockSettings,
supports: {
align: true,
wideAlign: false,
alignWide: false,
},
} );

Expand All @@ -154,7 +154,7 @@ describe( 'align', () => {
...blockSettings,
supports: {
align: true,
wideAlign: false,
alignWide: false,
},
} );

Expand Down Expand Up @@ -183,7 +183,7 @@ describe( 'align', () => {
...blockSettings,
supports: {
align: true,
wideAlign: false,
alignWide: false,
},
} );

Expand Down
36 changes: 15 additions & 21 deletions blocks/library/button/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ import './editor.scss';
import './style.scss';
import RichText from '../../rich-text';
import UrlInput from '../../url-input';
import BlockControls from '../../block-controls';
import BlockAlignmentToolbar from '../../block-alignment-toolbar';
import ColorPalette from '../../color-palette';
import ContrastChecker from '../../contrast-checker';
import InspectorControls from '../../inspector-controls';
Expand Down Expand Up @@ -67,18 +65,12 @@ class ButtonBlock extends Component {
text,
url,
title,
align,
color,
textColor,
clear,
} = attributes;

return [
Copy link
Member

Choose a reason for hiding this comment

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

This no longer needs to be returned as an array.

Copy link
Member Author

@gziolo gziolo Feb 22, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

There is an optional form in line 117: https://github.com/WordPress/gutenberg/blob/a3ceee55afbe0200a1567945e14de6f3b3f14f61/blocks/library/button/index.js#L117. It only seems that you can omit array structure here :)

isSelected && (
<BlockControls key="controls">
<BlockAlignmentToolbar value={ align } onChange={ this.updateAlignment } />
</BlockControls>
),
<span key="button" className={ className } title={ title } ref={ this.bindRef }>
Copy link
Member

Choose a reason for hiding this comment

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

And this will no longer need key when not returned as part of an array.

Copy link
Member Author

Choose a reason for hiding this comment

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

See my previous comment.

<RichText
tagName="span"
Expand Down Expand Up @@ -157,10 +149,6 @@ const blockAttributes = {
source: 'children',
selector: 'a',
},
align: {
type: 'string',
default: 'none',
},
color: {
type: 'string',
},
Expand All @@ -182,13 +170,13 @@ export const settings = {

attributes: blockAttributes,

getEditWrapperProps( attributes ) {
const { align, clear } = attributes;
const props = { 'data-resized': true };
supports: {
align: true,
alignWide: false,
},

if ( 'left' === align || 'right' === align || 'center' === align ) {
props[ 'data-align' ] = align;
}
getEditWrapperProps( { clear } ) {
const props = { 'data-resized': true };

if ( clear ) {
props[ 'data-clear' ] = 'true';
Expand All @@ -200,7 +188,7 @@ export const settings = {
edit: ButtonBlock,

save( { attributes } ) {
const { url, text, title, align, color, textColor } = attributes;
const { url, text, title, color, textColor } = attributes;

const buttonStyle = {
backgroundColor: color,
Expand All @@ -210,7 +198,7 @@ export const settings = {
const linkClass = 'wp-block-button__link';

return (
<div className={ `align${ align }` }>
Copy link
Member

Choose a reason for hiding this comment

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

This is the only unfortunate thing about these "magic" supports. It becomes very non-obvious why we need the div wrapper at all.

Copy link
Member Author

@gziolo gziolo Feb 22, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, this might have up to 3 class names inserted in here :)

<div>
<a className={ linkClass } href={ url } title={ title } style={ buttonStyle }>
{ text }
</a>
Expand All @@ -219,7 +207,13 @@ export const settings = {
},

deprecated: [ {
attributes: blockAttributes,
attributes: {
...blockAttributes,
align: {
Copy link
Member

Choose a reason for hiding this comment

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

Since a deprecated entry supports supports, could we just include align there?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think it will work. The deprecated entry doesn't copy attributes, supports and save: ...omit( blockType, [ 'attributes', 'save', 'supports' ] ). Hooks we have with the current implementation don't modify the deprecated block types. @youknowriad can you confirm if the current implementation is valid?

type: 'string',
default: 'none',
},
},

save( { attributes } ) {
const { url, text, title, align, color, textColor } = attributes;
Expand Down
19 changes: 3 additions & 16 deletions blocks/library/categories/block.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ import { times, unescape } from 'lodash';
import './editor.scss';
import './style.scss';
import InspectorControls from '../../inspector-controls';
import BlockControls from '../../block-controls';
import BlockAlignmentToolbar from '../../block-alignment-toolbar';

class CategoriesBlock extends Component {
constructor() {
Expand Down Expand Up @@ -142,11 +140,11 @@ class CategoriesBlock extends Component {
}

render() {
const { attributes, focus, setAttributes } = this.props;
const { align, displayAsDropdown, showHierarchy, showPostCounts } = attributes;
const { attributes, isSelected } = this.props;
const { displayAsDropdown, showHierarchy, showPostCounts } = attributes;
const categories = this.getCategories();

const inspectorControls = focus && (
const inspectorControls = isSelected && (
<InspectorControls key="inspector">
<h3>{ __( 'Categories Settings' ) }</h3>
<ToggleControl
Expand Down Expand Up @@ -182,17 +180,6 @@ class CategoriesBlock extends Component {

return [
inspectorControls,
focus && (
<BlockControls key="controls">
<BlockAlignmentToolbar
value={ align }
onChange={ ( nextAlign ) => {
setAttributes( { align: nextAlign } );
} }
controls={ [ 'left', 'center', 'right', 'full' ] }
/>
</BlockControls>
),
<div key="categories" className={ this.props.className }>
{
displayAsDropdown ?
Expand Down
11 changes: 1 addition & 10 deletions blocks/library/categories/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,22 +34,13 @@ export const settings = {
type: 'boolean',
default: false,
},
align: {
type: 'string',
},
},

supports: {
align: [ 'left', 'center', 'right', 'full' ],
html: false,
},

getEditWrapperProps( attributes ) {
const { align } = attributes;
if ( 'left' === align || 'right' === align || 'full' === align ) {
return { 'data-align': align };
}
},

edit: CategoriesBlock,

save() {
Expand Down
34 changes: 9 additions & 25 deletions blocks/library/columns/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ import { RangeControl } from '@wordpress/components';
*/
import './style.scss';
import InspectorControls from '../../inspector-controls';
import BlockControls from '../../block-controls';
import BlockAlignmentToolbar from '../../block-alignment-toolbar';
import InnerBlocks from '../../inner-blocks';

/**
Expand Down Expand Up @@ -54,34 +52,20 @@ export const settings = {
type: 'number',
default: 2,
},
align: {
type: 'string',
},
},

description: __( 'A multi-column layout of content.' ),

getEditWrapperProps( attributes ) {
const { align } = attributes;

return { 'data-align': align };
supports: {
align: [ 'wide', 'full' ],
},

edit( { attributes, setAttributes, className, focus } ) {
const { align, columns } = attributes;
description: __( 'A multi-column layout of content.' ),

edit( { attributes, setAttributes, className, isSelected } ) {
const { columns } = attributes;
const classes = classnames( className, `has-${ columns }-columns` );

return [
...focus ? [
<BlockControls key="controls">
<BlockAlignmentToolbar
controls={ [ 'wide', 'full' ] }
value={ align }
onChange={ ( nextAlign ) => {
setAttributes( { align: nextAlign } );
} }
/>
</BlockControls>,
isSelected && (
<InspectorControls key="inspector">
<RangeControl
label={ __( 'Columns' ) }
Expand All @@ -94,8 +78,8 @@ export const settings = {
min={ 2 }
max={ 6 }
/>
</InspectorControls>,
] : [],
</InspectorControls>
),
<div className={ classes } key="container">
<InnerBlocks layouts={ getColumnLayouts( columns ) } />
</div>,
Expand Down
28 changes: 6 additions & 22 deletions blocks/library/cover-image/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,8 @@ import AlignmentToolbar from '../../alignment-toolbar';
import MediaUpload from '../../media-upload';
import ImagePlaceholder from '../../image-placeholder';
import BlockControls from '../../block-controls';
import BlockAlignmentToolbar from '../../block-alignment-toolbar';
import InspectorControls from '../../inspector-controls';

const validAlignments = [ 'left', 'center', 'right', 'wide', 'full' ];

export const name = 'core/cover-image';

export const settings = {
Expand All @@ -46,9 +43,6 @@ export const settings = {
url: {
type: 'string',
},
align: {
type: 'string',
},
contentAlign: {
type: 'string',
default: 'center',
Expand All @@ -66,6 +60,10 @@ export const settings = {
},
},

supports: {
align: true,
},

transforms: {
from: [
{
Expand All @@ -87,16 +85,8 @@ export const settings = {
],
},

getEditWrapperProps( attributes ) {
const { align } = attributes;
if ( -1 !== validAlignments.indexOf( align ) ) {
return { 'data-align': align };
}
},

edit( { attributes, setAttributes, isSelected, className } ) {
const { url, title, align, contentAlign, id, hasParallax, dimRatio } = attributes;
const updateAlignment = ( nextAlign ) => setAttributes( { align: nextAlign } );
const { url, title, contentAlign, id, hasParallax, dimRatio } = attributes;
const onSelectImage = ( media ) => setAttributes( { url: media.url, id: media.id } );
const toggleParallax = () => setAttributes( { hasParallax: ! hasParallax } );
const setDimRatio = ( ratio ) => setAttributes( { dimRatio: ratio } );
Expand Down Expand Up @@ -124,11 +114,6 @@ export const settings = {
);
const controls = isSelected && [
<BlockControls key="controls">
<BlockAlignmentToolbar
value={ align }
onChange={ updateAlignment }
/>

{ alignmentToolbar }
<Toolbar>
<MediaUpload
Expand Down Expand Up @@ -211,7 +196,7 @@ export const settings = {
},

save( { attributes, className } ) {
const { url, title, hasParallax, dimRatio, align, contentAlign } = attributes;
const { url, title, hasParallax, dimRatio, contentAlign } = attributes;
const style = url ?
{ backgroundImage: `url(${ url })` } :
undefined;
Expand All @@ -223,7 +208,6 @@ export const settings = {
'has-parallax': hasParallax,
[ `has-${ contentAlign }-content` ]: contentAlign !== 'center',
},
align ? `align${ align }` : null,
);

return (
Expand Down
Loading