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 API: Allow "array of attributes to be compared" for isActive property in block variations #30913

Merged
merged 19 commits into from
Apr 22, 2021
Merged
Show file tree
Hide file tree
Changes from 14 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
2 changes: 1 addition & 1 deletion docs/reference-guides/block-api/block-variations.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ An object describing a variation defined for the block type can contain the foll
- `block` - Used by blocks to filter specific block variations. Mostly used in Placeholder patterns like `Columns` block.
- `transform` - Block Variation will be shown in the component for Block Variations transformations.
- `keywords` (optional, type `string[]`) - An array of terms (which can be translated) that help users discover the variation while searching.
- `isActive` (optional, type `Function`) - A function that accepts a block's attributes and the variation's attributes and determines if a variation is active. This function doesn't try to find a match dynamically based on all block's attributes, as in many cases some attributes are irrelevant. An example would be for `embed` block where we only care about `providerNameSlug` attribute's value.
- `isActive` (optional, type `Function|string[]`) - A function that accepts a block's attributes and the variation's attributes and determines if a variation is active. This function doesn't try to find a match dynamically based on all block's attributes, as in many cases some attributes are irrelevant. An example would be for `embed` block where we only care about `providerNameSlug` attribute's value. We can also use a `string[]` to tell which attributes should be compared as a shorthand. Each attributes will be matched and the variation will be active if all of them are matching.
ntsekouras marked this conversation as resolved.
Show resolved Hide resolved

