Fix multiselecting blocks using shift + arrow (#11237)

* Fix multiselecting blocks using shift + arrow * Test multiple shift + arrows selections * Fix end2end test failures
WordPress · Oct 30, 2018 · 646c531 · 646c531
1 parent db728b7
commit 646c531
Show file tree

Hide file tree

Showing 3 changed files with 71 additions and 53 deletions.
diff --git a/packages/editor/src/components/block-list/block.js b/packages/editor/src/components/block-list/block.js
@@ -93,6 +93,12 @@ export class BlockListBlock extends Component {
 		if ( this.props.isSelected && ! prevProps.isSelected ) {
 			this.focusTabbable( true );
 		}
+
+		// When triggering a multi-selection,
+		// move the focus to the wrapper of the first selected block.
+		if ( this.props.isFirstMultiSelected && ! prevProps.isFirstMultiSelected ) {
+			this.wrapperNode.focus();
+		}
 	}
 
 	setBlockListRef( node ) {
@@ -314,7 +320,11 @@ export class BlockListBlock extends Component {
 	deleteOrInsertAfterWrapper( event ) {
 		const { keyCode, target } = event;
 
-		if ( target !== this.wrapperNode || this.props.isLocked ) {
+		if (
+			! this.props.isSelected ||
+			target !== this.wrapperNode ||
+			this.props.isLocked
+		) {
 			return;
 		}
 

diff --git a/packages/editor/src/components/block-toolbar/index.js b/packages/editor/src/components/block-toolbar/index.js
@@ -2,8 +2,7 @@
  * WordPress Dependencies
  */
 import { withSelect } from '@wordpress/data';
-import { Component, createRef, Fragment } from '@wordpress/element';
-import { focus } from '@wordpress/dom';
+import { Fragment } from '@wordpress/element';
 
 /**
  * Internal Dependencies
@@ -14,63 +13,32 @@ import BlockControls from '../block-controls';
 import BlockFormatControls from '../block-format-controls';
 import BlockSettingsMenu from '../block-settings-menu';
 
-class BlockToolbar extends Component {
-	constructor() {
-		super( ...arguments );
-		this.container = createRef();
+function BlockToolbar( { blockClientIds, isValid, mode } ) {
+	if ( blockClientIds.length === 0 ) {
+		return null;
 	}
 
-	componentDidMount() {
-		if ( this.props.blockClientIds.length > 1 ) {
-			this.focusContainer();
-		}
-	}
-
-	componentDidUpdate( prevProps ) {
-		if (
-			prevProps.blockClientIds.length <= 1 &&
-			this.props.blockClientIds.length > 1
-		) {
-			this.focusContainer();
-		}
-	}
-
-	focusContainer() {
-		const tabbables = focus.tabbable.find( this.container.current );
-		if ( tabbables.length ) {
-			tabbables[ 0 ].focus();
-		}
-	}
-
-	render() {
-		const { blockClientIds, isValid, mode } = this.props;
-
-		if ( blockClientIds.length === 0 ) {
-			return null;
-		}
-
-		if ( blockClientIds.length > 1 ) {
-			return (
-				<div className="editor-block-toolbar" ref={ this.container }>
-					<MultiBlocksSwitcher />
-					<BlockSettingsMenu clientIds={ blockClientIds } />
-				</div>
-			);
-		}
-
+	if ( blockClientIds.length > 1 ) {
 		return (
 			<div className="editor-block-toolbar">
-				{ mode === 'visual' && isValid && (
-					<Fragment>
-						<BlockSwitcher clientIds={ blockClientIds } />
-						<BlockControls.Slot />
-						<BlockFormatControls.Slot />
-					</Fragment>
-				) }
+				<MultiBlocksSwitcher />
 				<BlockSettingsMenu clientIds={ blockClientIds } />
 			</div>
 		);
 	}
+
+	return (
+		<div className="editor-block-toolbar">
+			{ mode === 'visual' && isValid && (
+				<Fragment>
+					<BlockSwitcher clientIds={ blockClientIds } />
+					<BlockControls.Slot />
+					<BlockFormatControls.Slot />
+				</Fragment>
+			) }
+			<BlockSettingsMenu clientIds={ blockClientIds } />
+		</div>
+	);
 }
 
 export default withSelect( ( select ) => {

diff --git a/test/e2e/specs/multi-block-selection.test.js b/test/e2e/specs/multi-block-selection.test.js
@@ -10,7 +10,7 @@ import {
 } from '../support/utils';
 
 describe( 'Multi-block selection', () => {
-	beforeAll( async () => {
+	beforeEach( async () => {
 		await newPost();
 	} );
 
@@ -77,4 +77,44 @@ describe( 'Multi-block selection', () => {
 		await pressWithModifier( META_KEY, 'a' );
 		await expectMultiSelected( blocks, true );
 	} );
+
+	it( 'Should select/unselect multiple blocks using Shift + Arrows', async () => {
+		const firstBlockSelector = '[data-type="core/paragraph"]';
+		const secondBlockSelector = '[data-type="core/image"]';
+		const thirdBlockSelector = '[data-type="core/quote"]';
+		const multiSelectedCssClass = 'is-multi-selected';
+
+		// Creating test blocks
+		await clickBlockAppender();
+		await page.keyboard.type( 'First Paragraph' );
+		await insertBlock( 'Image' );
+		await insertBlock( 'Quote' );
+		await page.keyboard.type( 'Quote Block' );
+
+		const blocks = [ firstBlockSelector, secondBlockSelector, thirdBlockSelector ];
+		const expectMultiSelected = async ( selectors, areMultiSelected ) => {
+			for ( const selector of selectors ) {
+				const className = await page.$eval( selector, ( element ) => element.className );
+				if ( areMultiSelected ) {
+					expect( className ).toEqual( expect.stringContaining( multiSelectedCssClass ) );
+				} else {
+					expect( className ).not.toEqual( expect.stringContaining( multiSelectedCssClass ) );
+				}
+			}
+		};
+
+		// Default: No selection
+		await expectMultiSelected( blocks, false );
+
+		// Multiselect via Shift + click
+		await page.mouse.move( 200, 300 );
+		await page.click( firstBlockSelector );
+		await page.keyboard.down( 'Shift' );
+		await page.keyboard.press( 'ArrowDown' ); // Two blocks selected
+		await page.keyboard.press( 'ArrowDown' ); // Three blocks selected
+		await page.keyboard.up( 'Shift' );
+
+		// Verify selection
+		await expectMultiSelected( blocks, true );
+	} );
 } );