From 14d2bcd8594aaf46ce62bc983dbe0ab9df435361 Mon Sep 17 00:00:00 2001 From: morgan Date: Fri, 27 Oct 2017 10:19:46 +1000 Subject: [PATCH 1/4] only pass through potential focus while not in multi-block selection --- editor/modes/visual-editor/block.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/editor/modes/visual-editor/block.js b/editor/modes/visual-editor/block.js index 9992daea8e9fb5..236d473344eef4 100644 --- a/editor/modes/visual-editor/block.js +++ b/editor/modes/visual-editor/block.js @@ -290,7 +290,7 @@ class VisualEditorBlock extends Component { } render() { - const { block, multiSelectedBlockUids, order, mode } = this.props; + const { block, multiSelectedBlockUids, order, mode, hasMultiSelection } = this.props; const { name: blockName, isValid } = block; const blockType = getBlockType( blockName ); // translators: %s: Type of block (i.e. Text, Image etc) @@ -374,7 +374,7 @@ class VisualEditorBlock extends Component { { isValid && mode === 'visual' && ( 1, isHovered: isBlockHovered( state, ownProps.uid ), focus: getBlockFocus( state, ownProps.uid ), isSelecting: isMultiSelecting( state ), From 40c6efd8af0b99fa27ab75e8343f7d115c7d2bb3 Mon Sep 17 00:00:00 2001 From: morgan Date: Fri, 27 Oct 2017 11:26:54 +1000 Subject: [PATCH 2/4] focusing the next block after deleting --- editor/actions.js | 16 ++++++++++++++++ editor/effects.js | 22 +++++++++++++++++++++- editor/modes/visual-editor/index.js | 8 ++++---- 3 files changed, 41 insertions(+), 5 deletions(-) diff --git a/editor/actions.js b/editor/actions.js index 2f40c6a3820a46..30dd40ed5c0f31 100644 --- a/editor/actions.js +++ b/editor/actions.js @@ -284,6 +284,22 @@ export function removeBlocks( uids ) { }; } +/** + * Returns an action object used in signalling that the blocks + * corresponding to the specified UID set are to be removed, and then + * another block given focus. It differs from removeBlocks in that it + * calls focus on a block afterwards + * + * @param {String[]} uids Block UIDs + * @return {Object} Action object + */ +export function deleteBlocks( uids ) { + return { + type: 'DELETE_BLOCKS', + uids, + }; +} + /** * Returns an action object used in signalling that the block with the * specified UID is to be removed. diff --git a/editor/effects.js b/editor/effects.js index 404b5dcde53cd7..677add454e6a94 100644 --- a/editor/effects.js +++ b/editor/effects.js @@ -2,7 +2,7 @@ * External dependencies */ import { BEGIN, COMMIT, REVERT } from 'redux-optimist'; -import { get, uniqueId } from 'lodash'; +import { get, uniqueId, first, last } from 'lodash'; /** * WordPress dependencies @@ -22,6 +22,7 @@ import { replaceBlocks, createSuccessNotice, createErrorNotice, + removeBlocks, removeNotice, savePost, editPost, @@ -33,6 +34,8 @@ import { getDirtyMetaBoxes, getEditedPostContent, getPostEdits, + getPreviousBlock, + getNextBlock, isCurrentPostPublished, isEditedPostDirty, isEditedPostNew, @@ -184,6 +187,23 @@ export default { const message = action.error.message && action.error.code !== 'unknown_error' ? action.error.message : __( 'Trashing failed' ); store.dispatch( createErrorNotice( message, { id: TRASH_POST_NOTICE_ID } ) ); }, + DELETE_BLOCKS( action, store ) { + const { dispatch } = store; + const state = store.getState(); + + const blockUids = action.uids; + const firstBlock = first( blockUids ); + const lastBlock = last( blockUids ); + const priorBlock = getPreviousBlock( state, firstBlock ); + const nextBlock = getNextBlock( state, lastBlock ); + + dispatch( removeBlocks( blockUids ) ); + + const nextFocus = nextBlock || priorBlock; + if ( nextFocus ) { + dispatch( focusBlock( nextFocus.uid ) ); + } + }, MERGE_BLOCKS( action, store ) { const { dispatch } = store; const [ blockA, blockB ] = action.blocks; diff --git a/editor/modes/visual-editor/index.js b/editor/modes/visual-editor/index.js index fbafded4aceb84..f245c842e59afd 100644 --- a/editor/modes/visual-editor/index.js +++ b/editor/modes/visual-editor/index.js @@ -21,7 +21,7 @@ import PostTitle from '../../post-title'; import WritingFlow from '../../writing-flow'; import TableOfContents from '../../table-of-contents'; import { getBlockUids, getMultiSelectedBlockUids } from '../../selectors'; -import { clearSelectedBlock, multiSelect, redo, undo, removeBlocks } from '../../actions'; +import { clearSelectedBlock, multiSelect, redo, undo, deleteBlocks } from '../../actions'; class VisualEditor extends Component { constructor() { @@ -66,10 +66,10 @@ class VisualEditor extends Component { } deleteSelectedBlocks( event ) { - const { multiSelectedBlockUids, onRemove } = this.props; + const { multiSelectedBlockUids, onDelete } = this.props; if ( multiSelectedBlockUids.length ) { event.preventDefault(); - onRemove( multiSelectedBlockUids ); + onDelete( multiSelectedBlockUids ); } } @@ -116,6 +116,6 @@ export default connect( onMultiSelect: multiSelect, onRedo: redo, onUndo: undo, - onRemove: removeBlocks, + onDelete: deleteBlocks, } )( VisualEditor ); From 96beac8156118e3b62ebb16c7ea4c713a90b068e Mon Sep 17 00:00:00 2001 From: morgan Date: Fri, 27 Oct 2017 11:30:34 +1000 Subject: [PATCH 3/4] deleting and focusing blocks on BlockDelete button --- editor/block-settings-menu/block-delete-button.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/editor/block-settings-menu/block-delete-button.js b/editor/block-settings-menu/block-delete-button.js index 0b299d7a2d5462..b91c995215e407 100644 --- a/editor/block-settings-menu/block-delete-button.js +++ b/editor/block-settings-menu/block-delete-button.js @@ -13,7 +13,7 @@ import { IconButton } from '@wordpress/components'; /** * Internal dependencies */ -import { removeBlocks } from '../actions'; +import { deleteBlocks } from '../actions'; export function BlockDeleteButton( { onDelete, onClick = noop, small = false } ) { const label = __( 'Delete' ); @@ -34,7 +34,7 @@ export default connect( undefined, ( dispatch, ownProps ) => ( { onDelete() { - dispatch( removeBlocks( ownProps.uids ) ); + dispatch( deleteBlocks( ownProps.uids ) ); }, } ) )( BlockDeleteButton ); From cc43e6662cb4f2852cc07d0597dc254fac02c0e8 Mon Sep 17 00:00:00 2001 From: morgan Date: Fri, 27 Oct 2017 12:01:44 +1000 Subject: [PATCH 4/4] adding tests for DELETE_BLOCKS --- editor/effects.js | 8 +++-- editor/test/effects.js | 73 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 2 deletions(-) diff --git a/editor/effects.js b/editor/effects.js index 677add454e6a94..68af47f54a9d90 100644 --- a/editor/effects.js +++ b/editor/effects.js @@ -189,9 +189,13 @@ export default { }, DELETE_BLOCKS( action, store ) { const { dispatch } = store; - const state = store.getState(); - const blockUids = action.uids; + + if ( blockUids.length === 0 ) { + return; + } + + const state = store.getState(); const firstBlock = first( blockUids ); const lastBlock = last( blockUids ); const priorBlock = getPreviousBlock( state, firstBlock ); diff --git a/editor/test/effects.js b/editor/test/effects.js index fbc67a387fe36a..03f4bf390027cb 100644 --- a/editor/test/effects.js +++ b/editor/test/effects.js @@ -15,7 +15,9 @@ import { resetPost, setupNewPost, mergeBlocks, + deleteBlocks, focusBlock, + removeBlocks, replaceBlocks, editPost, savePost, @@ -31,6 +33,77 @@ describe( 'effects', () => { beforeEach( () => jest.resetAllMocks() ); + describe( '.DELETE_BLOCKS', () => { + const handler = effects.DELETE_BLOCKS; + + it( 'No blocks to be deleted', () => { + selectors.getNextBlock.mockReturnValue( null ); + selectors.getPreviousBlock.mockReturnValue( null ); + + const dispatch = jest.fn(); + handler( deleteBlocks( [ ] ), { dispatch, getState: () => 'state' } ); + + expect( selectors.getPreviousBlock ).toHaveBeenCalledTimes( 0 ); + expect( selectors.getNextBlock ).toHaveBeenCalledTimes( 0 ); + expect( dispatch ).toHaveBeenCalledTimes( 0 ); + } ); + + it( 'All blocks to be deleted', () => { + selectors.getNextBlock.mockReturnValue( null ); + selectors.getPreviousBlock.mockReturnValue( null ); + + const dispatch = jest.fn(); + handler( deleteBlocks( [ 'one', 'two', 'three' ] ), { dispatch, getState: () => 'state' } ); + + expect( selectors.getPreviousBlock ).toHaveBeenCalledWith( 'state', 'one' ); + expect( selectors.getNextBlock ).toHaveBeenCalledWith( 'state', 'three' ); + expect( dispatch ).toHaveBeenCalledTimes( 1 ); + expect( dispatch ).toHaveBeenCalledWith( removeBlocks( [ 'one', 'two', 'three' ] ) ); + } ); + + it( 'All blocks to be deleted but first', () => { + selectors.getNextBlock.mockReturnValue( { uid: 'first' } ); + selectors.getPreviousBlock.mockReturnValue( null ); + + const dispatch = jest.fn(); + handler( deleteBlocks( [ 'one', 'two', 'three' ] ), { dispatch, getState: () => 'state' } ); + + expect( selectors.getPreviousBlock ).toHaveBeenCalledWith( 'state', 'one' ); + expect( selectors.getNextBlock ).toHaveBeenCalledWith( 'state', 'three' ); + expect( dispatch ).toHaveBeenCalledTimes( 2 ); + expect( dispatch ).toHaveBeenCalledWith( removeBlocks( [ 'one', 'two', 'three' ] ) ); + expect( dispatch ).toHaveBeenCalledWith( focusBlock( 'first' ) ); + } ); + + it( 'All blocks to be deleted but last', () => { + selectors.getNextBlock.mockReturnValue( null ); + selectors.getPreviousBlock.mockReturnValue( { uid: 'last' } ); + + const dispatch = jest.fn(); + handler( deleteBlocks( [ 'one', 'two', 'three' ] ), { dispatch, getState: () => 'state' } ); + + expect( dispatch ).toHaveBeenCalledTimes( 2 ); + expect( selectors.getPreviousBlock ).toHaveBeenCalledWith( 'state', 'one' ); + expect( selectors.getNextBlock ).toHaveBeenCalledWith( 'state', 'three' ); + expect( dispatch ).toHaveBeenCalledWith( removeBlocks( [ 'one', 'two', 'three' ] ) ); + expect( dispatch ).toHaveBeenCalledWith( focusBlock( 'last' ) ); + } ); + + it( 'All blocks in the middle to be deleted', () => { + selectors.getNextBlock.mockReturnValue( { uid: 'first' } ); + selectors.getPreviousBlock.mockReturnValue( { uid: 'last' } ); + + const dispatch = jest.fn(); + handler( deleteBlocks( [ 'one', 'two', 'three' ] ), { dispatch, getState: () => 'state' } ); + + expect( dispatch ).toHaveBeenCalledTimes( 2 ); + expect( selectors.getPreviousBlock ).toHaveBeenCalledWith( 'state', 'one' ); + expect( selectors.getNextBlock ).toHaveBeenCalledWith( 'state', 'three' ); + expect( dispatch ).toHaveBeenCalledWith( removeBlocks( [ 'one', 'two', 'three' ] ) ); + expect( dispatch ).toHaveBeenCalledWith( focusBlock( 'first' ) ); + } ); + } ); + describe( '.MERGE_BLOCKS', () => { const handler = effects.MERGE_BLOCKS;