Block List: Start multi-selection tracking on mousemove (#4585)

* Block List: Start multi-selection tracking on mousemove

* State: Abort state update if multi-select action has no effect

editor/components/block-list/index.js

@@ -34,6 +34,7 @@ import {
 	getMultiSelectedBlockUids,
 	getSelectedBlock,
 	isSelectionEnabled,
+	isMultiSelecting,
 } from '../../store/selectors';
 import { startMultiSelect, stopMultiSelect, multiSelect, selectBlock } from '../../store/actions';
 import { documentHasSelection } from '../../utils/dom';
@@ -100,7 +101,21 @@ class BlockList extends Component {
 		}
 	}
 
+	/**
+	 * Handles a pointer move event to update the extent of the current cursor
+	 * multi-selection.
+	 *
+	 * @param {MouseEvent} event A mousemove event object.
+	 *
+	 * @returns {void}
+	 */
 	onPointerMove( { clientY } ) {
+		// We don't start multi-selection until the mouse starts moving, so as
+		// to avoid dispatching multi-selection actions on an in-place click.
+		if ( ! this.props.isMultiSelecting ) {
+			this.props.onStartMultiSelect();
+		}
+
 		const boundaries = this.nodes[ this.selectionAtStart ].getBoundingClientRect();
 		const y = clientY - boundaries.top;
 		const key = findLast( this.coordMapKeys, ( coordY ) => coordY < y );
@@ -138,6 +153,14 @@ class BlockList extends Component {
 		}
 	}
 
+	/**
+	 * Binds event handlers to the document for tracking a pending multi-select
+	 * in response to a mousedown event occurring in a rendered block.
+	 *
+	 * @param {string} uid UID of the block where mousedown occurred.
+	 *
+	 * @returns {void}
+	 */
 	onSelectionStart( uid ) {
 		if ( ! this.props.isSelectionEnabled ) {
 			return;
@@ -161,8 +184,6 @@ class BlockList extends Component {
 		// Capture scroll on all elements.
 		window.addEventListener( 'scroll', this.onScroll, true );
 		window.addEventListener( 'mouseup', this.onSelectionEnd );
-
-		this.props.onStartMultiSelect();
 	}
 
 	onSelectionChange( uid ) {
@@ -183,6 +204,11 @@ class BlockList extends Component {
 		}
 	}
 
+	/**
+	 * Handles a mouseup event to end the current cursor multi-selection.
+	 *
+	 * @returns {void}
+	 */
 	onSelectionEnd() {
 		// Cancel throttled calls.
 		this.onPointerMove.cancel();
@@ -195,7 +221,11 @@ class BlockList extends Component {
 		window.removeEventListener( 'scroll', this.onScroll, true );
 		window.removeEventListener( 'mouseup', this.onSelectionEnd );
 
-		this.props.onStopMultiSelect();
+		// We may or may not be in a multi-selection when mouseup occurs (e.g.
+		// an in-place mouse click), so only trigger stop if multi-selecting.
+		if ( this.props.isMultiSelecting ) {
+			this.props.onStopMultiSelect();
+		}
 	}
 
 	onShiftSelection( uid ) {
@@ -244,6 +274,7 @@ export default connect(
 		multiSelectedBlockUids: getMultiSelectedBlockUids( state ),
 		selectedBlock: getSelectedBlock( state ),
 		isSelectionEnabled: isSelectionEnabled( state ),
+		isMultiSelecting: isMultiSelecting( state ),
 	} ),
 	( dispatch ) => ( {
 		onStartMultiSelect() {

editor/store/reducer.js

@@ -375,6 +375,11 @@ export function blockSelection( state = {
 }, action ) {
 	switch ( action.type ) {
 		case 'CLEAR_SELECTED_BLOCK':
+			if ( state.start === null && state.end === null &&
+					state.focus === null && ! state.isMultiSelecting ) {
+				return state;
+			}
+
 			return {
 				...state,
 				start: null,
@@ -383,15 +388,24 @@ export function blockSelection( state = {
 				isMultiSelecting: false,
 			};
 		case 'START_MULTI_SELECT':
+			if ( state.isMultiSelecting ) {
+				return state;
+			}
+
 			return {
 				...state,
 				isMultiSelecting: true,
 			};
 		case 'STOP_MULTI_SELECT':
+			const nextFocus = state.start === state.end ? state.focus : null;
+			if ( ! state.isMultiSelecting && nextFocus === state.focus ) {
+				return state;
+			}
+
 			return {
 				...state,
 				isMultiSelecting: false,
-				focus: state.start === state.end ? state.focus : null,
+				focus: nextFocus,
 			};
 		case 'MULTI_SELECT':
 			return {

editor/store/test/reducer.js

@@ -809,6 +809,15 @@ describe( 'state', () => {
 			expect( state ).toEqual( { start: 'ribs', end: 'ribs', focus: { editable: 'citation' }, isMultiSelecting: true } );
 		} );
 
+		it( 'should return same reference if already multi-selecting', () => {
+			const original = deepFreeze( { start: 'ribs', end: 'ribs', focus: { editable: 'citation' }, isMultiSelecting: true } );
+			const state = blockSelection( original, {
+				type: 'START_MULTI_SELECT',
+			} );
+
+			expect( state ).toBe( original );
+		} );
+
 		it( 'should end multi selection with selection', () => {
 			const original = deepFreeze( { start: 'ribs', end: 'chicken', focus: { editable: 'citation' }, isMultiSelecting: true } );
 			const state = blockSelection( original, {
@@ -818,6 +827,15 @@ describe( 'state', () => {
 			expect( state ).toEqual( { start: 'ribs', end: 'chicken', focus: null, isMultiSelecting: false } );
 		} );
 
+		it( 'should return same reference if already ended multi-selecting', () => {
+			const original = deepFreeze( { start: 'ribs', end: 'chicken', focus: null, isMultiSelecting: false } );
+			const state = blockSelection( original, {
+				type: 'STOP_MULTI_SELECT',
+			} );
+
+			expect( state ).toBe( original );
+		} );
+
 		it( 'should end multi selection without selection', () => {
 			const original = deepFreeze( { start: 'ribs', end: 'ribs', focus: { editable: 'citation' }, isMultiSelecting: true } );
 			const state = blockSelection( original, {
@@ -838,14 +856,28 @@ describe( 'state', () => {
 			expect( state1 ).toBe( original );
 		} );
 
-		it( 'should unset multi selection and select inserted block', () => {
+		it( 'should unset multi selection', () => {
 			const original = deepFreeze( { start: 'ribs', end: 'chicken' } );
 
 			const state1 = blockSelection( original, {
 				type: 'CLEAR_SELECTED_BLOCK',
 			} );
 
 			expect( state1 ).toEqual( { start: null, end: null, focus: null, isMultiSelecting: false } );
+		} );
+
+		it( 'should return same reference if clearing selection but no selection', () => {
+			const original = deepFreeze( { start: null, end: null, focus: null, isMultiSelecting: false } );
+
+			const state1 = blockSelection( original, {
+				type: 'CLEAR_SELECTED_BLOCK',
+			} );
+
+			expect( state1 ).toBe( original );
+		} );
+
+		it( 'should select inserted block', () => {
+			const original = deepFreeze( { start: 'ribs', end: 'chicken' } );
 
 			const state3 = blockSelection( original, {
 				type: 'INSERT_BLOCKS',