From 98f8cc9eb00d36e7e725bc8b3d0d4154e725f459 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 22 Jun 2018 11:03:07 -0400 Subject: [PATCH 1/4] Blocks: Avoid setAttributes on end-of-paragraph split --- core-blocks/paragraph/index.js | 63 ++++++++++++++++++++-------- editor/components/rich-text/index.js | 19 ++++++++- 2 files changed, 63 insertions(+), 19 deletions(-) diff --git a/core-blocks/paragraph/index.js b/core-blocks/paragraph/index.js index 7f24c38f90b68..1d2e511c872f2 100644 --- a/core-blocks/paragraph/index.js +++ b/core-blocks/paragraph/index.js @@ -78,10 +78,12 @@ const FONT_SIZES = [ class ParagraphBlock extends Component { constructor() { super( ...arguments ); + this.onReplace = this.onReplace.bind( this ); this.toggleDropCap = this.toggleDropCap.bind( this ); this.getFontSize = this.getFontSize.bind( this ); this.setFontSize = this.setFontSize.bind( this ); + this.splitBlock = this.splitBlock.bind( this ); } onReplace( blocks ) { @@ -137,11 +139,53 @@ class ParagraphBlock extends Component { } ); } + /** + * Split handler for RichText value, namely when content is pasted or the + * user presses the Enter key. + * + * @param {?Array} before Optional before value, to be used as content + * in place of what exists currently for the + * block. If undefined, the block is deleted. + * @param {?Array} after Optional after value, to be appended in a new + * paragraph block to the set of blocks passed + * as spread. + * @param {...WPBlock} blocks Optional blocks inserted between the before + * and after value blocks. + */ + splitBlock( before, after, ...blocks ) { + const { + attributes, + insertBlocksAfter, + setAttributes, + onReplace, + } = this.props; + + if ( after ) { + // After content should be appended to the set of spread blocks as + // a new paragraph block to insert after. + blocks.push( createBlock( name, { content: after } ) ); + } + + if ( blocks.length ) { + insertBlocksAfter( blocks ); + } + + const { content } = attributes; + if ( ! before ) { + // If before content is omitted, treat as intent to delete block. + onReplace( [] ); + } else if ( content !== before ) { + // Only update content if it has in-fact changed. In case that user + // has created a new paragraph at end of an existing one, the value + // of before will be strictly equal to the current content. + setAttributes( { content: before } ); + } + } + render() { const { attributes, setAttributes, - insertBlocksAfter, mergeBlocks, onReplace, className, @@ -230,22 +274,7 @@ class ParagraphBlock extends Component { content: nextContent, } ); } } - onSplit={ insertBlocksAfter ? - ( before, after, ...blocks ) => { - if ( after ) { - blocks.push( createBlock( name, { content: after } ) ); - } - - insertBlocksAfter( blocks ); - - if ( before ) { - setAttributes( { content: before } ); - } else { - onReplace( [] ); - } - } : - undefined - } + onSplit={ this.splitBlock } onMerge={ mergeBlocks } onReplace={ this.onReplace } onRemove={ () => onReplace( [] ) } diff --git a/editor/components/rich-text/index.js b/editor/components/rich-text/index.js index 3fcec36dcaba5..6f4054c760c34 100644 --- a/editor/components/rich-text/index.js +++ b/editor/components/rich-text/index.js @@ -573,12 +573,27 @@ export class RichText extends Component { * @param {Object} context The context for splitting. */ splitContent( blocks = [], context = {} ) { - if ( ! this.props.onSplit ) { + const { onSplit } = this.props; + if ( ! onSplit ) { return; } - const { dom } = this.editor; const rootNode = this.editor.getBody(); + + // In case split occurs at the trailing or leading edge of the field, + // shortcut with assumption that the before/after values respectively + // reflect the current value. This also provides an opportunity for the + // parent component to determine whether the before/after value has + // changed using a trivial strict equality operation. + if ( isHorizontalEdge( rootNode ) ) { + onSplit( this.props.value, [], ...blocks ); + return; + } else if ( isHorizontalEdge( rootNode, true ) ) { + onSplit( [], this.props.value, ...blocks ); + return; + } + + const { dom } = this.editor; const beforeRange = dom.createRng(); const afterRange = dom.createRng(); const selectionRange = this.editor.selection.getRng(); From cdbb0d8ef16a6c6740c665c386b2e9144106e2d1 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 29 Jun 2018 10:14:40 -0400 Subject: [PATCH 2/4] Blocks: Preserve respect of insertBlocksAfter undefined For locked blocks. See also: https://github.com/WordPress/gutenberg/issues/6587 --- core-blocks/paragraph/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core-blocks/paragraph/index.js b/core-blocks/paragraph/index.js index 1d2e511c872f2..2f42bad2aff13 100644 --- a/core-blocks/paragraph/index.js +++ b/core-blocks/paragraph/index.js @@ -166,7 +166,7 @@ class ParagraphBlock extends Component { blocks.push( createBlock( name, { content: after } ) ); } - if ( blocks.length ) { + if ( blocks.length && insertBlocksAfter ) { insertBlocksAfter( blocks ); } From 2ea9199a6e13ea6128ea8a8aa268df6f0b5b646a Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 29 Jun 2018 10:16:35 -0400 Subject: [PATCH 3/4] Blocks: Clarify after split insertion coment --- core-blocks/paragraph/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core-blocks/paragraph/index.js b/core-blocks/paragraph/index.js index 2f42bad2aff13..3d055b765d5d6 100644 --- a/core-blocks/paragraph/index.js +++ b/core-blocks/paragraph/index.js @@ -161,8 +161,8 @@ class ParagraphBlock extends Component { } = this.props; if ( after ) { - // After content should be appended to the set of spread blocks as - // a new paragraph block to insert after. + // Append "After" content as a new paragraph block to the end of + // any other blocks being inserted after the current paragraph. blocks.push( createBlock( name, { content: after } ) ); } From 7a8ec43e585848f643efdaa3add003cc5a54283d Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 29 Jun 2018 10:18:11 -0400 Subject: [PATCH 4/4] Rich Text: Test edge as empty after / before fragment Accommodates case where childNodes exist, but none which would qualify after filtering. Also preserves existing logic for detecting / handling paste context. --- editor/components/rich-text/index.js | 58 ++++++++++++++-------------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/editor/components/rich-text/index.js b/editor/components/rich-text/index.js index 6f4054c760c34..5d43e8be510ca 100644 --- a/editor/components/rich-text/index.js +++ b/editor/components/rich-text/index.js @@ -580,25 +580,13 @@ export class RichText extends Component { const rootNode = this.editor.getBody(); - // In case split occurs at the trailing or leading edge of the field, - // shortcut with assumption that the before/after values respectively - // reflect the current value. This also provides an opportunity for the - // parent component to determine whether the before/after value has - // changed using a trivial strict equality operation. - if ( isHorizontalEdge( rootNode ) ) { - onSplit( this.props.value, [], ...blocks ); - return; - } else if ( isHorizontalEdge( rootNode, true ) ) { - onSplit( [], this.props.value, ...blocks ); - return; - } - - const { dom } = this.editor; - const beforeRange = dom.createRng(); - const afterRange = dom.createRng(); - const selectionRange = this.editor.selection.getRng(); - + let before, after; if ( rootNode.childNodes.length ) { + const { dom } = this.editor; + const beforeRange = dom.createRng(); + const afterRange = dom.createRng(); + const selectionRange = this.editor.selection.getRng(); + beforeRange.setStart( rootNode, 0 ); beforeRange.setEnd( selectionRange.startContainer, selectionRange.startOffset ); @@ -609,20 +597,32 @@ export class RichText extends Component { const afterFragment = afterRange.extractContents(); const { format } = this.props; - let before = domToFormat( filterEmptyNodes( beforeFragment.childNodes ), format, this.editor ); - let after = domToFormat( filterEmptyNodes( afterFragment.childNodes ), format, this.editor ); + before = domToFormat( filterEmptyNodes( beforeFragment.childNodes ), format, this.editor ); + after = domToFormat( filterEmptyNodes( afterFragment.childNodes ), format, this.editor ); + } else { + before = []; + after = []; + } - if ( context.paste ) { - before = this.isEmpty( before ) ? null : before; - after = this.isEmpty( after ) ? null : after; - } + // In case split occurs at the trailing or leading edge of the field, + // assume that the before/after values respectively reflect the current + // value. This also provides an opportunity for the parent component to + // determine whether the before/after value has changed using a trivial + // strict equality operation. + if ( this.isEmpty( after ) ) { + before = this.props.value; + } else if ( this.isEmpty( before ) ) { + after = this.props.value; + } - this.restoreContentAndSplit( before, after, blocks ); - } else if ( context.paste ) { - this.restoreContentAndSplit( null, null, blocks ); - } else { - this.restoreContentAndSplit( [], [], blocks ); + // If pasting and the split would result in no content other than the + // pasted blocks, remove the before and after blocks. + if ( context.paste ) { + before = this.isEmpty( before ) ? null : before; + after = this.isEmpty( after ) ? null : after; } + + this.restoreContentAndSplit( before, after, blocks ); } onNewBlock() {