The main difference between style variations and block variations is that a style variation just applies a `css class` to the block, so it can be styled in an alternative way. If we want to apply initial attributes or inner blocks, we fall in block variation territory.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,21 +43,19 @@ export default function useBlockDisplayInformation( clientId ) {
const { getBlockName, getBlockAttributes } = select(
blockEditorStore
);
const { getBlockType, getBlockVariations } = select( blocksStore );
const { getBlockType, getActiveBlockVariation } = select(
blocksStore
);
const blockName = getBlockName( clientId );
const blockType = getBlockType( blockName );
if ( ! blockType ) return null;
const variations = getBlockVariations( blockName );
const blockTypeInfo = {
title: blockType.title,
icon: blockType.icon,
description: blockType.description,
};
if ( ! variations?.length ) return blockTypeInfo;
const attributes = getBlockAttributes( clientId );
const match = variations.find( ( variation ) =>
variation.isActive?.( attributes, variation.attributes )
);
const match = getActiveBlockVariation( blockName, attributes );
if ( ! match ) return blockTypeInfo;
return {
title: match.title || blockType.title,
Expand Down
10 changes: 5 additions & 5 deletions packages/block-editor/src/store/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ import {
/**
* WordPress dependencies
*/
import { combineReducers } from '@wordpress/data';
import { getBlockVariations } from '@wordpress/blocks';
import { combineReducers, select } from '@wordpress/data';
import { store as blocksStore } from '@wordpress/blocks';
/**
* Internal dependencies
*/
Expand Down Expand Up @@ -1510,9 +1510,9 @@ export function preferences( state = PREFERENCES_DEFAULTS, action ) {
case 'REPLACE_BLOCKS':
return action.blocks.reduce( ( prevState, block ) => {
const { attributes, name: blockName } = block;
const variations = getBlockVariations( blockName );
const match = variations?.find( ( variation ) =>
variation.isActive?.( attributes, variation.attributes )
const match = select( blocksStore ).getActiveBlockVariation(
blockName,
attributes
);
// If a block variation match is found change the name to be the same with the
// one that is used for block variations in the Inserter (`getItemFromVariation`).
Expand Down
42 changes: 42 additions & 0 deletions packages/blocks/src/api/registration.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
pick,
pickBy,
some,
castArray,
} from 'lodash';

/**
Expand Down Expand Up @@ -303,11 +304,50 @@ export function registerBlockType( name, settings ) {
return;
}

validateVariations( settings );

dispatch( blocksStore ).addBlockTypes( settings );

return settings;
}

function validateVariations( settings ) {
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'm not really sure where to put this. 😄 I'm also wondering why don't we dispatch( blocksStore ).addBlockVariation( settings.variations ) instead of handling variations here:

...get( blockType, [ 'variations' ], [] ),

For this reason we need to do the variations validation in two functions.

Copy link
Member

Choose a reason for hiding this comment

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

Can we extract it to another PR? It isn't a blocker really and it requires some more thinking because you can register block variations also through the store directly. I don't remember exactly but most likely they are stored in two places in the store. It's similar to how block styles can be registered, so plugin authors could register styles or variations before the block gets registered so plugins wouldn't have to declare dependencies on each other.

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 makes sense. I didn't know it meant to be that way - registering variation even without an existing block type. This comes with its own challenges 😄

I revert the commit.

Copy link
Member

Choose a reason for hiding this comment

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

I double-checked, it's stored in one reducer but you can add variations with ADD_BLOCK_TYPES or ADD_BLOCK_VARIATIONS 😄

if ( ! Array.isArray( settings.variations ) ) {
return;
}
const attributeKeys = Object.keys( settings.attributes || {} );
settings.variations.forEach( ( variation ) => {
if ( Array.isArray( variation.isActive ) ) {
const undefinedAttributes = variation.isActive.filter(
( attribute ) => ! attributeKeys.includes( attribute )
);
if ( undefinedAttributes.length === 0 ) {
return;
}

console.warn(
`The block "${ settings.name }"'s "${
variation.name
}" variation "isActive" array contains attributes ("${ undefinedAttributes.join(
'", "'
) }") that aren't defined in the block type. They will be ignored.`
);
variation.isActive = variation.isActive.filter( ( attribute ) =>
attributeKeys.includes( attribute )
);
} else if (
! isFunction( variation.isActive ) &&
typeof variation.isActive !== 'undefined' &&
variation.isActive !== null
) {
console.warn(
`The block "${ settings.name }"'s "${ variation.name }" variation "isActive" must be either an array or a function.`
);
delete variation.isActive;
}
} );
}

/**
* Registers a new block collection to group blocks in the same namespace in the inserter.
*
Expand Down Expand Up @@ -577,6 +617,8 @@ export const getBlockVariations = ( blockName, scope ) => {
* @param {WPBlockVariation} variation Object describing a block variation.
*/
export const registerBlockVariation = ( blockName, variation ) => {
const blockType = getBlockType( blockName );
validateVariations( { ...blockType, variations: castArray( variation ) } );
dispatch( blocksStore ).addBlockVariations( blockName, variation );
};

Expand Down
54 changes: 54 additions & 0 deletions packages/blocks/src/api/test/registration.js
Original file line number Diff line number Diff line change
Expand Up @@ -648,6 +648,60 @@ describe( 'blocks', () => {
} );
} );

it( "should warn and remove isActive if a variation's isActive is not an array or function", () => {
const blockType = {
save: noop,
category: 'text',
title: 'block title',
variations: [
{
name: 'variation-1',
isActive: 'ignore this',
},
],
};
registerBlockType( 'core/test-block-with-variations', blockType );
expect(
getBlockType( 'core/test-block-with-variations' ).variations
).toEqual( [
{
name: 'variation-1',
},
] );
expect( console ).toHaveWarnedWith(
'The block "core/test-block-with-variations"\'s "variation-1" variation "isActive" must be either an array or a function.'
);
} );

it( "should warn and remove extra attribute if a variation isActive's array contains attribute that is not defined in the block type", () => {
const blockType = {
save: noop,
category: 'text',
title: 'block title',
attributes: {
link: {},
},
variations: [
{
name: 'variation-1',
isActive: [ 'link', 'text' ],
},
],
};
registerBlockType( 'core/test-block-with-variations', blockType );
expect(
getBlockType( 'core/test-block-with-variations' ).variations
).toEqual( [
{
name: 'variation-1',
isActive: [ 'link' ],
},
] );
expect( console ).toHaveWarnedWith(
'The block "core/test-block-with-variations"\'s "variation-1" variation "isActive" array contains attributes ("text") that aren\'t defined in the block type. They will be ignored.'
);
} );

describe( 'applyFilters', () => {
afterEach( () => {
removeAllFilters( 'blocks.registerBlockType' );
Expand Down
28 changes: 28 additions & 0 deletions packages/blocks/src/store/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,34 @@ export function getBlockVariations( state, blockName, scope ) {
} );
}

/**
* Returns the active block variation for a given block based on its attributes.
ntsekouras marked this conversation as resolved.
Show resolved Hide resolved
*
* @param {Object} state Data state.
* @param {string} blockName Name of block (example: “core/columns”).
* @param {Object} attributes Block attributes used to determine active variation.
* @param {WPBlockVariationScope} [scope] Block variation scope name.
*
* @return {(WPBlockVariation|undefined)} Active block variation.
*/
export function getActiveBlockVariation( state, blockName, attributes, scope ) {
const variations = getBlockVariations( state, blockName, scope );

const match = variations?.find( ( variation ) => {
if ( Array.isArray( variation.isActive ) ) {
david-szabo97 marked this conversation as resolved.
Show resolved Hide resolved
return variation.isActive.every(
( attribute ) =>
attributes[ attribute ] ===
variation.attributes[ attribute ]
);
}

return variation.isActive?.( attributes, variation.attributes );
} );

return match;
}

/**
* Returns the default block variation for the given block type.
* When there are multiple variations annotated as the default one,
Expand Down
Loading