diff --git a/editor/components/block-list/index.js b/editor/components/block-list/index.js index 7ceb4d2db9954f..c6546aeb0b8ec1 100644 --- a/editor/components/block-list/index.js +++ b/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() { diff --git a/editor/store/reducer.js b/editor/store/reducer.js index d075e2cfe37be4..26d9ba77a5e783 100644 --- a/editor/store/reducer.js +++ b/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 { diff --git a/editor/store/test/reducer.js b/editor/store/test/reducer.js index 243253fe4eab78..b545dbbed087d7 100644 --- a/editor/store/test/reducer.js +++ b/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,7 +856,7 @@ 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, { @@ -846,6 +864,20 @@ describe( 'state', () => { } ); 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',