Block Switcher: Render disabled button even if multi-selection (#13431)

WordPress · Jan 25, 2019 · c8cb3c2 · c8cb3c2
1 parent 947ff27
commit c8cb3c2
Show file tree

Hide file tree

Showing 4 changed files with 75 additions and 26 deletions.
diff --git a/packages/editor/CHANGELOG.md b/packages/editor/CHANGELOG.md
@@ -4,6 +4,10 @@
 
 - Added `createCustomColorsHOC` for creating a higher order `withCustomColors` component.
 
+### Bug Fixes
+
+- BlockSwitcher will now consistently render an icon for block multi-selections.
+
 ### Internal
 
 - Removed `jQuery` dependency

diff --git a/packages/editor/src/components/block-switcher/index.js b/packages/editor/src/components/block-switcher/index.js
@@ -1,7 +1,7 @@
 /**
  * External dependencies
  */
-import { castArray, filter, first, mapKeys, orderBy } from 'lodash';
+import { castArray, filter, first, mapKeys, orderBy, uniq, map } from 'lodash';
 
 /**
  * WordPress dependencies
@@ -53,21 +53,28 @@ export class BlockSwitcher extends Component {
 			'desc'
 		);
 
-		const sourceBlockName = blocks[ 0 ].name;
-		const blockType = getBlockType( sourceBlockName );
+		// When selection consists of blocks of multiple types, display an
+		// appropriate icon to communicate the non-uniformity.
+		const isSelectionOfSameType = uniq( map( blocks, 'name' ) ).length === 1;
+
+		let icon;
+		if ( isSelectionOfSameType ) {
+			const sourceBlockName = blocks[ 0 ].name;
+			const blockType = getBlockType( sourceBlockName );
+			icon = blockType.icon;
+		} else {
+			icon = 'layout';
+		}
 
 		if ( ! hasBlockStyles && ! possibleBlockTransformations.length ) {
-			if ( blocks.length > 1 ) {
-				return null;
-			}
 			return (
 				<Toolbar>
 					<IconButton
 						disabled
 						className="editor-block-switcher__no-switcher-icon"
 						label={ __( 'Block icon' ) }
 					>
-						<BlockIcon icon={ blockType.icon } showColors />
+						<BlockIcon icon={ icon } showColors />
 					</IconButton>
 				</Toolbar>
 			);
@@ -110,7 +117,7 @@ export class BlockSwitcher extends Component {
 								tooltip={ label }
 								onKeyDown={ openOnArrowDown }
 							>
-								<BlockIcon icon={ blockType.icon } showColors />
+								<BlockIcon icon={ icon } showColors />
 								<SVG className="editor-block-switcher__transform" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24"><Path d="M6.5 8.9c.6-.6 1.4-.9 2.2-.9h6.9l-1.3 1.3 1.4 1.4L19.4 7l-3.7-3.7-1.4 1.4L15.6 6H8.7c-1.4 0-2.6.5-3.6 1.5l-2.8 2.8 1.4 1.4 2.8-2.8zm13.8 2.4l-2.8 2.8c-.6.6-1.3.9-2.1.9h-7l1.3-1.3-1.4-1.4L4.6 16l3.7 3.7 1.4-1.4L8.4 17h6.9c1.3 0 2.6-.5 3.5-1.5l2.8-2.8-1.3-1.4z" /></SVG>
 							</IconButton>
 						</Toolbar>

diff --git a/packages/editor/src/components/block-switcher/test/__snapshots__/index.js.snap b/packages/editor/src/components/block-switcher/test/__snapshots__/index.js.snap
@@ -1,5 +1,30 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`BlockSwitcher should render disabled block switcher with multi block of different types when no transforms 1`] = `
+<Toolbar>
+  <IconButton
+    className="editor-block-switcher__no-switcher-icon"
+    disabled={true}
+    label="Block icon"
+  >
+    <BlockIcon
+      icon="layout"
+      showColors={true}
+    />
+  </IconButton>
+</Toolbar>
+`;
+
+exports[`BlockSwitcher should render enabled block switcher with multi block when transforms exist 1`] = `
+<Dropdown
+  className="editor-block-switcher"
+  contentClassName="editor-block-switcher__popover"
+  position="bottom right"
+  renderContent={[Function]}
+  renderToggle={[Function]}
+/>
+`;
+
 exports[`BlockSwitcher should render switcher with blocks 1`] = `
 <Dropdown
   className="editor-block-switcher"

diff --git a/packages/editor/src/components/block-switcher/test/index.js b/packages/editor/src/components/block-switcher/test/index.js
@@ -42,7 +42,7 @@ describe( 'BlockSwitcher', () => {
 			level: 3,
 		},
 		isValid: true,
-		name: 'core/paragraph',
+		name: 'core/heading',
 		originalContent: '<h3>I am the greatest!</h3>',
 		clientId: 'c2403fd2-4e63-5ffa-b71c-1e0ea656c5b0',
 	};
@@ -54,11 +54,19 @@ describe( 'BlockSwitcher', () => {
 			edit: () => { },
 			save: () => {},
 			transforms: {
-				to: [ {
-					type: 'block',
-					blocks: [ 'core/paragraph' ],
-					transform: () => {},
-				} ],
+				to: [
+					{
+						type: 'block',
+						blocks: [ 'core/paragraph' ],
+						transform: () => {},
+					},
+					{
+						type: 'block',
+						blocks: [ 'core/paragraph' ],
+						transform: () => {},
+						isMultiBlock: true,
+					},
+				],
 			},
 		} );
 
@@ -93,6 +101,7 @@ describe( 'BlockSwitcher', () => {
 			headingBlock1,
 		];
 		const inserterItems = [
+			{ name: 'core/heading', frecency: 1 },
 			{ name: 'core/paragraph', frecency: 1 },
 		];
 
@@ -101,24 +110,28 @@ describe( 'BlockSwitcher', () => {
 		expect( wrapper ).toMatchSnapshot();
 	} );
 
-	test( 'should not render block switcher with multi block of different types.', () => {
-		const blocks = [
-			headingBlock1,
-			textBlock,
+	test( 'should render disabled block switcher with multi block of different types when no transforms', () => {
+		const blocks = [ headingBlock1, textBlock ];
+		const inserterItems = [
+			{ name: 'core/heading', frecency: 1 },
+			{ name: 'core/paragraph', frecency: 1 },
 		];
-		const wrapper = shallow( <BlockSwitcher blocks={ blocks } /> );
 
-		expect( wrapper.html() ).toBeNull();
+		const wrapper = shallow( <BlockSwitcher blocks={ blocks } inserterItems={ inserterItems } /> );
+
+		expect( wrapper ).toMatchSnapshot();
 	} );
 
-	test( 'should not render a component when the multi selected types of blocks match.', () => {
-		const blocks = [
-			headingBlock1,
-			headingBlock2,
+	test( 'should render enabled block switcher with multi block when transforms exist', () => {
+		const blocks = [ headingBlock1, headingBlock2 ];
+		const inserterItems = [
+			{ name: 'core/heading', frecency: 1 },
+			{ name: 'core/paragraph', frecency: 1 },
 		];
-		const wrapper = shallow( <BlockSwitcher blocks={ blocks } /> );
 
-		expect( wrapper.html() ).toBeNull();
+		const wrapper = shallow( <BlockSwitcher blocks={ blocks } inserterItems={ inserterItems } /> );
+
+		expect( wrapper ).toMatchSnapshot();
 	} );
 
 	describe( 'Dropdown', () => {