Blocks: Add initial API for Block Variations (formerly Patterns) by gziolo · Pull Request #18270 · WordPress/gutenberg

gziolo · 2019-11-04T14:07:14Z

Description

Part 1 of #16283.

This is my proposal for the initial API:

/**
 * An object describing a pattern defined for the block type.
 *
 * @typedef {Object} WPBlockPattern
 *
 * @property {string}  name          The unique and machine-readable name.
 * @property {string}  label         A human-readable label.
 * @property {WPIcon}  [icon]        An icon helping to visualize the pattern.
 * @property {boolean} [isDefault]   Indicates whether the current pattern is the default one.
 *                                   Defaults to `false`.
 * @property {Object}  [attributes]  Values which override block attributes.
 * @property {Array[]} [innerBlocks] Initial configuration of nested blocks.
 */

innerBlocks needs more thoughts. I would prefer to defer this decision until the very first block is converted to use this new API. See more detailed explanation in #18270 (comment).

New APIs proposed in `@wordpress/blocks`

`wp.data.*`

/**
 * Registers a new block pattern for the given block.
 *
 * @param {string}         blockName Name of the block (example: “core/columns”).
 * @param {WPBlockPattern} pattern   Object describing a block pattern.
 */
function __experimentalRegisterBlockPattern( blockName, pattern );

/**
 * Unregisters a block pattern defined for the given block.
 *
 * @param {string} blockName   Name of the block (example: “core/columns”).
 * @param {string} patternName Name of the pattern defined for the block.
 */
function __experimentalUnregisterBlockPattern( blockName, patternName );

Actions

/**
 * Returns an action object used in signalling that new block patterns have been added.
 *
 * @param {string}                          blockName Block name.
 * @param {WPBlockPattern|WPBlockPattern[]} patterns  Block patterns.
 *
 * @return {Object} Action object.
 */
function __experimentalAddBlockPatterns( blockName, patterns );

/**
 * Returns an action object used in signalling that block patterns have been removed.
 *
 * @param {string}          blockName    Block name.
 * @param {string|string[]} patternNames Block pattern names.
 *
 * @return {Object} Action object.
 */
function __experimentalRemoveBlockPatterns( blockName, patternNames )

Selectors

/**
 * Returns block patterns by block name.
 *
 * @param {Object} state      Data state.
 * @param {string} blockName  Block type name.
 *
 * @return {?(WPBlockPattern[])} Block patterns.
 */
function __experimentalGetBlockPatterns( state, blockName );

How has this been tested?

npm run test

All added unit tests should pass.

gziolo · 2019-11-04T14:09:01Z

