Display matching variation icon in Block Switcher (#27903)

* Display matching variation icon in Block Switcher * Fix dependency in useSelect
WordPress · Jan 7, 2021 · 1740d48 · youknowriad · Jan 7, 2021 · 1740d48
1 parent 4a81120
commit 1740d48
Show file tree

Hide file tree

Showing 3 changed files with 70 additions and 52 deletions.
diff --git a/packages/block-editor/src/components/block-switcher/index.js b/packages/block-editor/src/components/block-switcher/index.js
@@ -20,56 +20,53 @@ import { stack } from '@wordpress/icons';
 /**
  * Internal dependencies
  */
+import { store as blockEditorStore } from '../../store';
+import useBlockDisplayInformation from '../use-block-display-information';
 import BlockIcon from '../block-icon';
 import BlockTransformationsMenu from './block-transformations-menu';
 import BlockStylesMenu from './block-styles-menu';
 
-const BlockSwitcher = ( { clientIds } ) => {
-	const { replaceBlocks } = useDispatch( 'core/block-editor' );
-	const {
-		blocks,
-		possibleBlockTransformations,
-		hasBlockStyles,
-		icon,
-	} = useSelect(
+export const BlockSwitcherDropdownMenu = ( { clientIds, blocks } ) => {
+	const { replaceBlocks } = useDispatch( blockEditorStore );
+	const blockInformation = useBlockDisplayInformation( blocks[ 0 ].clientId );
+	const { possibleBlockTransformations, hasBlockStyles, icon } = useSelect(
 		( select ) => {
-			const {
-				getBlocksByClientId,
-				getBlockRootClientId,
-				getBlockTransformItems,
-			} = select( 'core/block-editor' );
+			const { getBlockRootClientId, getBlockTransformItems } = select(
+				blockEditorStore
+			);
 			const { getBlockStyles, getBlockType } = select( blocksStore );
 			const rootClientId = getBlockRootClientId(
 				castArray( clientIds )[ 0 ]
 			);
-			const _blocks = getBlocksByClientId( clientIds );
-			const isSingleBlockSelected = _blocks?.length === 1;
-			const firstBlock = !! _blocks?.length ? _blocks[ 0 ] : null;
+			const [ { name: firstBlockName } ] = blocks;
+			const _isSingleBlockSelected = blocks.length === 1;
 			const styles =
-				isSingleBlockSelected && getBlockStyles( firstBlock.name );
-			// When selection consists of blocks of multiple types, display an
-			// appropriate icon to communicate the non-uniformity.
-			const isSelectionOfSameType =
-				uniq( ( _blocks || [] ).map( ( { name } ) => name ) ).length ===
-				1;
+				_isSingleBlockSelected && getBlockStyles( firstBlockName );
+			let _icon;
+			if ( _isSingleBlockSelected ) {
+				_icon = blockInformation?.icon; // Take into account active block variations.
+			} else {
+				const isSelectionOfSameType =
+					uniq( blocks.map( ( { name } ) => name ) ).length === 1;
+				// When selection consists of blocks of multiple types, display an
+				// appropriate icon to communicate the non-uniformity.
+				_icon = isSelectionOfSameType
+					? getBlockType( firstBlockName )?.icon
+					: stack;
+			}
 			return {
-				blocks: _blocks,
-				possibleBlockTransformations:
-					_blocks && getBlockTransformItems( _blocks, rootClientId ),
+				possibleBlockTransformations: getBlockTransformItems(
+					blocks,
+					rootClientId
+				),
 				hasBlockStyles: !! styles?.length,
-				icon: isSelectionOfSameType
-					? getBlockType( firstBlock.name )?.icon
-					: stack,
+				icon: _icon,
 			};
 		},
-		[ clientIds ]
+		[ clientIds, blocks, blockInformation?.icon ]
 	);
-
-	if ( ! blocks?.length ) return null;
-
 	const onTransform = ( name ) =>
 		replaceBlocks( clientIds, switchToBlockType( blocks, name ) );
-
 	const hasPossibleBlockTransformations = !! possibleBlockTransformations.length;
 	if ( ! hasBlockStyles && ! hasPossibleBlockTransformations ) {
 		return (
@@ -83,7 +80,6 @@ const BlockSwitcher = ( { clientIds } ) => {
 			</ToolbarGroup>
 		);
 	}
-
 	const blockSwitcherLabel =
 		1 === blocks.length
 			? __( 'Change block type or style' )
@@ -152,4 +148,16 @@ const BlockSwitcher = ( { clientIds } ) => {
 	);
 };
 
+export const BlockSwitcher = ( { clientIds } ) => {
+	const blocks = useSelect(
+		( select ) =>
+			select( blockEditorStore ).getBlocksByClientId( clientIds ),
+		[ clientIds ]
+	);
+
+	return !! blocks?.length ? (
+		<BlockSwitcherDropdownMenu clientIds={ clientIds } blocks={ blocks } />
+	) : null;
+};
+
 export default BlockSwitcher;
diff --git a/packages/block-editor/src/components/block-switcher/test/__snapshots__/index.js.snap b/packages/block-editor/src/components/block-switcher/test/__snapshots__/index.js.snap
@@ -1,6 +1,6 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`BlockSwitcher should render disabled block switcher with multi block of different types when no transforms 1`] = `
+exports[` BlockSwitcherDropdownMenu should render disabled block switcher with multi block of different types when no transforms 1`] = `
 <ToolbarGroup>
   <ForwardRef(ToolbarButton)
     className="block-editor-block-switcher__no-switcher-icon"
@@ -25,15 +25,15 @@ exports[`BlockSwitcher should render disabled block switcher with multi block of
 </ToolbarGroup>
 `;
 
-exports[`BlockSwitcher should render enabled block switcher with multi block when transforms exist 1`] = `
+exports[` BlockSwitcherDropdownMenu should render enabled block switcher with multi block when transforms exist 1`] = `
 <ToolbarGroup>
   <ForwardRef(ToolbarItem)>
     <Component />
   </ForwardRef(ToolbarItem)>
 </ToolbarGroup>
 `;
 
-exports[`BlockSwitcher should render switcher with blocks 1`] = `
+exports[` BlockSwitcherDropdownMenu should render switcher with blocks 1`] = `
 <ToolbarGroup>
   <ForwardRef(ToolbarItem)>
     <Component />

diff --git a/packages/block-editor/src/components/block-switcher/test/index.js b/packages/block-editor/src/components/block-switcher/test/index.js
@@ -15,11 +15,18 @@ import { stack } from '@wordpress/icons';
 /**
  * Internal dependencies
  */
-import BlockSwitcher from '../';
+import { BlockSwitcher, BlockSwitcherDropdownMenu } from '../';
 
 jest.mock( '@wordpress/data/src/components/use-select', () => jest.fn() );
 
 describe( 'BlockSwitcher', () => {
+	test( 'should not render block switcher without blocks', () => {
+		useSelect.mockImplementation( () => ( {} ) );
+		const wrapper = shallow( <BlockSwitcher /> );
+		expect( wrapper.html() ).toBeNull();
+	} );
+} );
+describe( ' BlockSwitcherDropdownMenu', () => {
 	const headingBlock1 = {
 		attributes: {
 			content: [ 'How are you?' ],
@@ -97,56 +104,59 @@ describe( 'BlockSwitcher', () => {
 		unregisterBlockType( 'core/paragraph' );
 	} );
 
-	test( 'should not render block switcher without blocks', () => {
-		useSelect.mockImplementation( () => ( {} ) );
-		const wrapper = shallow( <BlockSwitcher /> );
-		expect( wrapper.html() ).toBeNull();
-	} );
-
 	test( 'should render switcher with blocks', () => {
 		useSelect.mockImplementation( () => ( {
-			blocks: [ headingBlock1 ],
 			possibleBlockTransformations: [
 				{ name: 'core/heading', frecency: 1 },
 				{ name: 'core/paragraph', frecency: 1 },
 			],
 		} ) );
-		const wrapper = shallow( <BlockSwitcher /> );
+		const wrapper = shallow(
+			<BlockSwitcherDropdownMenu blocks={ [ headingBlock1 ] } />
+		);
 		expect( wrapper ).toMatchSnapshot();
 	} );
 
 	test( 'should render disabled block switcher with multi block of different types when no transforms', () => {
 		useSelect.mockImplementation( () => ( {
-			blocks: [ headingBlock1, textBlock ],
 			possibleBlockTransformations: [],
 			icon: stack,
 		} ) );
-		const wrapper = shallow( <BlockSwitcher /> );
+		const wrapper = shallow(
+			<BlockSwitcherDropdownMenu
+				blocks={ [ headingBlock1, textBlock ] }
+			/>
+		);
 		expect( wrapper ).toMatchSnapshot();
 	} );
 
 	test( 'should render enabled block switcher with multi block when transforms exist', () => {
 		useSelect.mockImplementation( () => ( {
-			blocks: [ headingBlock1, headingBlock2 ],
 			possibleBlockTransformations: [
 				{ name: 'core/heading', frecency: 1 },
 				{ name: 'core/paragraph', frecency: 1 },
 			],
 		} ) );
-		const wrapper = shallow( <BlockSwitcher /> );
+		const wrapper = shallow(
+			<BlockSwitcherDropdownMenu
+				blocks={ [ headingBlock1, headingBlock2 ] }
+			/>
+		);
 		expect( wrapper ).toMatchSnapshot();
 	} );
 
 	describe( 'Dropdown', () => {
 		beforeAll( () => {
 			useSelect.mockImplementation( () => ( {
-				blocks: [ headingBlock1 ],
 				possibleBlockTransformations: [
 					{ name: 'core/paragraph', frecency: 3 },
 				],
 			} ) );
 		} );
-		const getDropdown = () => mount( <BlockSwitcher /> ).find( 'Dropdown' );
+		const getDropdown = () =>
+			mount(
+				<BlockSwitcherDropdownMenu blocks={ [ headingBlock1 ] } />
+			).find( 'Dropdown' );
 
 		test( 'should dropdown exist', () => {
 			expect( getDropdown() ).toHaveLength( 1 );