From 5e4945db42afe1a4b72934fd03a220ea27fcac23 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Thu, 20 Apr 2017 11:55:23 +0100 Subject: [PATCH 1/9] Blocks: Handle the backspace at the beginning of blocks - Adds a `merge` function to the block API --- blocks/components/editable/index.js | 30 +++++++++++- blocks/library/heading/index.js | 61 +++++++++++++----------- blocks/library/text/index.js | 9 +++- editor/modes/visual-editor/block.js | 73 ++++++++++++++++++++++++----- editor/state.js | 8 +++- editor/test/state.js | 28 +++++++++++ element/index.js | 18 +++++++ 7 files changed, 186 insertions(+), 41 deletions(-) diff --git a/blocks/components/editable/index.js b/blocks/components/editable/index.js index 072615f44564f..6b407b835f246 100644 --- a/blocks/components/editable/index.js +++ b/blocks/components/editable/index.js @@ -15,6 +15,7 @@ import './style.scss'; // as we're doing here; instead, we should consider a common components path. import Toolbar from '../../../editor/components/toolbar'; +const KEYCODE_BACKSPACE = 8; const htmlToReactParser = new HtmlToReactParser(); const formatMap = { strong: 'bold', @@ -51,7 +52,7 @@ export default class Editable extends wp.element.Component { this.bindEditorNode = this.bindEditorNode.bind( this ); this.onFocus = this.onFocus.bind( this ); this.onNodeChange = this.onNodeChange.bind( this ); - + this.onKeyDown = this.onKeyDown.bind( this ); this.state = { formats: {} }; @@ -85,6 +86,7 @@ export default class Editable extends wp.element.Component { editor.on( 'NewBlock', this.onNewBlock ); editor.on( 'focusin', this.onFocus ); editor.on( 'nodechange', this.onNodeChange ); + editor.on( 'keydown', this.onKeyDown ); } onInit() { @@ -110,6 +112,32 @@ export default class Editable extends wp.element.Component { this.props.onChange( this.getContent() ); } + isStartOfEditor() { + const range = this.editor.selection.getRng(); + if ( range.startOffset !== 0 || ! range.collapsed ) { + return false; + } + const start = range.startContainer; + const body = this.editor.getBody(); + let element = start; + do { + const child = element; + element = element.parentNode; + if ( element.childNodes[ 0 ] !== child ) { + return false; + } + } while ( element !== body ); + return true; + } + + onKeyDown( event ) { + if ( this.props.onMerge && event.keyCode === KEYCODE_BACKSPACE && this.isStartOfEditor() ) { + event.preventDefault(); + this.onChange(); + this.props.onMerge( this.editor.getContent() ); + } + } + onNewBlock() { if ( this.props.tagName || ! this.props.onSplit ) { return; diff --git a/blocks/library/heading/index.js b/blocks/library/heading/index.js index b7b804cd093b4..fc57f5c4998e3 100644 --- a/blocks/library/heading/index.js +++ b/blocks/library/heading/index.js @@ -31,32 +31,6 @@ registerBlock( 'core/heading', { } ) ) ], - edit( { attributes, setAttributes, focus, setFocus } ) { - const { content, nodeName = 'H2', align } = attributes; - - return ( - setAttributes( { content: value } ) } - style={ align ? { textAlign: align } : null } - /> - ); - }, - - save( { attributes } ) { - const { align, nodeName = 'H2', content } = attributes; - const Tag = nodeName.toLowerCase(); - - return ( - - { content } - - ); - }, - transforms: { from: [ { @@ -88,5 +62,38 @@ registerBlock( 'core/heading', { } } ] - } + }, + + merge( attributes, attributesToMerge ) { + return { + content: wp.element.concatValues( attributes.content, attributesToMerge.content ) + }; + }, + + edit( { attributes, setAttributes, focus, setFocus, mergeWithPrevious } ) { + const { content, nodeName = 'H2', align } = attributes; + + return ( + setAttributes( { content: value } ) } + style={ align ? { textAlign: align } : null } + onMerge={ mergeWithPrevious } + /> + ); + }, + + save( { attributes } ) { + const { align, nodeName = 'H2', content } = attributes; + const Tag = nodeName.toLowerCase(); + + return ( + + { content } + + ); + }, } ); diff --git a/blocks/library/text/index.js b/blocks/library/text/index.js index 75c60a6c9d79a..36da78b5280c5 100644 --- a/blocks/library/text/index.js +++ b/blocks/library/text/index.js @@ -44,7 +44,13 @@ registerBlock( 'core/text', { } ], - edit( { attributes, setAttributes, insertBlockAfter, focus, setFocus } ) { + merge( attributes, attributesToMerge ) { + return { + content: wp.element.concatValues( attributes.content, attributesToMerge.content ) + }; + }, + + edit( { attributes, setAttributes, insertBlockAfter, focus, setFocus, mergeWithPrevious } ) { const { content =

