Editor: PostTextEditor: Fix case where empty state value defers to pr…

…op (#9513)

* Editor: PostTextEditor: Fix case where empty state value defers to prop

* PostTextEditor: Always derive state from props except when dirty

packages/editor/CHANGELOG.md

@@ -1,6 +1,6 @@
 ## 3.0.0 (Unreleased)
 
-### Breaking Change
+### Breaking Changes
 
 - The `wideAlign` block supports hook has been removed. Use `alignWide` instead.
 - `fetchSharedBlocks` action has been removed. Use `fetchReusableBlocks` instead.
@@ -18,3 +18,7 @@
 ### Deprecation
 
 - `wp.editor.RichTextProvider` flagged for deprecation. Please use `wp.data.select( 'core/editor' )` methods instead.
+
+### Bug Fixes
+
+- The `PostTextEditor` component will respect its in-progress state edited value, even if the assigned prop value changes.

packages/editor/src/components/post-text-editor/index.js

@@ -13,67 +13,59 @@ import { parse } from '@wordpress/blocks';
 import { withSelect, withDispatch } from '@wordpress/data';
 import { withInstanceId, compose } from '@wordpress/compose';
 
-/**
- * Returns the PostTextEditor state given a set of props.
- *
- * @param {Object} props Component props.
- *
- * @return {Object} State object.
- */
-function computeDerivedState( props ) {
-	return {
-		persistedValue: props.value,
-		value: props.value,
-		isDirty: false,
-	};
-}
-
-class PostTextEditor extends Component {
+export class PostTextEditor extends Component {
 	constructor() {
 		super( ...arguments );
 
-		this.startEditing = this.startEditing.bind( this );
 		this.edit = this.edit.bind( this );
 		this.stopEditing = this.stopEditing.bind( this );
 
-		this.state = {
-			value: null,
-			isDirty: false,
-		};
+		this.state = {};
 	}
 
-	static getDerivedPropsFromState( props, state ) {
-		// If we receive a new value while we're editing (but before we've made
-		// changes), go ahead and clobber the local state
-		if ( state.persistedValue !== props.value && ! state.isDirty ) {
-			return computeDerivedState( props );
+	static getDerivedStateFromProps( props, state ) {
+		if ( state.isDirty ) {
+			return null;
 		}
 
-		return null;
-	}
-
-	startEditing() {
-		// Copying the post content into local state ensures that edits won't be
-		// clobbered by changes to global editor state
-		this.setState( { value: this.props.value } );
+		return {
+			value: props.value,
+			isDirty: false,
+		};
 	}
 
+	/**
+	 * Handles a textarea change event to notify the onChange prop callback and
+	 * reflect the new value in the component's own state. This marks the start
+	 * of the user's edits, if not already changed, preventing future props
+	 * changes to value from replacing the rendered value. This is expected to
+	 * be followed by a reset to dirty state via `stopEditing`.
+	 *
+	 * @see stopEditing
+	 *
+	 * @param {Event} event Change event.
+	 */
 	edit( event ) {
 		const value = event.target.value;
 		this.props.onChange( value );
 		this.setState( { value, isDirty: true } );
 	}
 
+	/**
+	 * Function called when the user has completed their edits, responsible for
+	 * ensuring that changes, if made, are surfaced to the onPersist prop
+	 * callback and resetting dirty state.
+	 */
 	stopEditing() {
 		if ( this.state.isDirty ) {
 			this.props.onPersist( this.state.value );
+			this.setState( { isDirty: false } );
 		}
-
-		this.setState( { value: null, isDirty: false } );
 	}
 
 	render() {
-		const { value, placeholder, instanceId } = this.props;
+		const { value } = this.state;
+		const { placeholder, instanceId } = this.props;
 		const decodedPlaceholder = decodeEntities( placeholder );
 
 		return (
@@ -83,8 +75,7 @@ class PostTextEditor extends Component {
 				</label>
 				<Textarea
 					autoComplete="off"
-					value={ this.state.value || value }
-					onFocus={ this.startEditing }
+					value={ value }
 					onChange={ this.edit }
 					onBlur={ this.stopEditing }
 					className="editor-post-text-editor"

packages/editor/src/components/post-text-editor/test/index.js

@@ -0,0 +1,147 @@
+/**
+ * External dependencies
+ */
+import { create } from 'react-test-renderer';
+import Textarea from 'react-autosize-textarea';
+
+/**
+ * Internal dependencies
+ */
+import { PostTextEditor } from '../';
+
+// "Downgrade" ReactAutosizeTextarea to a regular textarea. Assumes aligned
+// props interface.
+jest.mock( 'react-autosize-textarea', () => ( props ) => <textarea { ...props } /> );
+
+describe( 'PostTextEditor', () => {
+	it( 'should render via the prop value', () => {
+		const wrapper = create( <PostTextEditor value="Hello World" /> );
+
+		const textarea = wrapper.root.findByType( Textarea );
+		expect( textarea.props.value ).toBe( 'Hello World' );
+	} );
+
+	it( 'should render via the state value when edits made', () => {
+		const onChange = jest.fn();
+		const wrapper = create(
+			<PostTextEditor
+				value="Hello World"
+				onChange={ onChange }
+			/>
+		);
+
+		const textarea = wrapper.root.findByType( Textarea );
+		textarea.props.onChange( { target: { value: 'Hello Chicken' } } );
+
+		expect( textarea.props.value ).toBe( 'Hello Chicken' );
+		expect( onChange ).toHaveBeenCalledWith( 'Hello Chicken' );
+	} );
+
+	it( 'should render via the state value when edits made, even if prop value changes', () => {
+		const onChange = jest.fn();
+		const wrapper = create(
+			<PostTextEditor
+				value="Hello World"
+				onChange={ onChange }
+			/>
+		);
+
+		const textarea = wrapper.root.findByType( Textarea );
+		textarea.props.onChange( { target: { value: 'Hello Chicken' } } );
+
+		wrapper.update(
+			<PostTextEditor
+				value="Goodbye World"
+				onChange={ onChange }
+			/>
+		);
+
+		expect( textarea.props.value ).toBe( 'Hello Chicken' );
+		expect( onChange ).toHaveBeenCalledWith( 'Hello Chicken' );
+	} );
+
+	it( 'should render via the state value when edits made, even if prop value changes and state value empty', () => {
+		const onChange = jest.fn();
+		const wrapper = create(
+			<PostTextEditor
+				value="Hello World"
+				onChange={ onChange }
+			/>
+		);
+
+		const textarea = wrapper.root.findByType( Textarea );
+		textarea.props.onChange( { target: { value: '' } } );
+
+		wrapper.update(
+			<PostTextEditor
+				value="Goodbye World"
+				onChange={ onChange }
+			/>
+		);
+
+		expect( textarea.props.value ).toBe( '' );
+		expect( onChange ).toHaveBeenCalledWith( '' );
+	} );
+
+	it( 'calls onPersist after changes made and user stops editing', () => {
+		const onPersist = jest.fn();
+		const wrapper = create(
+			<PostTextEditor
+				value="Hello World"
+				onChange={ () => {} }
+				onPersist={ onPersist }
+			/>
+		);
+
+		const textarea = wrapper.root.findByType( Textarea );
+		textarea.props.onChange( { target: { value: '' } } );
+		textarea.props.onBlur();
+
+		expect( onPersist ).toHaveBeenCalledWith( '' );
+	} );
+
+	it( 'does not call onPersist after user stops editing without changes', () => {
+		const onPersist = jest.fn();
+		const wrapper = create(
+			<PostTextEditor
+				value="Hello World"
+				onChange={ () => {} }
+				onPersist={ onPersist }
+			/>
+		);
+
+		const textarea = wrapper.root.findByType( Textarea );
+		textarea.props.onBlur();
+
+		expect( onPersist ).not.toHaveBeenCalled();
+	} );
+
+	it( 'resets to prop value after user stops editing', () => {
+		// This isn't the most realistic case, since typically we'd assume the
+		// parent renderer to pass the value as it had received onPersist. The
+		// test here is more an edge case to stress that it's intentionally
+		// differentiating between state and prop values.
+		const wrapper = create(
+			<PostTextEditor
+				value="Hello World"
+				onChange={ () => {} }
+				onPersist={ () => {} }
+			/>
+		);
+
+		const textarea = wrapper.root.findByType( Textarea );
+		textarea.props.onChange( { target: { value: '' } } );
+
+		wrapper.update(
+			<PostTextEditor
+				value="Goodbye World"
+				onChange={ () => {} }
+				onPersist={ () => {} }
+			/>
+		);
+
+		textarea.props.onBlur();
+
+		expect( textarea.props.value ).toBe( 'Goodbye World' );
+	} );
+} );