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

Blocks: Add initial API for Block Variations (formerly Patterns) #18270

Merged
merged 7 commits into from
Nov 8, 2019
Merged
Show file tree
Hide file tree
Changes from 3 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
76 changes: 59 additions & 17 deletions packages/blocks/src/api/registration.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,21 @@ import { isValidIcon, normalizeIconObject } from './utils';
import { DEPRECATED_ENTRY_KEYS } from './constants';

/**
* Render behavior of a block type icon; one of a Dashicon slug, an element,
* An icon type definition. One of a Dashicon slug, an element,
* or a component.
*
* @typedef {(string|WPElement|WPComponent)} WPBlockTypeIconRender
* @typedef {(string|WPElement|WPComponent)} WPIcon
*
* @see https://developer.wordpress.org/resource/dashicons/
*/

/**
* Render behavior of a block type icon; one of a Dashicon slug, an element,
* or a component.
*
* @typedef {WPIcon} WPBlockTypeIconRender
*/

/**
* An object describing a normalized block type icon.
*
Expand All @@ -57,26 +64,41 @@ import { DEPRECATED_ENTRY_KEYS } from './constants';
* @typedef {(WPBlockTypeIconDescriptor|WPBlockTypeIconRender)} WPBlockTypeIcon
*/

/**
* 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.
*/

/**
* Defined behavior of a block type.
*
* @typedef {Object} WPBlock
*
* @property {string} name Block type's namespaced name.
* @property {string} title Human-readable block type label.
* @property {string} category Block type category classification,
* used in search interfaces to arrange
* block types by category.
* @property {WPBlockTypeIcon} [icon] Block type icon.
* @property {string[]} [keywords] Additional keywords to produce block
* type as result in search interfaces.
* @property {Object} [attributes] Block type attributes.
* @property {WPComponent} [save] Optional component describing
* serialized markup structure of a
* block type.
* @property {WPComponent} edit Component rendering an element to
* manipulate the attributes of a block
* in the context of an editor.
* @property {string} name Block type's namespaced name.
* @property {string} title Human-readable block type label.
* @property {string} category Block type category classification,
* used in search interfaces to arrange
* block types by category.
* @property {WPBlockTypeIcon} [icon] Block type icon.
* @property {string[]} [keywords] Additional keywords to produce block
* type as result in search interfaces.
* @property {Object} [attributes] Block type attributes.
* @property {WPComponent} [save] Optional component describing
* serialized markup structure of a
* block type.
* @property {WPComponent} edit Component rendering an element to
* manipulate the attributes of a block
* in the context of an editor.
* @property {WPBlockPattern[]} [patterns] The list of block patterns.
*/