, align } = attributes; return ( @@ -64,6 +70,7 @@ registerBlock( 'core/text', { content: after } ) ); } } + onMerge={ mergeWithPrevious } /> ); }, diff --git a/editor/modes/visual-editor/block.js b/editor/modes/visual-editor/block.js index f524930ee38fd..916cb4c831a95 100644 --- a/editor/modes/visual-editor/block.js +++ b/editor/modes/visual-editor/block.js @@ -19,6 +19,7 @@ class VisualEditorBlock extends wp.element.Component { this.setAttributes = this.setAttributes.bind( this ); this.maybeDeselect = this.maybeDeselect.bind( this ); this.maybeHover = this.maybeHover.bind( this ); + this.mergeWithPrevious = this.mergeWithPrevious.bind( this ); this.previousOffset = null; } @@ -38,7 +39,7 @@ class VisualEditorBlock extends wp.element.Component { setAttributes( attributes ) { const { block, onChange } = this.props; - onChange( { + onChange( block.uid, { attributes: { ...block.attributes, ...attributes @@ -61,6 +62,44 @@ class VisualEditorBlock extends wp.element.Component { } } + mergeWithPrevious() { + const { block, previousBlock, onRemove, onChange } = this.props; + + // Do nothing when it's the first block + if ( ! previousBlock ) { + return; + } + + const previousBlockSettings = wp.blocks.getBlockSettings( previousBlock.blockType ); + + // Remove the previous block if it's not mergeable + if ( ! previousBlockSettings.merge ) { + onRemove( previousBlock.uid ); + return; + } + + // We can only merge blocks with similar types + // thus, we transform the block to merge first + const blockWithTheSameType = previousBlock.blockType === block.blockType + ? block + : wp.blocks.switchToBlockType( block, previousBlock.blockType ); + + // If the block types can not match, do nothing + if ( ! blockWithTheSameType ) { + return; + } + + // Calling the merge to update the attributes and remove the block to be merged + const updatedAttributes = previousBlockSettings.merge( previousBlock.attributes, blockWithTheSameType.attributes ); + onChange( previousBlock.uid, { + attributes: { + ...previousBlock.attributes, + ...updatedAttributes + } + } ); + onRemove( block.uid ); + } + componentDidUpdate() { if ( this.previousOffset ) { window.scrollTo( @@ -137,6 +176,7 @@ class VisualEditorBlock extends wp.element.Component { setAttributes={ this.setAttributes } insertBlockAfter={ onInsertAfter } setFocus={ onFocus } + mergeWithPrevious={ this.mergeWithPrevious } /> ); @@ -145,19 +185,23 @@ class VisualEditorBlock extends wp.element.Component { } export default connect( - ( state, ownProps ) => ( { - order: state.blocks.order.indexOf( ownProps.uid ), - block: state.blocks.byUid[ ownProps.uid ], - isSelected: state.selectedBlock.uid === ownProps.uid, - isHovered: state.hoveredBlock === ownProps.uid, - focus: state.selectedBlock.uid === ownProps.uid ? state.selectedBlock.focus : null, - isTyping: state.selectedBlock.uid === ownProps.uid ? state.selectedBlock.typing : false, - } ), + ( state, ownProps ) => { + const order = state.blocks.order.indexOf( ownProps.uid ); + return { + previousBlock: order === 0 ? null : state.blocks.byUid[ state.blocks.order[ order - 1 ] ], + block: state.blocks.byUid[ ownProps.uid ], + isSelected: state.selectedBlock.uid === ownProps.uid, + isHovered: state.hoveredBlock === ownProps.uid, + focus: state.selectedBlock.uid === ownProps.uid ? state.selectedBlock.focus : null, + isTyping: state.selectedBlock.uid === ownProps.uid ? state.selectedBlock.typing : false, + order + }; + }, ( dispatch, ownProps ) => ( { - onChange( updates ) { + onChange( uid, updates ) { dispatch( { type: 'UPDATE_BLOCK', - uid: ownProps.uid, + uid, updates } ); }, @@ -210,6 +254,13 @@ export default connect( uid: ownProps.uid, config } ); + }, + + onRemove( uid ) { + dispatch( { + type: 'REMOVE_BLOCK', + uid + } ); } } ) )( VisualEditorBlock ); diff --git a/editor/state.js b/editor/state.js index f9e35785522ca..399738e6495a2 100644 --- a/editor/state.js +++ b/editor/state.js @@ -2,7 +2,7 @@ * External dependencies */ import { combineReducers, createStore } from 'redux'; -import { keyBy, last } from 'lodash'; +import { keyBy, last, omit, without } from 'lodash'; /** * Internal dependencies @@ -43,6 +43,9 @@ export const blocks = combineUndoableReducers( { ...state, [ action.uid ]: action.block }; + + case 'REMOVE_BLOCK': + return omit( state, action.uid ); } return state; @@ -87,6 +90,9 @@ export const blocks = combineUndoableReducers( { action.uid, ...state.slice( index + 2 ) ]; + + case 'REMOVE_BLOCK': + return without( state, action.uid ); } return state; diff --git a/editor/test/state.js b/editor/test/state.js index 5c7e89c25ef1f..c00a153e945c6 100644 --- a/editor/test/state.js +++ b/editor/test/state.js @@ -195,6 +195,34 @@ describe( 'state', () => { expect( state.order ).to.equal( original.order ); } ); + + it( 'should remove the block', () => { + const original = blocks( undefined, { + type: 'REPLACE_BLOCKS', + blockNodes: [ { + uid: 'chicken', + blockType: 'core/test-block', + attributes: {} + }, { + uid: 'ribs', + blockType: 'core/test-block', + attributes: {} + } ] + } ); + const state = blocks( original, { + type: 'REMOVE_BLOCK', + uid: 'chicken' + } ); + + expect( state.order ).to.eql( [ 'ribs' ] ); + expect( state.byUid ).to.eql( { + ribs: { + uid: 'ribs', + blockType: 'core/test-block', + attributes: {} + } + } ); + } ); } ); describe( 'hoveredBlock()', () => { diff --git a/element/index.js b/element/index.js index 846c51b98be24..d437f5c3c1661 100644 --- a/element/index.js +++ b/element/index.js @@ -4,6 +4,7 @@ import { createElement, Component, cloneElement, Children } from 'react'; import { render } from 'react-dom'; import { renderToStaticMarkup } from 'react-dom/server'; +import { isString } from 'lodash'; /** * Returns a new element of given type. Type can be either a string tag name or @@ -65,3 +66,20 @@ export function renderToString( element ) { return renderToStaticMarkup( element ); } + +export function concatValues( value1, value2 ) { + const toArray = value => Array.isArray( value ) ? Children.toArray( value ) : [ value ]; + + return toArray( value1 ) + .concat( toArray( value2 ) ) + .map( ( elt, index ) => { + if ( isString( elt ) ) { + return elt; + } + + return { + ...elt, + key: index + }; + } ); +} From 71061289c56a27262d38ff2ee490b9d69f3b6641 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Mon, 24 Apr 2017 15:34:45 +0100 Subject: [PATCH 2/9] replacing `childNodes[ 0 ]` with `firstChild` --- blocks/components/editable/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/blocks/components/editable/index.js b/blocks/components/editable/index.js index 6b407b835f246..ee7591c0d4d41 100644 --- a/blocks/components/editable/index.js +++ b/blocks/components/editable/index.js @@ -123,7 +123,7 @@ export default class Editable extends wp.element.Component { do { const child = element; element = element.parentNode; - if ( element.childNodes[ 0 ] !== child ) { + if ( element.firstChild !== child ) { return false; } } while ( element !== body ); From d5fc22a190499088010baab58228116228c6f25f Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Tue, 25 Apr 2017 09:42:34 +0100 Subject: [PATCH 3/9] Element: Rename concatChildren helper and add unit tests --- blocks/library/heading/index.js | 2 +- blocks/library/text/index.js | 2 +- element/index.js | 41 ++++++++++++++++++++++----------- element/test/index.js | 22 +++++++++++++++++- 4 files changed, 51 insertions(+), 16 deletions(-) diff --git a/blocks/library/heading/index.js b/blocks/library/heading/index.js index fc57f5c4998e3..8c2c5d4e546c6 100644 --- a/blocks/library/heading/index.js +++ b/blocks/library/heading/index.js @@ -66,7 +66,7 @@ registerBlock( 'core/heading', { merge( attributes, attributesToMerge ) { return { - content: wp.element.concatValues( attributes.content, attributesToMerge.content ) + content: wp.element.concatChildren( attributes.content, attributesToMerge.content ) }; }, diff --git a/blocks/library/text/index.js b/blocks/library/text/index.js index 36da78b5280c5..9d82d80b19419 100644 --- a/blocks/library/text/index.js +++ b/blocks/library/text/index.js @@ -46,7 +46,7 @@ registerBlock( 'core/text', { merge( attributes, attributesToMerge ) { return { - content: wp.element.concatValues( attributes.content, attributesToMerge.content ) + content: wp.element.concatChildren( attributes.content, attributesToMerge.content ) }; }, diff --git a/element/index.js b/element/index.js index d437f5c3c1661..fa8ff7cccab63 100644 --- a/element/index.js +++ b/element/index.js @@ -67,19 +67,34 @@ export function renderToString( element ) { return renderToStaticMarkup( element ); } -export function concatValues( value1, value2 ) { - const toArray = value => Array.isArray( value ) ? Children.toArray( value ) : [ value ]; +/** + * Concat two React children objects + * + * @param {?Object} children1 First children value + * @param {?Object} children2 Second children value + * + * @return {Array} The concatenation + */ +export function concatChildren( children1, children2 ) { + const toArray = ( children ) => { + if ( ! children ) { + return []; + } + + return Array.isArray( children ) ? Children.toArray( children ) : [ children ]; + }; - return toArray( value1 ) - .concat( toArray( value2 ) ) - .map( ( elt, index ) => { - if ( isString( elt ) ) { - return elt; - } + return [ + ...toArray( children1 ), + ...toArray( children2 ) + ].map( ( elt, index ) => { + if ( isString( elt ) ) { + return elt; + } - return { - ...elt, - key: index - }; - } ); + return { + ...elt, + key: index + }; + } ); } diff --git a/element/test/index.js b/element/test/index.js index 333acd3ffb9ee..47d5c688ae59a 100644 --- a/element/test/index.js +++ b/element/test/index.js @@ -6,7 +6,7 @@ import { expect } from 'chai'; /** * Internal dependencies */ -import { createElement, renderToString } from '../'; +import { createElement, renderToString, concatChildren } from '../'; describe( 'element', () => { describe( 'renderToString', () => { @@ -35,4 +35,24 @@ describe( 'element', () => { ) ).to.equal( 'Courgette' ); } ); } ); + + describe( 'concatChildren', () => { + it( 'should return an empty array for undefined children', () => { + expect( concatChildren() ).to.eql( [] ); + } ); + + it( 'should concat the string arrays', () => { + expect( concatChildren( [ 'a' ], 'b' ) ).to.eql( [ 'a', 'b' ] ); + } ); + + it( 'should concat the object arrays and rewrite keys', () => { + const concat = concatChildren( + [ createElement( 'strong', null, 'Courgette' ) ], + createElement( 'strong', null, 'Concombre' ) + ); + expect( concat.length ).to.equal( 2 ); + expect( concat[ 0 ].key = 0 ); + expect( concat[ 1 ].key = 1 ); + } ); + } ); } ); From f0c34ba01a8a9ca2719cd2fa4175ac6277af9da0 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Tue, 25 Apr 2017 09:47:34 +0100 Subject: [PATCH 4/9] Style: avoid ternary and fallback using || --- editor/modes/visual-editor/block.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/editor/modes/visual-editor/block.js b/editor/modes/visual-editor/block.js index 916cb4c831a95..31789867c6b1b 100644 --- a/editor/modes/visual-editor/block.js +++ b/editor/modes/visual-editor/block.js @@ -188,7 +188,7 @@ export default connect( ( state, ownProps ) => { const order = state.blocks.order.indexOf( ownProps.uid ); return { - previousBlock: order === 0 ? null : state.blocks.byUid[ state.blocks.order[ order - 1 ] ], + previousBlock: state.blocks.byUid[ state.blocks.order[ order - 1 ] ] || null, block: state.blocks.byUid[ ownProps.uid ], isSelected: state.selectedBlock.uid === ownProps.uid, isHovered: state.hoveredBlock === ownProps.uid, From ae4f41a73a302556e3d616ae086b19cdd0c8b312 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Wed, 26 Apr 2017 09:15:04 +0100 Subject: [PATCH 5/9] Fix merging two consecutive empty headings --- blocks/components/editable/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/blocks/components/editable/index.js b/blocks/components/editable/index.js index ee7591c0d4d41..2a9b4e3da6cfb 100644 --- a/blocks/components/editable/index.js +++ b/blocks/components/editable/index.js @@ -120,13 +120,13 @@ export default class Editable extends wp.element.Component { const start = range.startContainer; const body = this.editor.getBody(); let element = start; - do { + while ( element !== body ) { const child = element; element = element.parentNode; if ( element.firstChild !== child ) { return false; } - } while ( element !== body ); + } return true; } From 15b70c1cf2c315f4e9db44beda2b9743c078587d Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Wed, 26 Apr 2017 09:29:31 +0100 Subject: [PATCH 6/9] Fix TinyMCE isCollapsed error when merging two blocks --- blocks/components/editable/index.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/blocks/components/editable/index.js b/blocks/components/editable/index.js index 2a9b4e3da6cfb..cf61d955724ef 100644 --- a/blocks/components/editable/index.js +++ b/blocks/components/editable/index.js @@ -132,9 +132,10 @@ export default class Editable extends wp.element.Component { onKeyDown( event ) { if ( this.props.onMerge && event.keyCode === KEYCODE_BACKSPACE && this.isStartOfEditor() ) { - event.preventDefault(); this.onChange(); - this.props.onMerge( this.editor.getContent() ); + + // Debouncing this call avoids TinyMCE error while handling `keydown` event + setTimeout( () => this.props.onMerge( this.editor.getContent() ) ); } } From 129ffd7e5879fb90c541c195083cbccf6b48ceb9 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 26 Apr 2017 09:29:46 -0400 Subject: [PATCH 7/9] Stop keydown event propagation if editor removed by merge --- blocks/components/editable/index.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/blocks/components/editable/index.js b/blocks/components/editable/index.js index cf61d955724ef..db9977dc63399 100644 --- a/blocks/components/editable/index.js +++ b/blocks/components/editable/index.js @@ -133,9 +133,13 @@ export default class Editable extends wp.element.Component { onKeyDown( event ) { if ( this.props.onMerge && event.keyCode === KEYCODE_BACKSPACE && this.isStartOfEditor() ) { this.onChange(); + this.props.onMerge( this.editor.getContent() ); - // Debouncing this call avoids TinyMCE error while handling `keydown` event - setTimeout( () => this.props.onMerge( this.editor.getContent() ) ); + // If merge causes editor to be removed, stop other callbacks from + // trying to handle the event + if ( this.editor.removed ) { + event.stopImmediatePropagation(); + } } } From 1b360dc952f95c67d4e6d83aa7e870465af6435e Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Thu, 27 Apr 2017 09:16:19 +0100 Subject: [PATCH 8/9] Update concatChildren to accomodate multiple children --- element/index.js | 41 +++++++++++++++-------------------------- 1 file changed, 15 insertions(+), 26 deletions(-) diff --git a/element/index.js b/element/index.js index fa8ff7cccab63..3608995e00370 100644 --- a/element/index.js +++ b/element/index.js @@ -4,7 +4,6 @@ import { createElement, Component, cloneElement, Children } from 'react'; import { render } from 'react-dom'; import { renderToStaticMarkup } from 'react-dom/server'; -import { isString } from 'lodash'; /** * Returns a new element of given type. Type can be either a string tag name or @@ -68,33 +67,23 @@ export function renderToString( element ) { } /** - * Concat two React children objects + * Concatenate two or more React children objects * - * @param {?Object} children1 First children value - * @param {?Object} children2 Second children value - * - * @return {Array} The concatenation + * @param {...?Object} childrens Set of children to concatenate + * @return {Array} The concatenated value */ -export function concatChildren( children1, children2 ) { - const toArray = ( children ) => { - if ( ! children ) { - return []; - } - - return Array.isArray( children ) ? Children.toArray( children ) : [ children ]; - }; +export function concatChildren( ...childrens ) { + return childrens.reduce( ( memo, children, i ) => { + Children.forEach( children, ( child, j ) => { + if ( 'string' !== typeof child ) { + child = cloneElement( child, { + key: [ i, j ].join() + } ); + } - return [ - ...toArray( children1 ), - ...toArray( children2 ) - ].map( ( elt, index ) => { - if ( isString( elt ) ) { - return elt; - } + memo.push( child ); + } ); - return { - ...elt, - key: index - }; - } ); + return memo; + }, [] ); } From a2c106575df55ebc2348725bfa3b10f9db6974c5 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Thu, 27 Apr 2017 09:31:49 +0100 Subject: [PATCH 9/9] ConcatChildren: Fix unit tests --- element/test/index.js | 8 ++++---- languages/gutenberg.pot | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/element/test/index.js b/element/test/index.js index 47d5c688ae59a..842ac6a0465e1 100644 --- a/element/test/index.js +++ b/element/test/index.js @@ -47,12 +47,12 @@ describe( 'element', () => { it( 'should concat the object arrays and rewrite keys', () => { const concat = concatChildren( - [ createElement( 'strong', null, 'Courgette' ) ], - createElement( 'strong', null, 'Concombre' ) + [ createElement( 'strong', {}, 'Courgette' ) ], + createElement( 'strong', {}, 'Concombre' ) ); expect( concat.length ).to.equal( 2 ); - expect( concat[ 0 ].key = 0 ); - expect( concat[ 1 ].key = 1 ); + expect( concat[ 0 ].key ).to.equal( '0,0' ); + expect( concat[ 1 ].key ).to.equal( '1,0' ); } ); } ); } ); diff --git a/languages/gutenberg.pot b/languages/gutenberg.pot index c600080cd9959..616d851037d34 100644 --- a/languages/gutenberg.pot +++ b/languages/gutenberg.pot @@ -3,15 +3,15 @@ msgstr "" "Content-Type: text/plain; charset=utf-8\n" "X-Generator: babel-plugin-wp-i18n\n" -#: blocks/components/editable/index.js:28 +#: blocks/components/editable/index.js:29 msgid "Bold" msgstr "" -#: blocks/components/editable/index.js:33 +#: blocks/components/editable/index.js:34 msgid "Italic" msgstr "" -#: blocks/components/editable/index.js:38 +#: blocks/components/editable/index.js:39 msgid "Strikethrough" msgstr ""