+ *
+ * @return {Object} Updated state.
+ */
+export function blockPatterns( state = {}, action ) {


This is nearly identical to blockStyles. In theory, we could extract duplicated code and abstract it away. However, I'm not convinced that this will make it easier to read in the long run. If you think this is a better way to go, I'm happy to refactor.

This is nearly identical to blockStyles. In theory, we could extract duplicated code and abstract it away. However, I'm not convinced that this will make it easier to read in the long run. If you think this is a better way to go, I'm happy to refactor.

Since we're inheriting the logic, I'm not overly concerned with addressing it here, but a point for both this and the blockStyles upon which it's based:

Do we really need to track a value for each registered block type? Can't this be something where we just manage the normalization to an empty array from the selector? Seems like it may make the implementation a little simpler, and avoid the (albeit minimal) memory usage involved with tracking a bunch of key/values for blocks without patterns.

That's a good point, indeed. I guess this would force us to use memoization for the selector, but it seems like a good trade-off given that the implementation would get much simpler in the reducer. I'll open a follow-up shortly after I merge this one which is going to explore this idea for both reducers.

Opened #18398 to keep track of it.

gziolo · 2019-11-05T08:47:49Z

+ * @param {string}         blockName Name of the block (example: “core/columns”).
+ * @param {WPBlockPattern} pattern   Object describing a block pattern.
+ */
+export const __experimentalRegisterBlockPattern = ( blockName, pattern ) => {


@youknowriad - do we even need those APIs in the first place? I followed registerBlockStyle and unregisterBlockStyle but we could also use data API directly. What do you think?

@youknowriad - do we even need those APIs in the first place? I followed registerBlockStyle and unregisterBlockStyle but we could also use data API directly. What do you think?

In my opinion, I think we should have them:

It's slightly more convenient / obvious to use, than via the equivalent data actions

Consistency for consistency's sake

I used this wp.blocks.* API in the examples presenting how you can tweak the Columns block with custom patterns so maybe it's indeed more convenient. The benefit is also that you don't have to worry whether the core/blocks store is initialized as it's embedded in the module. Wheres using the data module directly is prone to error in that regard.

aduth · 2019-11-06T22:23:34Z

innerBlocks needs more thoughts. I would prefer to defer this decision until the very first block is converted to use this new API.

Based on #18283, is there any concern with this just being the block templates array syntax? I'm not really sure what's hinted at here as requiring further consideration.

gziolo · 2019-11-07T10:17:39Z

innerBlocks needs more thoughts. I would prefer to defer this decision until the very first block is converted to use this new API.

Based on #18283, is there any concern with this just being the block templates array syntax? I'm not really sure what's hinted at here as requiring further consideration.

I don't have any concerns about this part. It's super solid, well-documented and extensively tested on production. I have been thinking about the name which is less of an issue but I noticed that we use template and innerBlocks to describe the same concepts. I'd love we use only one of them moving forward in all places. It probably would be a good idea to perform an audit and introduce an alias for backward compatibility, in effect start using only one of those names. The other thing I was contemplating is related config options you usually see with templates like the list of allowed blocks or the type of locking. I don't know if we should plan ahead to include them in the proposal as well.

aduth · 2019-11-07T22:06:25Z

I don't have any concerns about this part. It's super solid, well-documented and extensively tested on production. I have been thinking about the name which is less of an issue but I noticed that we use template and innerBlocks to describe the same concepts. I'd love we use only one of them moving forward in all places. It probably would be a good idea to perform an audit and introduce an alias for backward compatibility, in effect start using only one of those names.

It's a good point. I'm not sure what the best option is here, since innerBlocks doesn't make much sense in the context of the post type template, and conversely template is also not very obvious in describing the (for lack of a better term) inner blocks of a block pattern.

Another option, if we standardize on "template" as the name for this format, is to include a prefix describing it for a pattern as applying to its inner blocks, i.e. innerBlocksTemplate.

aduth

Looks pretty flawless to me! A few minor points, but nothing blocking.

aduth · 2019-11-07T22:08:01Z

+ * @param {string}         blockName Name of the block (example: “core/columns”).
+ * @param {WPBlockPattern} pattern   Object describing a block pattern.
+ */
+export const __experimentalRegisterBlockPattern = ( blockName, pattern ) => {


@youknowriad - do we even need those APIs in the first place? I followed registerBlockStyle and unregisterBlockStyle but we could also use data API directly. What do you think?

In my opinion, I think we should have them:

It's slightly more convenient / obvious to use, than via the equivalent data actions

Consistency for consistency's sake

aduth · 2019-11-07T22:11:41Z

+ *
+ * @return {Object} Updated state.
+ */
+export function blockPatterns( state = {}, action ) {


This is nearly identical to blockStyles. In theory, we could extract duplicated code and abstract it away. However, I'm not convinced that this will make it easier to read in the long run. If you think this is a better way to go, I'm happy to refactor.

Since we're inheriting the logic, I'm not overly concerned with addressing it here, but a point for both this and the blockStyles upon which it's based:

Do we really need to track a value for each registered block type? Can't this be something where we just manage the normalization to an empty array from the selector? Seems like it may make the implementation a little simpler, and avoid the (albeit minimal) memory usage involved with tracking a bunch of key/values for blocks without patterns.

Co-Authored-By: Andrew Duthie <andrew@andrewduthie.com>

gziolo · 2019-11-08T06:49:25Z

Another option, if we standardize on "template" as the name for this format, is to include a prefix describing it for a pattern as applying to its inner blocks, i.e. innerBlocksTemplate.

Yes, this one is longer but it probably fits better. @youknowriad what do you think? I know that you used innerBlocks in the context of example setting for blocks which I followed here:

gutenberg/packages/block-library/src/columns/index.js

Lines 27 to 31 in e1fb88e

    
           example: { 
        
           	innerBlocks: [ 
        
           		{ 
        
           			name: 'core/column', 
        
           			innerBlocks: [

mcsf

Nice work.

mcsf · 2019-11-08T14:12:44Z

+					return uniqBy( [
+						...get( blockType, [ 'patterns' ], [] ),
+						...get( state, [ blockType.name ], [] ),
+					], ( style ) => style.name );


I think this should read ( pattern ) => pattern.name :)

Oooops 😂

Refactor planned in #18398 is going to fix it.

youknowriad · 2019-11-11T11:02:36Z

nice work here 👍

gziolo self-assigned this Nov 4, 2019

gziolo commented Nov 4, 2019

View reviewed changes

Comment thread packages/blocks/src/api/registration.js Outdated

gziolo commented Nov 4, 2019

View reviewed changes

Comment thread packages/blocks/src/store/test/selectors.js Outdated

gziolo commented Nov 4, 2019

View reviewed changes

Comment thread packages/blocks/src/api/registration.js Outdated

gziolo requested a review from youknowriad November 4, 2019 14:13

gziolo mentioned this pull request Nov 4, 2019

Variations (formerly patterns) API for blocks #16283

Closed

16 tasks

gziolo requested review from mcsf and mtias November 4, 2019 14:20

gziolo added 3 commits November 5, 2019 09:45

Blocks: Add initial API for Block Patterns

73ff4e5

Update packages/blocks/src/api/registration.js

1f1e2c7

Convert all public APIs to experimental

2cc172b

gziolo force-pushed the add/patterns-api branch from 66f451f to 2cc172b Compare November 5, 2019 08:45

Expose block patterns registration APIs

5593de0

gziolo commented Nov 5, 2019

View reviewed changes

gziolo marked this pull request as ready for review November 5, 2019 08:48

gziolo requested review from ajitbohra, ellatrix, nerrad and ntwb as code owners November 5, 2019 08:48

Fix typo in the name of selector

f6feeef

gziolo mentioned this pull request Nov 5, 2019

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

Merged

5 tasks

Blocks: Update Patterns API integration with the store

61701db

aduth approved these changes Nov 7, 2019

View reviewed changes

Update packages/blocks/src/store/selectors.js

0f296bc

Co-Authored-By: Andrew Duthie <andrew@andrewduthie.com>

gziolo merged commit a71241c into master Nov 8, 2019

gziolo deleted the add/patterns-api branch November 8, 2019 12:27

gziolo mentioned this pull request Nov 8, 2019

Blocks: Use selectors to fetch details about for patterns and styles … #18398

Closed

6 tasks

mcsf reviewed Nov 8, 2019

View reviewed changes

youknowriad added this to the Gutenberg 6.9 milestone Nov 11, 2019

gziolo mentioned this pull request Nov 27, 2019

Blocks: Fix typo in the blockPatterns reducer #18781

Merged

gziolo changed the title ~~Blocks: Add initial API for Block Patterns~~ Blocks: Add initial API for Block Variations (renamed from Patterns) Sep 9, 2021

gziolo changed the title ~~Blocks: Add initial API for Block Variations (renamed from Patterns)~~ Blocks: Add initial API for Block Variations (formerly Patterns) Sep 9, 2021

naiba4 mentioned this pull request Apr 6, 2024

[Snyk] Security upgrade eslint from 8.57.0 to 9.0.0 naiba4/gutenberg#52

Open

MaxMood96 mentioned this pull request Apr 6, 2024

[Snyk] Security upgrade eslint from 8.57.0 to 9.0.0 MaxMood96/gutenberg#151

Open

jay-Anonymous mentioned this pull request Apr 6, 2024

[Snyk] Security upgrade eslint from 8.57.0 to 9.0.0 jay-Anonymous/gutenberg#8

Open

monzerfoda mentioned this pull request Apr 6, 2024

[Snyk] Security upgrade eslint from 4.19.1 to 9.0.0 monzerfoda/gutenberg#23

Open

soloinovator mentioned this pull request Apr 6, 2024

[Snyk] Security upgrade eslint from 8.57.0 to 9.0.0 soloinovator/gutenberg#137

Open

yassine4999 mentioned this pull request Apr 6, 2024

[Snyk] Security upgrade eslint from 8.57.0 to 9.0.0 yassine4999/gutenberg#41

Open

petrescugit mentioned this pull request Apr 6, 2024

[Snyk] Security upgrade eslint from 8.57.0 to 9.0.0 petrescugit/gutenberg#19

Open

yaseenaljamal mentioned this pull request Apr 6, 2024

[Snyk] Security upgrade eslint from 8.57.0 to 9.0.0 yaseenaljamal/gutenberg#20

Open

makbarGroup mentioned this pull request Apr 6, 2024

[Snyk] Security upgrade eslint from 4.19.1 to 9.0.0 makbarGroup/gutenberg#59

Open

This was referenced Apr 6, 2024

[Snyk] Security upgrade eslint from 7.32.0 to 9.0.0 Verdinjoshua26/gutenberg#114

Open

[Snyk] Fix for 2 vulnerabilities Verdinjoshua26/gutenberg#115

Open

Uh oh!

Conversation

gziolo commented Nov 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

New APIs proposed in @wordpress/blocks

wp.data.*

Actions

Selectors

How has this been tested?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aduth commented Nov 6, 2019

Uh oh!

gziolo commented Nov 7, 2019

Uh oh!

aduth commented Nov 7, 2019

Uh oh!

aduth left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gziolo commented Nov 8, 2019

Uh oh!

mcsf left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

youknowriad commented Nov 11, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

gziolo commented Nov 4, 2019 •

edited

Loading

New APIs proposed in `@wordpress/blocks`

`wp.data.*`