/**
Expand Down Expand Up @@ -436,3 +458,23 @@ export const registerBlockStyle = ( blockName, styleVariation ) => {
export const unregisterBlockStyle = ( blockName, styleVariationName ) => {
dispatch( 'core/blocks' ).removeBlockStyles( blockName, styleVariationName );
};

/**
* 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.
*/
export const __experimentalRegisterBlockPattern = ( blockName, pattern ) => {
Copy link
Member Author

Choose a reason for hiding this comment

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

@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?

Copy link
Member

Choose a reason for hiding this comment

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

@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

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 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.

dispatch( 'core/blocks' ).addBlockPatterns( 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.
*/
export const __experimentalUnregisterBlockPattern = ( blockName, patternName ) => {
dispatch( 'core/blocks' ).removeBlockPatterns( blockName, patternName );
};
32 changes: 32 additions & 0 deletions packages/blocks/src/store/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,38 @@ export function removeBlockStyles( blockName, styleNames ) {
};
}

/**
* 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.
*/
export function __experimentalAddBlockPatterns( blockName, patterns ) {
return {
type: 'ADD_BLOCK_PATTERNS',
patterns: castArray( patterns ),
blockName,
};
}

/**
* 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.
*/
export function __experimentalRemoveBlockPatterns( blockName, patternNames ) {
return {
type: 'REMOVE_BLOCK_PATTERNS',
patternNames: castArray( patternNames ),
blockName,
};
}

/**
* Returns an action object used to set the default block name.
*
Expand Down
42 changes: 42 additions & 0 deletions packages/blocks/src/store/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,47 @@ export function blockStyles( state = {}, action ) {
return state;
}

/**
* Reducer managing the block patterns.
*
* @param {Object} state Current state.
* @param {Object} action Dispatched action.
*
* @return {Object} Updated state.
*/
export function blockPatterns( state = {}, action ) {
Copy link
Member Author

Choose a reason for hiding this comment

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

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.

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 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.

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

Opened #18398 to keep track of it.

switch ( action.type ) {
case 'ADD_BLOCK_TYPES':
return {
...state,
...mapValues( keyBy( action.blockTypes, 'name' ), ( blockType ) => {
return uniqBy( [
...get( blockType, [ 'patterns' ], [] ),
...get( state, [ blockType.name ], [] ),
], ( style ) => style.name );
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Oooops 😂

Copy link
Member Author

Choose a reason for hiding this comment

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

Refactor planned in #18398 is going to fix it.

} ),
};
case 'ADD_BLOCK_PATTERNS':
return {
...state,
[ action.blockName ]: uniqBy( [
...get( state, [ action.blockName ], [] ),
...( action.patterns ),
], ( pattern ) => pattern.name ),
};
case 'REMOVE_BLOCK_PATTERNS':
return {
...state,
[ action.blockName ]: filter(
get( state, [ action.blockName ], [] ),
( pattern ) => action.patternNames.indexOf( pattern.name ) === -1,
),
};
}

return state;
}

/**
* Higher-order Reducer creating a reducer keeping track of given block name.
*
Expand Down Expand Up @@ -162,6 +203,7 @@ export function categories( state = DEFAULT_CATEGORIES, action ) {
export default combineReducers( {
blockTypes,
blockStyles,
blockPatterns,
defaultBlockName,
freeformFallbackBlockName,
unregisteredFallbackBlockName,
Expand Down
12 changes: 12 additions & 0 deletions packages/blocks/src/store/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,18 @@ export function getBlockStyles( state, name ) {
return state.blockStyles[ name ];
}

/**
* Returns block patterns by block name.
*
* @param {Object} state Data state.
* @param {string} blockName Block type name.
*
* @return {?(WPBlockPattern[])} Block patterns.
gziolo marked this conversation as resolved.
Show resolved Hide resolved
*/
export function __experimentalGetBlockPattens( state, blockName ) {
return state.blockPatterns[ blockName ];
}

/**
* Returns all the available categories.
*
Expand Down
43 changes: 43 additions & 0 deletions packages/blocks/src/store/test/actions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/**
* Internal dependencies
*/
import {
__experimentalAddBlockPatterns,
__experimentalRemoveBlockPatterns,
} from '../actions';

describe( 'actions', () => {
describe( 'addBlockPatterns', () => {
const blockName = 'block/name';
const patternName = 'my-pattern';

it( 'should return the ADD_BLOCK_PATTERNS action', () => {
const pattern = {
name: patternName,
label: 'My Pattern',
attributes: {
example: 'foo',
},
};
const result = __experimentalAddBlockPatterns( blockName, pattern );
expect( result ).toEqual( {
type: 'ADD_BLOCK_PATTERNS',
patterns: [
pattern,
],
blockName,
} );
} );

it( 'should return the REMOVE_BLOCK_PATTERNS action', () => {
const result = __experimentalRemoveBlockPatterns( blockName, patternName );
expect( result ).toEqual( {
type: 'REMOVE_BLOCK_PATTERNS',
patternNames: [
patternName,
],
blockName,
} );
} );
} );
} );
108 changes: 108 additions & 0 deletions packages/blocks/src/store/test/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ import deepFreeze from 'deep-freeze';
* Internal dependencies
*/
import {
__experimentalAddBlockPatterns,
addBlockTypes,
__experimentalRemoveBlockPatterns,
} from '../actions';
import {
blockPatterns,
blockStyles,
blockTypes,
categories,
Expand Down Expand Up @@ -136,6 +142,108 @@ describe( 'blockStyles', () => {
} );
} );

describe( 'blockPatterns', () => {
const blockName = 'block/name';

const blockPatternName = 'pattern-name';
const blockPattern = {
name: blockPatternName,
label: 'My pattern',
};

const secondBlockPatternName = 'second-pattern-name';
const secondBlockPattern = {
name: secondBlockPatternName,
label: 'My Second Pattern',
};

it( 'should return an empty object as default state', () => {
const state = blockPatterns( undefined, {} );

expect( state ).toEqual( {} );
} );

it( 'should add a new block pattern when no pattern register', () => {
const initialState = deepFreeze( {} );

const state = blockPatterns(
initialState,
__experimentalAddBlockPatterns( blockName, blockPattern )
);

expect( state ).toEqual( {
[ blockName ]: [
blockPattern,
],
} );
} );

it( 'should add another pattern when a block pattern already present for the block', () => {
const initialState = deepFreeze( {
[ blockName ]: [
blockPattern,
],
} );

const state = blockPatterns(
initialState,
__experimentalAddBlockPatterns( blockName, secondBlockPattern ),
);

expect( state ).toEqual( {
[ blockName ]: [
blockPattern,
secondBlockPattern,
],
} );
} );

it( 'should prepend block patterns added when adding a block', () => {
const initialState = deepFreeze( {
[ blockName ]: [
secondBlockPattern,
],
} );

const state = blockPatterns(
initialState,
addBlockTypes( {
name: blockName,
patterns: [
blockPattern,
],
} )
);

expect( state ).toEqual( {
[ blockName ]: [
blockPattern,
secondBlockPattern,
],
} );
} );

it( 'should remove a block pattern', () => {
const initialState = deepFreeze( {
[ blockName ]: [
blockPattern,
secondBlockPattern,
],
} );

const state = blockPatterns(
initialState,
__experimentalRemoveBlockPatterns( blockName, blockPatternName )
);

expect( state ).toEqual( {
[ blockName ]: [
secondBlockPattern,
],
} );
} );
} );

describe( 'defaultBlockName', () => {
it( 'should return null as default state', () => {
expect( defaultBlockName( undefined, {} ) ).toBeNull();
Expand Down
Loading