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

Block Library: Update Columns block to use Patterns API #18283

Open
wants to merge 5 commits into
base: master
from

Conversation

@gziolo
Copy link
Member

gziolo commented Nov 5, 2019

Description

Part 2 of #16283.

Continuation of #18270. This branch is based on the add/patterns-api master branch.

This PR refactors Columns block to use this new API making the placeholder open for customization.

Selector

This PR also introduces a new selector which lets to find the default pattern:

/**
 * Returns the default block pattern for the given block type.
 * If there is no default pattern set, it returns the first item.
 *
 * @param {Object} state      Data state.
 * @param {string} blockName  Block type name.
 *
 * @return {?WPBlockPattern} The default block pattern.
 */
function __experimentalGetDefaultBlockPattern( state, blockName );

To ensure that icons have a shared size in the layout selection, this PR also adds a new prop to the IconButton component: size to make it possible to enforce one value for icons passed in various formats.

Components

__experimental BlockPatternPicker

I refactored an internal component InnerBlocksTemplatePicker from the InnerBlocks implementation. It's now called BlockPatternPicker and is exposed as an experimental feature __experimental BlockPatternPicker from @wordpress/block-editor package.

In addition, I slightly tweaked wordings in the UI to use the pattern keyword. It's also reflected in the name of props. I updated related docs as well.

Screen Shot 2019-11-06 at 18 47 28

IconButton

I added a new size prop to IconButton component to make it possible to scale all icons to the same size in the block pattern picker.

How has this been tested?

npm run test-unit – new unit tests were added.

Columns block UI

  1. Create a new post or open an existing one.
  2. Add the Column block.
  3. Click one of the layouts and double-check it was properly initialized.
  4. Add another Column block.
  5. Click the skip link and double-check that the default pattern was applied – 2 columns.

Extensibility

To add a new pattern to the Columns block:

wp.blocks.__experimentalRegisterBlockPattern( 'core/columns', { name: 'custom', label: 'Custom', isDefault: true, innerBlocks: [ [ 'core/column' ], [ 'core/column' ], [ 'core/column' ], [ 'core/column' ] ], icon: 'smiley' } ); 

To remove a pattern from the Columns block:

wp.blocks.__experimentalUnregisterBlockPattern( 'core/columns', 'two-columns-equal' ); 

When those two commands get executed, then the custom pattern becomes the default one. In effect, when you add a Column block and click the skip link, then 4 columns will be inserted.

Screen Shot 2019-11-05 at 13 51 54

IconButton

You can test in Storybook the IconButton and its new size prop. To do it run npm run design-system:dev and navigate to the proper page. You can use knobs to play with sizes:

Screen Shot 2019-11-05 at 13 51 36

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
@gziolo gziolo self-assigned this Nov 5, 2019
@gziolo gziolo requested review from mtias, aduth and mcsf Nov 5, 2019
@gziolo gziolo referenced this pull request Nov 5, 2019
3 of 9 tasks complete
@gziolo gziolo force-pushed the update/columns-block-patterns-api branch from 582aaaf to 3d385da Nov 5, 2019
@gziolo gziolo requested a review from chrisvanpatten as a code owner Nov 5, 2019
@gziolo gziolo changed the base branch from add/patterns-api to master Nov 8, 2019
@gziolo gziolo force-pushed the update/columns-block-patterns-api branch from c659e2e to 5744f19 Nov 8, 2019
@@ -15,6 +15,7 @@ export { default as BlockFormatControls } from './block-format-controls';
export { default as BlockIcon } from './block-icon';
export { default as BlockNavigationDropdown } from './block-navigation/dropdown';
export { default as __experimentalBlockNavigationList } from './block-navigation/list';
export { default as __experimentalBlockPatternPicker } from './block-pattern-picker';

This comment has been minimized.

Copy link
@youknowriad

youknowriad Nov 11, 2019

Contributor

I guess we don't need this to be exported and experimental since it's something built-in for block-editor package.

This comment has been minimized.

Copy link
@gziolo

gziolo Nov 12, 2019

Author Member

It depends on how it ends up. In #18343 I use this component from the @wordpress/block-library package. In the long run, I hope we can make it implementation detail of BlockEdit when block patterns are registered, but there might be some challenges in making it work this way like how to detect whether a block is in the placeholder mode. I also need to validate integration with the Table block this week. Let's revisit afterward.

Wrapping up, I 100 % agree with you, but I find it easier to keep it exposed for the time being for my experiments. I can move it to #18343 if that will help to merge this PR sooner.

This comment has been minimized.

Copy link
@youknowriad

youknowriad Nov 12, 2019

Contributor

how to detect whether a block is in the placeholder mode

I agree that this is the challenge, I remember discussing this with @mcsf and @mtias at some poinit (forgot the outcome of that discussion). but I really think we should build this as an implementation detail of BlockEdit (or other block editor components) from the start. Registering patterns for blocks, should be enough to tweak the behavior of the block (like styles, transforms)

This comment has been minimized.

Copy link
@gziolo

gziolo Nov 12, 2019

Author Member

I like your vision, this is how I see it myself as well. I think @mtias might be in opposition or I didn't understand him properly when we discussed on Slack last time :)

__experimentalAllowTemplateOptionSkip: allowTemplateOptionSkip,
__experimentalPatterns: patterns,
__experimentalOnSelectPattern: onSelectPattern,
__experimentalAllowPatternSkip: allowPatternSkip,

This comment has been minimized.

Copy link
@youknowriad

youknowriad Nov 11, 2019

Contributor

Why do we still need this InnerBlocks API. In my thinking, the introduction of the patterns API should remove any need for template/patterns behavior specific to InnerBlocks?

This comment has been minimized.

Copy link
@gziolo

gziolo Nov 12, 2019

Author Member

I can look into it, this would simplify the InnerBlocks documentation and usage. @aduth any thoughts on this? It seems like with the refactor proposed, it wouldn't be a lot of work to use this component standalone.

@gziolo gziolo referenced this pull request Nov 12, 2019
1 of 5 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.