From 4c579ed04a212aadf661f206adb19fc2485a8db1 Mon Sep 17 00:00:00 2001
From: John <johng75@gmail.com>
Date: Thu, 13 Sep 2018 10:02:45 +0100
Subject: [PATCH] Dont trigger invalid block on empty HTML content (#9546)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* Dont trigger invalid block on empty HTML content

If a block’s HTML is set to the empty string we reset the block to it’s default HTML and mark it as valid. This is more useful than the block becoming invalid and needing additional work to resolve

* Only update HTML content if block is reset
---
 .../src/components/block-list/block-html.js   |  20 +++-
 .../components/block-list/test/block-html.js  | 110 ++++++++++++++++++
 2 files changed, 125 insertions(+), 5 deletions(-)
 create mode 100644 packages/editor/src/components/block-list/test/block-html.js

diff --git a/packages/editor/src/components/block-list/block-html.js b/packages/editor/src/components/block-list/block-html.js
index 3c70ebcfd8b928..5e4c37a7d7f791 100644
--- a/packages/editor/src/components/block-list/block-html.js
+++ b/packages/editor/src/components/block-list/block-html.js
@@ -10,10 +10,10 @@ import { isEqual } from 'lodash';
  */
 import { Component } from '@wordpress/element';
 import { compose } from '@wordpress/compose';
-import { getBlockAttributes, getBlockContent, getBlockType, isValidBlock } from '@wordpress/blocks';
+import { getBlockAttributes, getBlockContent, getBlockType, isValidBlock, getSaveContent } from '@wordpress/blocks';
 import { withSelect, withDispatch } from '@wordpress/data';
 
-class BlockHTML extends Component {
+export class BlockHTML extends Component {
 	constructor( props ) {
 		super( ...arguments );
 		this.onChange = this.onChange.bind( this );
@@ -32,10 +32,20 @@ class BlockHTML extends Component {
 	}
 
 	onBlur() {
+		const { html } = this.state;
 		const blockType = getBlockType( this.props.block.name );
-		const attributes = getBlockAttributes( blockType, this.state.html, this.props.block.attributes );
-		const isValid = isValidBlock( this.state.html, blockType, attributes );
-		this.props.onChange( this.props.clientId, attributes, this.state.html, isValid );
+		const attributes = getBlockAttributes( blockType, html, this.props.block.attributes );
+
+		// If html is empty  we reset the block to the default HTML and mark it as valid to avoid triggering an error
+		const content = html ? html : getSaveContent( blockType, attributes );
+		const isValid = html ? isValidBlock( content, blockType, attributes ) : true;
+
+		this.props.onChange( this.props.clientId, attributes, content, isValid );
+
+		// Ensure the state is updated if we reset so it displays the default content
+		if ( ! html ) {
+			this.setState( { html: content } );
+		}
 	}
 
 	onChange( event ) {
diff --git a/packages/editor/src/components/block-list/test/block-html.js b/packages/editor/src/components/block-list/test/block-html.js
new file mode 100644
index 00000000000000..442e55de3f5bba
--- /dev/null
+++ b/packages/editor/src/components/block-list/test/block-html.js
@@ -0,0 +1,110 @@
+/**
+ * External dependencies
+ */
+import { shallow } from 'enzyme';
+
+/**
+ * WordPress dependencies
+ */
+import { createBlock } from '@wordpress/blocks';
+import { registerCoreBlocks } from '@wordpress/block-library';
+
+/**
+ * Internal dependencies
+ */
+import { BlockHTML } from '../block-html';
+
+describe( 'BlockHTML', () => {
+	beforeAll( () => {
+		registerCoreBlocks();
+	} );
+
+	it( 'renders HTML editor', () => {
+		const wrapper = shallow( <BlockHTML block={ { isValid: true } } /> );
+
+		expect( wrapper.name() ).toBe( 'TextareaAutosize' );
+	} );
+
+	describe( 'block content', () => {
+		it( 'use originalContent for an invalid block', () => {
+			const wrapper = shallow( <BlockHTML block={ { isValid: false, originalContent: 'test' } } /> );
+
+			expect( wrapper.state( 'html' ) ).toBe( 'test' );
+		} );
+
+		it( 'use block content for a valid block', () => {
+			const block = createBlock( 'core/paragraph', {
+				content: 'test-block',
+				isValid: true,
+			} );
+
+			const wrapper = shallow( <BlockHTML block={ block } /> );
+
+			expect( wrapper.state( 'html' ) ).toBe( '<p>test-block</p>' );
+		} );
+
+		it( 'onChange updates block html', () => {
+			const wrapper = shallow( <BlockHTML block={ { isValid: true } } /> );
+
+			wrapper.instance().onChange( { target: { value: 'update' } } );
+
+			expect( wrapper.state( 'html' ) ).toBe( 'update' );
+		} );
+	} );
+
+	describe( 'onBlur', () => {
+		const onChange = jest.fn();
+
+		beforeEach( () => {
+			onChange.mockClear();
+		} );
+
+		it( 'onBlur updates valid HTML string in block', () => {
+			const block = createBlock( 'core/paragraph', {
+				content: 'test-block',
+				isValid: true,
+			} );
+			const wrapper = shallow( <BlockHTML block={ block } onChange={ onChange } /> );
+
+			wrapper.instance().onChange( { target: { value: '<p>update</p>' } } );
+			wrapper.instance().onBlur();
+
+			expect( onChange ).toHaveBeenCalledTimes( 1 );
+			expect( onChange.mock.calls[ 0 ][ 2 ] ).toBe( '<p>update</p>' );
+			expect( onChange.mock.calls[ 0 ][ 3 ] ).toBe( true );
+		} );
+
+		it( 'onBlur updates invalid HTML string in block', () => {
+			const block = createBlock( 'core/paragraph', {
+				content: 'test-block',
+				isValid: true,
+			} );
+			const wrapper = shallow( <BlockHTML block={ block } onChange={ onChange } /> );
+
+			wrapper.instance().onChange( { target: { value: '<p>update' } } );
+			wrapper.instance().onBlur();
+
+			expect( console ).toHaveErrored();
+			expect( console ).toHaveWarned();
+			expect( onChange ).toHaveBeenCalledTimes( 1 );
+			expect( onChange.mock.calls[ 0 ][ 2 ] ).toBe( '<p>update' );
+			expect( onChange.mock.calls[ 0 ][ 3 ] ).toBe( false );
+		} );
+
+		it( 'onBlur resets block to default content if supplied empty string', () => {
+			const block = createBlock( 'core/paragraph', {
+				content: 'test-block',
+				isValid: true,
+			} );
+			const wrapper = shallow( <BlockHTML block={ block } onChange={ onChange } /> );
+
+			wrapper.instance().onChange( { target: { value: '' } } );
+			wrapper.instance().onBlur();
+
+			expect( onChange ).toHaveBeenCalledTimes( 1 );
+			expect( onChange.mock.calls[ 0 ][ 2 ] ).toBe( '<p></p>' );
+			expect( onChange.mock.calls[ 0 ][ 3 ] ).toBe( true );
+			expect( wrapper.state( 'html' ) ).toBe( '<p></p>' );
+		} );
+	} );
+} );