From 8201df837eda8e8291d032951560710c6f564077 Mon Sep 17 00:00:00 2001
From: Andrew Serong <14988353+andrewserong@users.noreply.github.com>
Date: Tue, 5 Apr 2022 12:02:08 +1000
Subject: [PATCH 1/3] Block Variation Transforms: Display as icon buttons if
each block variation has a unique icon
---
.../block-variation-transforms/index.js | 137 ++++++++++++++----
.../block-variation-transforms/style.scss | 2 +-
.../src/utils/block-variation-transforms.js | 34 ++++-
.../utils/test/block-variation-transforms.js | 42 ++++++
.../block-library/src/group/variations.js | 3 +-
5 files changed, 181 insertions(+), 37 deletions(-)
diff --git a/packages/block-editor/src/components/block-variation-transforms/index.js b/packages/block-editor/src/components/block-variation-transforms/index.js
index bcb1ccb5bd6374..57923f714449b5 100644
--- a/packages/block-editor/src/components/block-variation-transforms/index.js
+++ b/packages/block-editor/src/components/block-variation-transforms/index.js
@@ -2,14 +2,16 @@
* WordPress dependencies
*/
import { store as blocksStore } from '@wordpress/blocks';
-import { __ } from '@wordpress/i18n';
+import { __, sprintf } from '@wordpress/i18n';
import {
+ Button,
DropdownMenu,
MenuGroup,
MenuItemsChoice,
+ VisuallyHidden,
} from '@wordpress/components';
import { useSelect, useDispatch } from '@wordpress/data';
-import { useState, useEffect } from '@wordpress/element';
+import { useEffect, useMemo, useState } from '@wordpress/element';
import { chevronDown } from '@wordpress/icons';
/**
@@ -18,6 +20,77 @@ import { chevronDown } from '@wordpress/icons';
import { __experimentalGetMatchingVariation as getMatchingVariation } from '../../utils';
import { store as blockEditorStore } from '../../store';
+function VariationsButtons( {
+ className,
+ onSelectVariation,
+ selectedValue,
+ variations,
+} ) {
+ return (
+
+ );
+}
+
+function VariationsDropdown( {
+ className,
+ onSelectVariation,
+ selectedValue,
+ variations,
+} ) {
+ const selectOptions = variations.map(
+ ( { name, title, description } ) => ( {
+ value: name,
+ label: title,
+ info: description,
+ } )
+ );
+
+ return (
+
+ { () => (
+
+
+
+
+
+ ) }
+
+ );
+}
+
function __experimentalBlockVariationTransforms( { blockClientId } ) {
const [ selectedValue, setSelectedValue ] = useState();
const { updateBlockAttributes } = useDispatch( blockEditorStore );
@@ -41,15 +114,20 @@ function __experimentalBlockVariationTransforms( { blockClientId } ) {
getMatchingVariation( blockAttributes, variations )?.name
);
}, [ blockAttributes, variations ] );
+
+ // Check if each variation has a unique icon.
+ const hasUniqueIcons = useMemo( () => {
+ const variationIcons = new Set();
+ variations.forEach( ( variation ) => {
+ if ( variation.icon ) {
+ variationIcons.add( variation.icon );
+ }
+ } );
+ return variationIcons.size === variations.length;
+ }, [ variations ] );
+
if ( ! variations?.length ) return null;
- const selectOptions = variations.map(
- ( { name, title, description } ) => ( {
- value: name,
- label: title,
- info: description,
- } )
- );
const onSelectVariation = ( variationName ) => {
updateBlockAttributes( blockClientId, {
...variations.find( ( { name } ) => name === variationName )
@@ -57,30 +135,27 @@ function __experimentalBlockVariationTransforms( { blockClientId } ) {
} );
};
const baseClass = 'block-editor-block-variation-transforms';
+
+ // If each variation has a unique icon, then render the variations as a set of buttons.
+ if ( hasUniqueIcons ) {
+ return (
+
+ );
+ }
+
+ // Fallback to a dropdown list of options if each variation does not have a unique icon.
return (
-
- { () => (
-
-
-
-
-
- ) }
-
+ onSelectVariation={ onSelectVariation }
+ selectedValue={ selectedValue }
+ variations={ variations }
+ />
);
}
diff --git a/packages/block-editor/src/components/block-variation-transforms/style.scss b/packages/block-editor/src/components/block-variation-transforms/style.scss
index 2a02afdf6b7476..b828bc6020bfbe 100644
--- a/packages/block-editor/src/components/block-variation-transforms/style.scss
+++ b/packages/block-editor/src/components/block-variation-transforms/style.scss
@@ -1,5 +1,5 @@
.block-editor-block-variation-transforms {
- padding: 0 $grid-unit-20 $grid-unit-20 56px;
+ padding: 0 $grid-unit-20 $grid-unit-20 52px;
width: 100%;
.components-dropdown-menu__toggle {
diff --git a/packages/block-editor/src/utils/block-variation-transforms.js b/packages/block-editor/src/utils/block-variation-transforms.js
index 19f8e05c822056..1962a96d7f025f 100644
--- a/packages/block-editor/src/utils/block-variation-transforms.js
+++ b/packages/block-editor/src/utils/block-variation-transforms.js
@@ -13,6 +13,10 @@ import { isMatch } from 'lodash';
* This is a simple implementation for now as it takes into account only the attributes
* of a block variation and not `InnerBlocks`.
*
+ * If a variation provides an `isActive` function, this is used to check whether or not
+ * there is a matching variation, falling back to a simple comparison between the
+ * block's attributes and the variation's attributes.
+ *
* @param {Object} blockAttributes - The block attributes to try to find a match.
* @param {WPBlockVariation[]} variations - A list of block variations to test for a match.
* @return {?WPBlockVariation} - If a match is found returns it. If not or more than one matches are found returns `undefined`.
@@ -21,11 +25,33 @@ export const __experimentalGetMatchingVariation = (
blockAttributes,
variations
) => {
- if ( ! variations || ! blockAttributes ) return;
- const matches = variations.filter( ( { attributes } ) => {
- if ( ! attributes || ! Object.keys( attributes ).length ) return false;
+ if ( ! variations || ! blockAttributes ) {
+ return;
+ }
+
+ const matches = variations.filter( ( { attributes, isActive } ) => {
+ if ( ! attributes || ! Object.keys( attributes ).length ) {
+ return false;
+ }
+
+ // If an `isActive` function is provided for the variation, use this to determine
+ // whether or not there is a match, as some variations involve more logic than a
+ // simple match between the block's attributes and the variation's attributes.
+ if ( typeof isActive === 'function' ) {
+ if ( isActive( blockAttributes ) ) {
+ return true;
+ }
+ return false;
+ }
+
+ // If no `isActive` function is provided, match the block attributes against
+ // the variation's attributes.
return isMatch( blockAttributes, attributes );
} );
- if ( matches.length !== 1 ) return;
+
+ if ( matches.length !== 1 ) {
+ return;
+ }
+
return matches[ 0 ];
};
diff --git a/packages/block-editor/src/utils/test/block-variation-transforms.js b/packages/block-editor/src/utils/test/block-variation-transforms.js
index 17c82bdb951329..9986257bbbb5e5 100644
--- a/packages/block-editor/src/utils/test/block-variation-transforms.js
+++ b/packages/block-editor/src/utils/test/block-variation-transforms.js
@@ -40,6 +40,19 @@ describe( 'getMatchingVariation', () => {
getMatchingVariation( { level: 1, other: 'prop' }, variations )
).toBeUndefined();
} );
+ it( 'when isActive uses different logic to attributes, but attributes match', () => {
+ const variations = [
+ {
+ name: 'one',
+ attributes: { level: 1, content: 'hi' },
+ isActive: ( blockAttributes ) => ! blockAttributes.level,
+ },
+ ];
+
+ expect(
+ getMatchingVariation( { level: 1, content: 'hi' }, variations )
+ ).toBeUndefined();
+ } );
} );
describe( 'should find a match', () => {
it( 'when variation has one attribute', () => {
@@ -66,5 +79,34 @@ describe( 'getMatchingVariation', () => {
).name
).toEqual( 'one' );
} );
+ it( 'when isActive uses different logic to attributes, and attributes do not match', () => {
+ const variations = [
+ {
+ name: 'one',
+ attributes: { level: 1, content: 'hi' },
+ isActive: ( blockAttributes ) => ! blockAttributes.level,
+ },
+ ];
+
+ expect(
+ getMatchingVariation( { content: 'hi' }, variations ).name
+ ).toEqual( 'one' );
+ } );
+
+ it( 'when isActive uses different logic to attributes, and attributes do match', () => {
+ const variations = [
+ {
+ name: 'one',
+ attributes: { level: 1, content: 'hi' },
+ isActive: ( blockAttributes ) =>
+ ! blockAttributes.level || blockAttributes.level === 1,
+ },
+ ];
+
+ expect(
+ getMatchingVariation( { level: 1, content: 'hi' }, variations )
+ .name
+ ).toEqual( 'one' );
+ } );
} );
} );
diff --git a/packages/block-library/src/group/variations.js b/packages/block-library/src/group/variations.js
index 047c08b881d148..76c366ec7f23dd 100644
--- a/packages/block-library/src/group/variations.js
+++ b/packages/block-library/src/group/variations.js
@@ -2,7 +2,7 @@
* WordPress dependencies
*/
import { __ } from '@wordpress/i18n';
-import { row, stack } from '@wordpress/icons';
+import { group, row, stack } from '@wordpress/icons';
const variations = [
{
@@ -14,6 +14,7 @@ const variations = [
isActive: ( blockAttributes ) =>
! blockAttributes.layout ||
blockAttributes.layout?.type === 'default',
+ icon: group,
},
{
name: 'group-row',
From d2f9e69a00522cedb91c2eb1b7329b279c36047e Mon Sep 17 00:00:00 2001
From: Andrew Serong <14988353+andrewserong@users.noreply.github.com>
Date: Tue, 5 Apr 2022 16:35:07 +1000
Subject: [PATCH 2/3] Remove internal state, switch to using
getActiveBlockVariation selector
---
.../block-variation-transforms/index.js | 51 ++++++++++++-------
.../src/utils/block-variation-transforms.js | 34 ++-----------
.../utils/test/block-variation-transforms.js | 42 ---------------
3 files changed, 36 insertions(+), 91 deletions(-)
diff --git a/packages/block-editor/src/components/block-variation-transforms/index.js b/packages/block-editor/src/components/block-variation-transforms/index.js
index 57923f714449b5..05e98c3e1e6542 100644
--- a/packages/block-editor/src/components/block-variation-transforms/index.js
+++ b/packages/block-editor/src/components/block-variation-transforms/index.js
@@ -11,13 +11,12 @@ import {
VisuallyHidden,
} from '@wordpress/components';
import { useSelect, useDispatch } from '@wordpress/data';
-import { useEffect, useMemo, useState } from '@wordpress/element';
+import { useMemo } from '@wordpress/element';
import { chevronDown } from '@wordpress/icons';
/**
* Internal dependencies
*/
-import { __experimentalGetMatchingVariation as getMatchingVariation } from '../../utils';
import { store as blockEditorStore } from '../../store';
function VariationsButtons( {
@@ -36,11 +35,15 @@ function VariationsButtons( {
key={ variation.name }
icon={ variation.icon }
isPressed={ selectedValue === variation.name }
- label={ sprintf(
- /* translators: %s: Name of the block variation */
- __( 'Transform to %s' ),
- variation.title
- ) }
+ label={
+ selectedValue === variation.name
+ ? variation.title
+ : sprintf(
+ /* translators: %s: Name of the block variation */
+ __( 'Transform to %s' ),
+ variation.title
+ )
+ }
onClick={ () => onSelectVariation( variation.name ) }
aria-label={ variation.title }
showTooltip
@@ -92,28 +95,36 @@ function VariationsDropdown( {
}
function __experimentalBlockVariationTransforms( { blockClientId } ) {
- const [ selectedValue, setSelectedValue ] = useState();
const { updateBlockAttributes } = useDispatch( blockEditorStore );
- const { variations, blockAttributes } = useSelect(
+ const { variations, blockAttributes, blockName } = useSelect(
( select ) => {
const { getBlockVariations } = select( blocksStore );
const { getBlockName, getBlockAttributes } = select(
blockEditorStore
);
- const blockName = blockClientId && getBlockName( blockClientId );
+ const name = blockClientId && getBlockName( blockClientId );
return {
- variations:
- blockName && getBlockVariations( blockName, 'transform' ),
blockAttributes: getBlockAttributes( blockClientId ),
+ blockName: name,
+ variations: name && getBlockVariations( name, 'transform' ),
};
},
[ blockClientId ]
);
- useEffect( () => {
- setSelectedValue(
- getMatchingVariation( blockAttributes, variations )?.name
- );
- }, [ blockAttributes, variations ] );
+ const { activeBlockVariation } = useSelect(
+ ( select ) => {
+ const { getActiveBlockVariation } = select( blocksStore );
+ return {
+ activeBlockVariation: getActiveBlockVariation(
+ blockName,
+ blockAttributes
+ ),
+ };
+ },
+ [ blockAttributes, blockName ]
+ );
+
+ const selectedValue = activeBlockVariation?.name;
// Check if each variation has a unique icon.
const hasUniqueIcons = useMemo( () => {
@@ -126,16 +137,18 @@ function __experimentalBlockVariationTransforms( { blockClientId } ) {
return variationIcons.size === variations.length;
}, [ variations ] );
- if ( ! variations?.length ) return null;
-
const onSelectVariation = ( variationName ) => {
updateBlockAttributes( blockClientId, {
...variations.find( ( { name } ) => name === variationName )
.attributes,
} );
};
+
const baseClass = 'block-editor-block-variation-transforms';
+ // Skip rendering if there are no variations
+ if ( ! variations?.length ) return null;
+
// If each variation has a unique icon, then render the variations as a set of buttons.
if ( hasUniqueIcons ) {
return (
diff --git a/packages/block-editor/src/utils/block-variation-transforms.js b/packages/block-editor/src/utils/block-variation-transforms.js
index 1962a96d7f025f..19f8e05c822056 100644
--- a/packages/block-editor/src/utils/block-variation-transforms.js
+++ b/packages/block-editor/src/utils/block-variation-transforms.js
@@ -13,10 +13,6 @@ import { isMatch } from 'lodash';
* This is a simple implementation for now as it takes into account only the attributes
* of a block variation and not `InnerBlocks`.
*
- * If a variation provides an `isActive` function, this is used to check whether or not
- * there is a matching variation, falling back to a simple comparison between the
- * block's attributes and the variation's attributes.
- *
* @param {Object} blockAttributes - The block attributes to try to find a match.
* @param {WPBlockVariation[]} variations - A list of block variations to test for a match.
* @return {?WPBlockVariation} - If a match is found returns it. If not or more than one matches are found returns `undefined`.
@@ -25,33 +21,11 @@ export const __experimentalGetMatchingVariation = (
blockAttributes,
variations
) => {
- if ( ! variations || ! blockAttributes ) {
- return;
- }
-
- const matches = variations.filter( ( { attributes, isActive } ) => {
- if ( ! attributes || ! Object.keys( attributes ).length ) {
- return false;
- }
-
- // If an `isActive` function is provided for the variation, use this to determine
- // whether or not there is a match, as some variations involve more logic than a
- // simple match between the block's attributes and the variation's attributes.
- if ( typeof isActive === 'function' ) {
- if ( isActive( blockAttributes ) ) {
- return true;
- }
- return false;
- }
-
- // If no `isActive` function is provided, match the block attributes against
- // the variation's attributes.
+ if ( ! variations || ! blockAttributes ) return;
+ const matches = variations.filter( ( { attributes } ) => {
+ if ( ! attributes || ! Object.keys( attributes ).length ) return false;
return isMatch( blockAttributes, attributes );
} );
-
- if ( matches.length !== 1 ) {
- return;
- }
-
+ if ( matches.length !== 1 ) return;
return matches[ 0 ];
};
diff --git a/packages/block-editor/src/utils/test/block-variation-transforms.js b/packages/block-editor/src/utils/test/block-variation-transforms.js
index 9986257bbbb5e5..17c82bdb951329 100644
--- a/packages/block-editor/src/utils/test/block-variation-transforms.js
+++ b/packages/block-editor/src/utils/test/block-variation-transforms.js
@@ -40,19 +40,6 @@ describe( 'getMatchingVariation', () => {
getMatchingVariation( { level: 1, other: 'prop' }, variations )
).toBeUndefined();
} );
- it( 'when isActive uses different logic to attributes, but attributes match', () => {
- const variations = [
- {
- name: 'one',
- attributes: { level: 1, content: 'hi' },
- isActive: ( blockAttributes ) => ! blockAttributes.level,
- },
- ];
-
- expect(
- getMatchingVariation( { level: 1, content: 'hi' }, variations )
- ).toBeUndefined();
- } );
} );
describe( 'should find a match', () => {
it( 'when variation has one attribute', () => {
@@ -79,34 +66,5 @@ describe( 'getMatchingVariation', () => {
).name
).toEqual( 'one' );
} );
- it( 'when isActive uses different logic to attributes, and attributes do not match', () => {
- const variations = [
- {
- name: 'one',
- attributes: { level: 1, content: 'hi' },
- isActive: ( blockAttributes ) => ! blockAttributes.level,
- },
- ];
-
- expect(
- getMatchingVariation( { content: 'hi' }, variations ).name
- ).toEqual( 'one' );
- } );
-
- it( 'when isActive uses different logic to attributes, and attributes do match', () => {
- const variations = [
- {
- name: 'one',
- attributes: { level: 1, content: 'hi' },
- isActive: ( blockAttributes ) =>
- ! blockAttributes.level || blockAttributes.level === 1,
- },
- ];
-
- expect(
- getMatchingVariation( { level: 1, content: 'hi' }, variations )
- .name
- ).toEqual( 'one' );
- } );
} );
} );
From eebbba78b7d94c1ae2b38f12d75bfd8500d867d8 Mon Sep 17 00:00:00 2001
From: Andrew Serong <14988353+andrewserong@users.noreply.github.com>
Date: Wed, 6 Apr 2022 10:07:24 +1000
Subject: [PATCH 3/3] Consolidate useSelect and simplify component rendering
---
.../block-variation-transforms/index.js | 39 +++++--------------
1 file changed, 10 insertions(+), 29 deletions(-)
diff --git a/packages/block-editor/src/components/block-variation-transforms/index.js b/packages/block-editor/src/components/block-variation-transforms/index.js
index 05e98c3e1e6542..b8099e46c025b0 100644
--- a/packages/block-editor/src/components/block-variation-transforms/index.js
+++ b/packages/block-editor/src/components/block-variation-transforms/index.js
@@ -96,32 +96,24 @@ function VariationsDropdown( {
function __experimentalBlockVariationTransforms( { blockClientId } ) {
const { updateBlockAttributes } = useDispatch( blockEditorStore );
- const { variations, blockAttributes, blockName } = useSelect(
+ const { activeBlockVariation, variations } = useSelect(
( select ) => {
- const { getBlockVariations } = select( blocksStore );
+ const { getActiveBlockVariation, getBlockVariations } = select(
+ blocksStore
+ );
const { getBlockName, getBlockAttributes } = select(
blockEditorStore
);
const name = blockClientId && getBlockName( blockClientId );
- return {
- blockAttributes: getBlockAttributes( blockClientId ),
- blockName: name,
- variations: name && getBlockVariations( name, 'transform' ),
- };
- },
- [ blockClientId ]
- );
- const { activeBlockVariation } = useSelect(
- ( select ) => {
- const { getActiveBlockVariation } = select( blocksStore );
return {
activeBlockVariation: getActiveBlockVariation(
- blockName,
- blockAttributes
+ name,
+ getBlockAttributes( blockClientId )
),
+ variations: name && getBlockVariations( name, 'transform' ),
};
},
- [ blockAttributes, blockName ]
+ [ blockClientId ]
);
const selectedValue = activeBlockVariation?.name;
@@ -149,21 +141,10 @@ function __experimentalBlockVariationTransforms( { blockClientId } ) {
// Skip rendering if there are no variations
if ( ! variations?.length ) return null;
- // If each variation has a unique icon, then render the variations as a set of buttons.
- if ( hasUniqueIcons ) {
- return (
-
- );
- }
+ const Component = hasUniqueIcons ? VariationsButtons : VariationsDropdown;
- // Fallback to a dropdown list of options if each variation does not have a unique icon.
return (
-