From 0bd4d0d4582184acd2cac01a2b0109af6f7e9ace Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Thu, 11 May 2017 11:50:27 +0100 Subject: [PATCH 1/2] Test: Move the merge block handler to the actions file and unit test it --- editor/actions.js | 55 +++++++++ editor/block-switcher/index.js | 10 +- editor/modes/visual-editor/block.js | 56 ++-------- editor/test/actions.js | 168 ++++++++++++++++++++++++++++ 4 files changed, 235 insertions(+), 54 deletions(-) create mode 100644 editor/test/actions.js diff --git a/editor/actions.js b/editor/actions.js index 978780b9f7c7f..85024762b0138 100644 --- a/editor/actions.js +++ b/editor/actions.js @@ -4,6 +4,22 @@ import { get } from 'lodash'; import { parse, stringify } from 'querystring'; +export function focusBlock( uid, config ) { + return { + type: 'UPDATE_FOCUS', + uid, + config, + }; +} + +export function replaceBlocks( uids, blocks ) { + return { + type: 'REPLACE_BLOCKS', + uids, + blocks, + }; +} + export function savePost( dispatch, postId, edits ) { const toSend = postId ? { id: postId, ...edits } : edits; const isNew = ! postId; @@ -41,3 +57,42 @@ export function savePost( dispatch, postId, edits ) { } ); } ); } + +export function mergeBlocks( dispatch, blockA, blockB ) { + const blockASettings = wp.blocks.getBlockSettings( blockA.blockType ); + + // Only focus the previous block if it's not mergeable + if ( ! blockASettings.merge ) { + dispatch( focusBlock( blockA.uid ) ); + return; + } + + // We can only merge blocks with similar types + // thus, we transform the block to merge first + const blocksWithTheSameType = blockA.blockType === blockB.blockType + ? [ blockB ] + : wp.blocks.switchToBlockType( blockB, blockA.blockType ); + + // If the block types can not match, do nothing + if ( ! blocksWithTheSameType || ! blocksWithTheSameType.length ) { + return; + } + + // Calling the merge to update the attributes and remove the block to be merged + const updatedAttributes = blockASettings.merge( blockA.attributes, blocksWithTheSameType[ 0 ].attributes ); + + dispatch( focusBlock( blockA.uid, { offset: -1 } ) ); + dispatch( replaceBlocks( + [ blockA.uid, blockB.uid ], + [ + { + ...blockA, + attributes: { + ...blockA.attributes, + ...updatedAttributes, + }, + }, + ...blocksWithTheSameType.slice( 1 ), + ] + ) ); +} diff --git a/editor/block-switcher/index.js b/editor/block-switcher/index.js index 94b1f558b4216..9f2ad65352b91 100644 --- a/editor/block-switcher/index.js +++ b/editor/block-switcher/index.js @@ -15,6 +15,7 @@ import Dashicon from 'components/dashicon'; * Internal dependencies */ import './style.scss'; +import { replaceBlocks } from '../actions'; import { getBlock } from '../selectors'; class BlockSwitcher extends wp.element.Component { @@ -111,11 +112,10 @@ export default connect( } ), ( dispatch, ownProps ) => ( { onTransform( block, blockType ) { - dispatch( { - type: 'REPLACE_BLOCKS', - uids: [ ownProps.uid ], - blocks: wp.blocks.switchToBlockType( block, blockType ), - } ); + dispatch( replaceBlocks( + [ ownProps.uid ], + wp.blocks.switchToBlockType( block, blockType ) + ) ); }, } ) )( clickOutside( BlockSwitcher ) ); diff --git a/editor/modes/visual-editor/block.js b/editor/modes/visual-editor/block.js index 9b97a37a172a9..b5d4eb2f86396 100644 --- a/editor/modes/visual-editor/block.js +++ b/editor/modes/visual-editor/block.js @@ -16,6 +16,7 @@ import Toolbar from 'components/toolbar'; */ import BlockMover from '../../block-mover'; import BlockSwitcher from '../../block-switcher'; +import { focusBlock, mergeBlocks } from '../../actions'; import { getPreviousBlock, getBlock, @@ -101,49 +102,14 @@ class VisualEditorBlock extends wp.element.Component { } mergeWithPrevious() { - const { block, previousBlock, onFocus, replaceBlocks } = this.props; + const { block, previousBlock, onMerge } = this.props; // Do nothing when it's the first block if ( ! previousBlock ) { return; } - const previousBlockSettings = wp.blocks.getBlockSettings( previousBlock.blockType ); - - // Do nothing if the previous block is not mergeable - if ( ! previousBlockSettings.merge ) { - onFocus( previousBlock.uid ); - return; - } - - // We can only merge blocks with similar types - // thus, we transform the block to merge first - const blocksWithTheSameType = previousBlock.blockType === block.blockType - ? [ block ] - : wp.blocks.switchToBlockType( block, previousBlock.blockType ); - - // If the block types can not match, do nothing - if ( ! blocksWithTheSameType || ! blocksWithTheSameType.length ) { - return; - } - - // Calling the merge to update the attributes and remove the block to be merged - const updatedAttributes = previousBlockSettings.merge( previousBlock.attributes, blocksWithTheSameType[ 0 ].attributes ); - - onFocus( previousBlock.uid, { offset: -1 } ); - replaceBlocks( - [ previousBlock.uid, block.uid ], - [ - { - ...previousBlock, - attributes: { - ...previousBlock.attributes, - ...updatedAttributes, - }, - }, - ...blocksWithTheSameType.slice( 1 ), - ] - ); + onMerge( previousBlock, block ); } componentDidUpdate( prevProps ) { @@ -306,12 +272,8 @@ export default connect( } ); }, - onFocus( uid, config ) { - dispatch( { - type: 'UPDATE_FOCUS', - uid, - config, - } ); + onFocus( ...args ) { + dispatch( focusBlock( ...args ) ); }, onRemove( uid ) { @@ -321,12 +283,8 @@ export default connect( } ); }, - replaceBlocks( uids, blocks ) { - dispatch( { - type: 'REPLACE_BLOCKS', - uids, - blocks, - } ); + onMerge( ...args ) { + mergeBlocks( dispatch, ...args ); }, } ) )( VisualEditorBlock ); diff --git a/editor/test/actions.js b/editor/test/actions.js new file mode 100644 index 0000000000000..5d12bf6cdf8c9 --- /dev/null +++ b/editor/test/actions.js @@ -0,0 +1,168 @@ +/** + * External dependencies + */ +import { expect } from 'chai'; +import sinon from 'sinon'; + +/** + * WordPress dependencies + */ +import { getBlocks, unregisterBlock, registerBlock, createBlock } from 'blocks'; + +/** + * Internal dependencies + */ +import { focusBlock, replaceBlocks, mergeBlocks } from '../actions'; + +describe( 'actions', () => { + describe( 'focusBlock', () => { + it( 'should return the UPDATE_FOCUS action', () => { + const focusConfig = { + editable: 'cite', + }; + + expect( focusBlock( 'chicken', focusConfig ) ).to.eql( { + type: 'UPDATE_FOCUS', + uid: 'chicken', + config: focusConfig, + } ); + } ); + } ); + + describe( 'replaceBlocks', () => { + it( 'should return the REPLACE_BLOCKS action', () => { + const blocks = [ { + uid: 'ribs', + } ]; + + expect( replaceBlocks( [ 'chicken' ], blocks ) ).to.eql( { + type: 'REPLACE_BLOCKS', + uids: [ 'chicken' ], + blocks, + } ); + } ); + } ); + + describe( 'mergeBlocks', () => { + afterEach( () => { + getBlocks().forEach( ( block ) => { + unregisterBlock( block.slug ); + } ); + } ); + + it( 'should only focus the blockA if the blockA has no merge function', () => { + registerBlock( 'core/test-block', {} ); + const blockA = { + uid: 'chicken', + blockType: 'core/test-block', + }; + const blockB = { + uid: 'ribs', + blockType: 'core/test-block', + }; + const dispatch = sinon.spy(); + mergeBlocks( dispatch, blockA, blockB ); + + expect( dispatch ).to.have.been.calledOnce(); + expect( dispatch ).to.have.been.calledWith( focusBlock( 'chicken' ) ); + } ); + + it( 'should merge the blocks if blocks of the same type', () => { + registerBlock( 'core/test-block', { + merge( attributes, attributesToMerge ) { + return { + content: attributes.content + ' ' + attributesToMerge.content, + }; + }, + } ); + const blockA = { + uid: 'chicken', + blockType: 'core/test-block', + attributes: { content: 'chicken' }, + }; + const blockB = { + uid: 'ribs', + blockType: 'core/test-block', + attributes: { content: 'ribs' }, + }; + const dispatch = sinon.spy(); + mergeBlocks( dispatch, blockA, blockB ); + + expect( dispatch ).to.have.been.calledTwice(); + expect( dispatch ).to.have.been.calledWith( focusBlock( 'chicken', { offset: -1 } ) ); + expect( dispatch ).to.have.been.calledWith( replaceBlocks( [ 'chicken', 'ribs' ], [ { + uid: 'chicken', + blockType: 'core/test-block', + attributes: { content: 'chicken ribs' }, + } ] ) ); + } ); + + it( 'should not merge the blocks have different types without transformation', () => { + registerBlock( 'core/test-block', { + merge( attributes, attributesToMerge ) { + return { + content: attributes.content + ' ' + attributesToMerge.content, + }; + }, + } ); + registerBlock( 'core/test-block-2', {} ); + const blockA = { + uid: 'chicken', + blockType: 'core/test-block', + attributes: { content: 'chicken' }, + }; + const blockB = { + uid: 'ribs', + blockType: 'core/test-block2', + attributes: { content: 'ribs' }, + }; + const dispatch = sinon.spy(); + mergeBlocks( dispatch, blockA, blockB ); + + expect( dispatch ).to.have.not.been.called(); + } ); + + it( 'should transform and merge the blocks', () => { + registerBlock( 'core/test-block', { + merge( attributes, attributesToMerge ) { + return { + content: attributes.content + ' ' + attributesToMerge.content, + }; + }, + } ); + registerBlock( 'core/test-block-2', { + transforms: { + to: [ { + type: 'blocks', + blocks: [ 'core/test-block' ], + transform: ( { content2 } ) => { + return createBlock( 'core/test-block', { + content: content2, + } ); + }, + } ], + }, + } ); + const blockA = { + uid: 'chicken', + blockType: 'core/test-block', + attributes: { content: 'chicken' }, + }; + const blockB = { + uid: 'ribs', + blockType: 'core/test-block-2', + attributes: { content2: 'ribs' }, + }; + const dispatch = sinon.spy(); + mergeBlocks( dispatch, blockA, blockB ); + + expect( dispatch ).to.have.been.calledTwice(); + expect( dispatch ).to.have.been.calledWith( focusBlock( 'chicken', { offset: -1 } ) ); + expect( dispatch ).to.have.been.calledWith( replaceBlocks( [ 'chicken', 'ribs' ], [ { + uid: 'chicken', + blockType: 'core/test-block', + attributes: { content: 'chicken ribs' }, + } ] ) ); + } ); + } ); +} ); From f12e179e24e441fabffea5982467409c4464de03 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Fri, 12 May 2017 08:46:21 +0100 Subject: [PATCH 2/2] Blocks: Handle forward backspace --- blocks/editable/index.js | 29 +++++++++++++++++++++++-- blocks/library/heading/index.js | 4 ++-- blocks/library/quote/index.js | 4 ++-- blocks/library/text/index.js | 4 ++-- editor/modes/visual-editor/block.js | 21 ++++++++++++------ editor/selectors.js | 5 +++++ editor/test/selectors.js | 33 +++++++++++++++++++++++++++++ 7 files changed, 86 insertions(+), 14 deletions(-) diff --git a/blocks/editable/index.js b/blocks/editable/index.js index ec0ae2fb5c2b6..9c0ac6afc12d4 100644 --- a/blocks/editable/index.js +++ b/blocks/editable/index.js @@ -20,6 +20,7 @@ import FormatToolbar from './format-toolbar'; import TinyMCE from './tinymce'; const KEYCODE_BACKSPACE = 8; +const KEYCODE_DELETE = 46; const alignmentMap = { alignleft: 'left', @@ -177,10 +178,34 @@ export default class Editable extends wp.element.Component { return true; } + isEndOfEditor() { + const range = this.editor.selection.getRng(); + if ( range.endOffset !== range.endContainer.textContent.length || ! range.collapsed ) { + return false; + } + const start = range.endContainer; + const body = this.editor.getBody(); + let element = start; + while ( element !== body ) { + const child = element; + element = element.parentNode; + if ( element.lastChild !== child ) { + return false; + } + } + return true; + } + onKeyDown( event ) { - if ( this.props.onMerge && event.keyCode === KEYCODE_BACKSPACE && this.isStartOfEditor() ) { + if ( + this.props.onMerge && ( + ( event.keyCode === KEYCODE_BACKSPACE && this.isStartOfEditor() ) || + ( event.keyCode === KEYCODE_DELETE && this.isEndOfEditor() ) + ) + ) { + const forward = event.keyCode === KEYCODE_DELETE; this.onChange(); - this.props.onMerge( this.editor.getContent() ); + this.props.onMerge( forward ); event.preventDefault(); event.stopImmediatePropagation(); } diff --git a/blocks/library/heading/index.js b/blocks/library/heading/index.js index a8181d868ab43..6f5ae5cfd1f2d 100644 --- a/blocks/library/heading/index.js +++ b/blocks/library/heading/index.js @@ -79,7 +79,7 @@ registerBlock( 'core/heading', { }; }, - edit( { attributes, setAttributes, focus, setFocus, mergeWithPrevious } ) { + edit( { attributes, setAttributes, focus, setFocus, mergeBlocks } ) { const { content, nodeName = 'H2' } = attributes; return ( @@ -89,7 +89,7 @@ registerBlock( 'core/heading', { focus={ focus } onFocus={ setFocus } onChange={ ( value ) => setAttributes( { content: value } ) } - onMerge={ mergeWithPrevious } + onMerge={ mergeBlocks } inline /> ); diff --git a/blocks/library/quote/index.js b/blocks/library/quote/index.js index 5105caa0164c4..f8f84d70ec513 100644 --- a/blocks/library/quote/index.js +++ b/blocks/library/quote/index.js @@ -82,7 +82,7 @@ registerBlock( 'core/quote', { ], }, - edit( { attributes, setAttributes, focus, setFocus, mergeWithPrevious } ) { + edit( { attributes, setAttributes, focus, setFocus, mergeBlocks } ) { const { value, citation, style = 1 } = attributes; const focusedEditable = focus ? focus.editable || 'value' : null; @@ -97,7 +97,7 @@ registerBlock( 'core/quote', { } focus={ focusedEditable === 'value' ? focus : null } onFocus={ () => setFocus( { editable: 'value' } ) } - onMerge={ mergeWithPrevious } + onMerge={ mergeBlocks } showAlignments /> { ( ( citation && citation.length > 0 ) || !! focus ) && ( diff --git a/blocks/library/text/index.js b/blocks/library/text/index.js index 870d260cefe7e..af23cb5df122f 100644 --- a/blocks/library/text/index.js +++ b/blocks/library/text/index.js @@ -27,7 +27,7 @@ registerBlock( 'core/text', { }; }, - edit( { attributes, setAttributes, insertBlockAfter, focus, setFocus, mergeWithPrevious } ) { + edit( { attributes, setAttributes, insertBlockAfter, focus, setFocus, mergeBlocks } ) { const { content } = attributes; return ( @@ -46,7 +46,7 @@ registerBlock( 'core/text', { content: after, } ) ); } } - onMerge={ mergeWithPrevious } + onMerge={ mergeBlocks } showAlignments /> ); diff --git a/editor/modes/visual-editor/block.js b/editor/modes/visual-editor/block.js index b5d4eb2f86396..0decf7a12973e 100644 --- a/editor/modes/visual-editor/block.js +++ b/editor/modes/visual-editor/block.js @@ -19,6 +19,7 @@ import BlockSwitcher from '../../block-switcher'; import { focusBlock, mergeBlocks } from '../../actions'; import { getPreviousBlock, + getNextBlock, getBlock, getBlockFocus, getBlockOrder, @@ -36,7 +37,7 @@ class VisualEditorBlock extends wp.element.Component { this.maybeHover = this.maybeHover.bind( this ); this.maybeStartTyping = this.maybeStartTyping.bind( this ); this.removeOnBackspace = this.removeOnBackspace.bind( this ); - this.mergeWithPrevious = this.mergeWithPrevious.bind( this ); + this.mergeBlocks = this.mergeBlocks.bind( this ); this.previousOffset = null; } @@ -101,15 +102,22 @@ class VisualEditorBlock extends wp.element.Component { } } - mergeWithPrevious() { - const { block, previousBlock, onMerge } = this.props; + mergeBlocks( forward = false ) { + const { block, previousBlock, nextBlock, onMerge } = this.props; // Do nothing when it's the first block - if ( ! previousBlock ) { + if ( + ( ! forward && ! previousBlock ) || + ( forward && ! nextBlock ) + ) { return; } - onMerge( previousBlock, block ); + if ( forward ) { + onMerge( block, nextBlock ); + } else { + onMerge( previousBlock, block ); + } } componentDidUpdate( prevProps ) { @@ -200,7 +208,7 @@ class VisualEditorBlock extends wp.element.Component { setAttributes={ this.setAttributes } insertBlockAfter={ onInsertAfter } setFocus={ partial( onFocus, block.uid ) } - mergeWithPrevious={ this.mergeWithPrevious } + mergeBlocks={ this.mergeBlocks } /> @@ -213,6 +221,7 @@ export default connect( ( state, ownProps ) => { return { previousBlock: getPreviousBlock( state, ownProps.uid ), + nextBlock: getNextBlock( state, ownProps.uid ), block: getBlock( state, ownProps.uid ), isSelected: isBlockSelected( state, ownProps.uid ), isHovered: isBlockHovered( state, ownProps.uid ), diff --git a/editor/selectors.js b/editor/selectors.js index 7932d5b4f08b6..537e180a01bd9 100644 --- a/editor/selectors.js +++ b/editor/selectors.js @@ -68,6 +68,11 @@ export function getPreviousBlock( state, uid ) { return state.editor.blocksByUid[ state.editor.blockOrder[ order - 1 ] ] || null; } +export function getNextBlock( state, uid ) { + const order = getBlockOrder( state, uid ); + return state.editor.blocksByUid[ state.editor.blockOrder[ order + 1 ] ] || null; +} + export function isBlockSelected( state, uid ) { return state.selectedBlock.uid === uid; } diff --git a/editor/test/selectors.js b/editor/test/selectors.js index fe31e4108e1b6..6a13392185859 100644 --- a/editor/test/selectors.js +++ b/editor/test/selectors.js @@ -22,6 +22,7 @@ import { isFirstBlock, isLastBlock, getPreviousBlock, + getNextBlock, isBlockSelected, isBlockHovered, getBlockFocus, @@ -322,6 +323,38 @@ describe( 'selectors', () => { } ); } ); + describe( 'getNextBlock', () => { + it( 'should return the following block', () => { + const state = { + editor: { + blocksByUid: { + 23: { uid: 23, blockType: 'core/heading' }, + 123: { uid: 123, blockType: 'core/text' }, + }, + blockOrder: [ 123, 23 ], + }, + }; + + expect( getNextBlock( state, 123 ) ).to.eql( + { uid: 23, blockType: 'core/heading' }, + ); + } ); + + it( 'should return null for the last block', () => { + const state = { + editor: { + blocksByUid: { + 23: { uid: 23, blockType: 'core/heading' }, + 123: { uid: 123, blockType: 'core/text' }, + }, + blockOrder: [ 123, 23 ], + }, + }; + + expect( getNextBlock( state, 23 ) ).to.be.null(); + } ); + } ); + describe( 'isBlockSelected', () => { it( 'should return true if the block is selected', () => { const state = {