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

Add Columns template options, support InnerBlock templateOptions #16129

Merged
merged 17 commits into from
Jun 24, 2019
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
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
63 changes: 63 additions & 0 deletions packages/block-editor/src/components/inner-blocks/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,69 @@ const TEMPLATE = [ [ 'core/columns', {}, [

The previous example creates an InnerBlocks area containing two columns one with an image and the other with a paragraph.

### `__experimentaltemplateOptions`

* **Type:** `Array<Object>`

To present the user with a set of template choices for the inner blocks, you may provide an array of template options.

A template option is an object consisting of the following properties:

- `title` (`string`): A human-readable label which describes the template.
- `icon` (`WPElement|string`): An element or [Dashicon](https://developer.wordpress.org/resource/dashicons/) slug to show as a visual approximation of the template.
- `template` (`Array<Array>`): The template to apply when the option has been selected. See [`template` documentation](#template) for more information.

For the template placeholder selection to be displayed, you must render `InnerBlocks` with a `template` prop value of `null`. You may track this as state of your component, updating its value when receiving the selected template via `onSelectTemplateOption`.
youknowriad marked this conversation as resolved.
Show resolved Hide resolved

```jsx
import { useState } from '@wordpress/element';

const TEMPLATE_OPTIONS = [
{
title: 'Two Columns',
icon: <svg />,
template: [
[ 'core/column', { width: 50 } ],
[ 'core/column', { width: 50 } ],
],
},
{
title: 'Three Columns',
icon: <svg />,
template: [
[ 'core/column', { width: 33.33 } ],
[ 'core/column', { width: 33.33 } ],
[ 'core/column', { width: 33.33 } ],
],
},
];

function edit() {
const [ template, setTemplate ] = useState( null );

return (
<InnerBlocks
template={ template }
templateOptions={ TEMPLATE_OPTIONS }
onSelectTemplateOption={ setTemplate }
/>
);
}
```

### `__experimentalOnSelectTemplateOption`

* **Type:** `Function`

Callback function invoked when the user makes a template selection, used in combination with the `templateOptions` props. The selected template is passed as the first and only argument. The value of the template may be `undefined` if the `allowTemplateOptionSkip` prop is passed to `InnerBlocks` and the user opts to skip template selection.

### `__experimentalAllowTemplateOptionSkip`

* **Type:** `Boolean`
* **Default:** `false`

Whether to include a button in the template selection placeholder to allow the user to skip selection, used in combination with the `templateOptions` prop. When clicked, the `onSelectTemplateOption` callback will be passed an `undefined` value as the template.

### `templateInsertUpdatesSelection`
* **Type:** `Boolean`
* **Default:** `true`
Expand Down
24 changes: 19 additions & 5 deletions packages/block-editor/src/components/inner-blocks/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import DefaultBlockAppender from './default-block-appender';
*/
import BlockList from '../block-list';
import { withBlockEditContext } from '../block-edit/context';
import TemplatePicker from './template-picker';

class InnerBlocks extends Component {
constructor() {
Expand All @@ -48,6 +49,7 @@ class InnerBlocks extends Component {
if ( innerBlocks.length === 0 || this.getTemplateLock() === 'all' ) {
this.synchronizeBlocksWithTemplate();
}

if ( this.state.templateInProcess ) {
this.setState( {
templateInProcess: false,
Expand Down Expand Up @@ -107,20 +109,32 @@ class InnerBlocks extends Component {
clientId,
hasOverlay,
renderAppender,
template,
__experimentalTemplateOptions: templateOptions,
__experimentalOnSelectTemplateOption: onSelectTemplateOption,
__experimentalAllowTemplateOptionSkip: allowTemplateOptionSkip,
} = this.props;
const { templateInProcess } = this.state;

const isPlaceholder = template === null && !! templateOptions;

const classes = classnames( 'editor-inner-blocks block-editor-inner-blocks', {
'has-overlay': hasOverlay,
'has-overlay': hasOverlay && ! isPlaceholder,
} );

return (
<div className={ classes }>
{ ! templateInProcess && (
<BlockList
rootClientId={ clientId }
renderAppender={ renderAppender }
/>
isPlaceholder ?
<TemplatePicker
options={ templateOptions }
onSelect={ onSelectTemplateOption }
allowSkip={ allowTemplateOptionSkip }
/> :
<BlockList
rootClientId={ clientId }
renderAppender={ renderAppender }
/>
) }
</div>
);
Expand Down
66 changes: 66 additions & 0 deletions packages/block-editor/src/components/inner-blocks/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,69 @@
right: 0;
left: 0;
}

.block-editor-inner-blocks__template-picker {
.components-placeholder__instructions {
// Defer to vertical margins applied by template picker options.
margin-bottom: 0;
}

.components-placeholder__fieldset {
// Options will render horizontally, but the immediate children of the
// fieldset are the options and the skip button, oriented vertically.
flex-direction: column;
}

&.has-many-options .components-placeholder__fieldset {
// Allow options to occupy a greater amount of the available space if
// many options exist.
max-width: 90%;
}
}

.block-editor-inner-blocks__template-picker-options.block-editor-inner-blocks__template-picker-options {
display: flex;
flex-direction: row;
flex-wrap: nowrap;
width: 100%;
margin: $grid-size-large 0;
list-style: none;

> li {
flex-basis: 100%;
flex-shrink: 1;
margin: 0 $grid-size;
max-width: 100px;
}
}

.block-editor-inner-blocks__template-picker-option {
width: 100%;

&.components-icon-button {
// Override default styles inherited from <IconButton /> to center
// icon horizontally.
justify-content: center;
}

&.components-button {
// Override default styles inherited from <Button /> to allow button
// to grow vertically.
height: auto;
padding: 0;
}

&::before {
// Use `padding-bottom` trick to style element as perfect square.
content: "";
padding-bottom: 100%;
}

&:first-child {
margin-left: 0;
}

&:last-child {
margin-right: 0;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/**
Copy link
Member

Choose a reason for hiding this comment

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

Is this something we expect to only be used for inner blocks? Is there a use case for the selection screen to be used for pre-configured block attributes?

Copy link
Member Author

Choose a reason for hiding this comment

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

Is this something we expect to only be used for inner blocks? Is there a use case for the selection screen to be used for pre-configured block attributes?

That's a good point. It's unclear to me how this might look from a design perspective, since the current concepts seem to align quite closely with how blocks are arranged (technically aligning to the template option), though I don't see any reason why it couldn't be that the options configure a block's own attributes. It speaks a bit to the original questions about whether this be proposed an enhancement to the Placeholder component rather than to the InnerBlocks component, which could be more easily adapted. I still think we could extract something common regardless.

At least as experimental props, we could still opt toward the Placeholder improvements if own-block attributes option selection pans out to be a good idea to pursue.

* External dependencies
*/
import classnames from 'classnames';

/**
* WordPress dependencies
*/
import { __ } from '@wordpress/i18n';
import { Button, IconButton, Placeholder } from '@wordpress/components';

function InnerBlocksTemplatePicker( {
options,
onSelect,
allowSkip,
} ) {
const classes = classnames( 'block-editor-inner-blocks__template-picker', {
'has-many-options': options.length > 4,
} );

const instructions = allowSkip ?
__( 'Select a layout to start with, or make one yourself.' ) :
__( 'Select a layout to start with.' );

return (
<Placeholder
icon="layout"
label={ __( 'Choose Layout' ) }
instructions={ instructions }
className={ classes }
>
{
/*
* Disable reason: The `list` ARIA role is redundant but
Copy link
Member Author

@aduth aduth Jun 24, 2019

Choose a reason for hiding this comment

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

Not super relevant for this pull request, but is the approach we've taken that we need role="list" for every ul ? Especially considering it as a "bug" of a particular browser + screen reader combination, I wonder if this is something we could isolate to a Babel transform, to remove consideration from the individual developer. That, or an inverse ESLint rule to enforce it be applied.

Currently, it's far too easy to overlook (evidenced by the fact I'd omitted it in my original proposal).

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I know we did this for the Block Hierarchy as well. yeah, it does seem like we should have a rule for it.

* Safari+VoiceOver won't announce the list otherwise.
*/
/* eslint-disable jsx-a11y/no-redundant-roles */
}
<ul className="block-editor-inner-blocks__template-picker-options" role="list">
{ options.map( ( templateOption, index ) => (
<li key={ index }>
<IconButton
isLarge
icon={ templateOption.icon }
onClick={ () => onSelect( templateOption.template ) }
className="block-editor-inner-blocks__template-picker-option"
label={ templateOption.title }
/>
</li>
) ) }
</ul>
{ /* eslint-enable jsx-a11y/no-redundant-roles */ }
{ allowSkip && (
<div className="block-editor-inner-blocks__template-picker-skip">
<Button
isLink
onClick={ () => onSelect( undefined ) }
>
{ __( 'Skip' ) }
</Button>
</div>
) }
</Placeholder>
);
}

export default InnerBlocksTemplatePicker;
4 changes: 0 additions & 4 deletions packages/block-library/src/columns/block.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,6 @@
"name": "core/columns",
"category": "layout",
"attributes": {
"columns": {
"type": "number",
"default": 2
},
"verticalAlignment": {
"type": "string"
}
Expand Down
34 changes: 34 additions & 0 deletions packages/block-library/src/columns/deprecated.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
/**
* External dependencies
*/
import classnames from 'classnames';

/**
* WordPress dependencies
*/
Expand Down Expand Up @@ -96,4 +101,33 @@ export default [
);
},
},
{
attributes: {
columns: {
type: 'number',
default: 2,
},
},
migrate( attributes, innerBlocks ) {
attributes = {
...attributes,
columns: innerBlocks.length,
Copy link
Member Author

Choose a reason for hiding this comment

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

If we've dropped the columns attribute, this should no longer apply?

};

return [ attributes, innerBlocks ];
},
save( { attributes } ) {
const { verticalAlignment, columns } = attributes;

const wrapperClasses = classnames( `has-${ columns }-columns`, {
[ `are-vertically-aligned-${ verticalAlignment }` ]: verticalAlignment,
} );

return (
<div className={ wrapperClasses }>
<InnerBlocks.Content />
</div>
);
},
},
];
Loading