Try: Fix "undo trap" by supporting transient states when setting block attributes

packages/block-library/src/image/edit.js

@@ -32,7 +32,7 @@ import {
 	withNotices,
 	ToggleControl,
 } from '@wordpress/components';
-import { withSelect } from '@wordpress/data';
+import { withSelect, withDispatch } from '@wordpress/data';
 import {
 	RichText,
 	BlockControls,
@@ -92,7 +92,7 @@ const isTemporaryImage = ( id, url ) => ! id && isBlobURL( url );
 const isExternalImage = ( id, url ) => url && ! id && ! isBlobURL( url );
 
 class ImageEdit extends Component {
-	constructor( { attributes } ) {
+	constructor() {
 		super( ...arguments );
 		this.updateAlt = this.updateAlt.bind( this );
 		this.updateAlignment = this.updateAlignment.bind( this );
@@ -110,31 +110,24 @@ class ImageEdit extends Component {
 		this.onSetLinkDestination = this.onSetLinkDestination.bind( this );
 		this.onSetNewTab = this.onSetNewTab.bind( this );
 		this.getFilename = this.getFilename.bind( this );
-		this.toggleIsEditing = this.toggleIsEditing.bind( this );
 		this.onUploadError = this.onUploadError.bind( this );
 		this.onImageError = this.onImageError.bind( this );
 
 		this.state = {
 			captionFocused: false,
-			isEditing: ! attributes.url,
 		};
 	}
 
 	componentDidMount() {
-		const { attributes, setAttributes } = this.props;
+		const { attributes } = this.props;
 		const { id, url = '' } = attributes;
 
 		if ( isTemporaryImage( id, url ) ) {
-			const file = getBlobByURL( url );
+			this.props.popUndoLevel();
 
+			const file = getBlobByURL( url );
 			if ( file ) {
-				mediaUpload( {
-					filesList: [ file ],
-					onFileChange: ( [ image ] ) => {
-						setAttributes( pickRelevantMediaFiles( image ) );
-					},
-					allowedTypes: ALLOWED_MEDIA_TYPES,
-				} );
+				this.uploadFile( file );
 			}
 		}
 	}
@@ -154,12 +147,31 @@ class ImageEdit extends Component {
 		}
 	}
 
+	uploadFile( file ) {
+		this.props.setAttributes(
+			{ url: this.props.attributes.url },
+			{ transient: true }
+		);
+
+		mediaUpload( {
+			filesList: [ file ],
+			onFileChange: ( [ image ] ) => {
+				const { setAttributes } = this.props;
+				const { id, url, ...nextAttributes } = pickRelevantMediaFiles( image );
+				if ( isTemporaryImage( id, url ) ) {
+					setAttributes( { ...nextAttributes, id } );
+					setAttributes( { url }, { transient: true } );
+				} else {
+					setAttributes( { ...nextAttributes, id, url } );
+				}
+			},
+			allowedTypes: ALLOWED_MEDIA_TYPES,
+		} );
+	}
+
 	onUploadError( message ) {
 		const { noticeOperations } = this.props;
 		noticeOperations.createErrorNotice( message );
-		this.setState( {
-			isEditing: true,
-		} );
 	}
 
 	onSelectImage( media ) {
@@ -173,10 +185,6 @@ class ImageEdit extends Component {
 			return;
 		}
 
-		this.setState( {
-			isEditing: false,
-		} );
-
 		this.props.setAttributes( {
 			...pickRelevantMediaFiles( media ),
 			width: undefined,
@@ -212,10 +220,6 @@ class ImageEdit extends Component {
 				id: undefined,
 			} );
 		}
-
-		this.setState( {
-			isEditing: false,
-		} );
 	}
 
 	onImageError( url ) {
@@ -318,12 +322,6 @@ class ImageEdit extends Component {
 		];
 	}
 
-	toggleIsEditing() {
-		this.setState( {
-			isEditing: ! this.state.isEditing,
-		} );
-	}
-
 	getImageSizeOptions() {
 		const { imageSizes, image } = this.props;
 		return compact( map( imageSizes, ( { name, slug } ) => {
@@ -339,7 +337,6 @@ class ImageEdit extends Component {
 	}
 
 	render() {
-		const { isEditing } = this.state;
 		const {
 			attributes,
 			setAttributes,
@@ -376,7 +373,7 @@ class ImageEdit extends Component {
 						<IconButton
 							className="components-icon-button components-toolbar__control"
 							label={ __( 'Edit image' ) }
-							onClick={ this.toggleIsEditing }
+							onClick={ () => setAttributes( { url: undefined } ) }
 							icon="edit"
 						/>
 					</Toolbar>
@@ -414,8 +411,7 @@ class ImageEdit extends Component {
 			</BlockControls>
 		);
 
-		if ( isEditing ) {
-			const src = isExternal ? url : undefined;
+		if ( ! url ) {
 			return (
 				<Fragment>
 					{ controls }
@@ -428,7 +424,6 @@ class ImageEdit extends Component {
 						onError={ this.onUploadError }
 						accept="image/*"
 						allowedTypes={ ALLOWED_MEDIA_TYPES }
-						value={ { id, src } }
 					/>
 				</Fragment>
 			);
@@ -712,6 +707,13 @@ export default compose( [
 			imageSizes,
 		};
 	} ),
+	withDispatch( ( dispatch ) => {
+		const { popUndoLevel } = dispatch( 'core/editor' );
+
+		return {
+			popUndoLevel,
+		};
+	} ),
 	withViewportMatch( { isLargeViewport: 'medium' } ),
 	withNotices,
 ] )( ImageEdit );

packages/editor/src/components/autosave-monitor/index.js

@@ -7,14 +7,24 @@ import { withSelect, withDispatch } from '@wordpress/data';
 
 export class AutosaveMonitor extends Component {
 	componentDidUpdate( prevProps ) {
-		const { isDirty, editsReference, isAutosaveable } = this.props;
+		const {
+			isDirty,
+			editsReference,
+			isAutosaveable,
+			hasPendingBlockOperations,
+		} = this.props;
 
 		if (
 			prevProps.isDirty !== isDirty ||
 			prevProps.isAutosaveable !== isAutosaveable ||
-			prevProps.editsReference !== editsReference
+			prevProps.editsReference !== editsReference ||
+			prevProps.hasPendingBlockOperations !== hasPendingBlockOperations
 		) {
-			this.toggleTimer( isDirty && isAutosaveable );
+			this.toggleTimer(
+				isDirty &&
+				isAutosaveable &&
+				! hasPendingBlockOperations
+			);
 		}
 	}
 
@@ -45,6 +55,7 @@ export default compose( [
 			isEditedPostAutosaveable,
 			getEditorSettings,
 			getReferenceByDistinctEdits,
+			hasPendingBlockOperations,
 		} = select( 'core/editor' );
 
 		const { autosaveInterval } = getEditorSettings();
@@ -53,6 +64,7 @@ export default compose( [
 			isDirty: isEditedPostDirty(),
 			isAutosaveable: isEditedPostAutosaveable(),
 			editsReference: getReferenceByDistinctEdits(),
+			hasPendingBlockOperations: hasPendingBlockOperations(),
 			autosaveInterval,
 		};
 	} ),

packages/editor/src/components/block-list/block.js

@@ -165,10 +165,10 @@ export class BlockListBlock extends Component {
 		}
 	}
 
-	setAttributes( attributes ) {
+	setAttributes( attributes, options ) {
 		const { block, onChange } = this.props;
 		const type = getBlockType( block.name );
-		onChange( block.clientId, attributes );
+		onChange( block.clientId, attributes, options );
 
 		const metaAttributes = reduce( attributes, ( result, value, key ) => {
 			if ( get( type, [ 'attributes', key, 'source' ] ) === 'meta' ) {
@@ -703,8 +703,8 @@ const applyWithDispatch = withDispatch( ( dispatch, ownProps ) => {
 	} = dispatch( 'core/editor' );
 
 	return {
-		onChange( clientId, attributes ) {
-			updateBlockAttributes( clientId, attributes );
+		onChange( clientId, attributes, options ) {
+			updateBlockAttributes( clientId, attributes, options );
 		},
 		onSelect( clientId = ownProps.clientId, initialPosition ) {
 			selectBlock( clientId, initialPosition );

packages/editor/src/store/actions.js

@@ -122,16 +122,20 @@ export function receiveBlocks( blocks ) {
  * Returns an action object used in signalling that the block attributes with
  * the specified client ID has been updated.
  *
- * @param {string} clientId   Block client ID.
- * @param {Object} attributes Block attributes to be merged.
+ * @param {string}   clientId          Block client ID.
+ * @param {Object}   attributes        Block attributes to be merged.
+ * @param {?Object}  options           Optional options.
+ * @param {?boolean} options.transient Whether attribute should be considered
+ *                                     to be in a transient state.
  *
  * @return {Object} Action object.
  */
-export function updateBlockAttributes( clientId, attributes ) {
+export function updateBlockAttributes( clientId, attributes, options ) {
 	return {
 		type: 'UPDATE_BLOCK_ATTRIBUTES',
 		clientId,
 		attributes,
+		...options,
 	};
 }
 
@@ -476,6 +480,10 @@ export function createUndoLevel() {
 	return { type: 'CREATE_UNDO_LEVEL' };
 }
 
+export function popUndoLevel() {
+	return { type: 'POP_UNDO_LEVEL' };
+}
+
 /**
  * Returns an action object used in signalling that the blocks corresponding to
  * the set of specified client IDs are to be removed.

packages/editor/src/store/effects/posts.js

@@ -30,6 +30,7 @@ import {
 	getCurrentPostType,
 	isEditedPostSaveable,
 	isEditedPostNew,
+	hasPendingBlockOperations,
 	POST_UPDATE_TRANSACTION_ID,
 } from '../selectors';
 import { resolveSelector } from './utils';
@@ -52,6 +53,17 @@ export const requestPostUpdate = async ( action, store ) => {
 	const post = getCurrentPost( state );
 	const isAutosave = !! action.options.isAutosave;
 
+	if (
+		hasPendingBlockOperations( state ) &&
+		// Disable reason: A blocking prompt is justified to protect against
+		// potential content corruption.
+		//
+		// eslint-disable-next-line no-alert
+		! window.confirm( __( 'A block operation is pending. Are you sure you want to save?' ) )
+	) {
+		return;
+	}
+
 	// Prevent save if not saveable.
 	// We don't check for dirtiness here as this can be overriden in the UI.
 	if ( ! isEditedPostSaveable( state ) ) {

packages/editor/src/store/reducer.js

@@ -296,6 +296,7 @@ export const editor = flow( [
 	withHistory( {
 		resetTypes: [ 'SETUP_EDITOR_STATE' ],
 		ignoreTypes: [ 'RECEIVE_BLOCKS', 'RESET_POST', 'UPDATE_POST' ],
+		isIgnored: ( action ) => action.transient,
 		shouldOverwriteState,
 	} ),
 ] )( {
@@ -375,15 +376,23 @@ export const editor = flow( [
 						return state;
 					}
 
-					// Consider as updates only changed values
-					const nextAttributes = reduce( action.attributes, ( result, value, key ) => {
-						if ( value !== result[ key ] ) {
-							result = getMutateSafeObject( state[ action.clientId ].attributes, result );
-							result[ key ] = value;
-						}
-
-						return result;
-					}, state[ action.clientId ].attributes );
+					let nextAttributes;
+					if ( action.transient ) {
+						nextAttributes = omit(
+							state[ action.clientId ].attributes,
+							keys( action.attributes )
+						);
+					} else {
+						// Consider as updates only changed values
+						nextAttributes = reduce( action.attributes, ( result, value, key ) => {
+							if ( value !== result[ key ] ) {
+								result = getMutateSafeObject( state[ action.clientId ].attributes, result );
+								result[ key ] = value;
+							}
+
+							return result;
+						}, state[ action.clientId ].attributes );
+					}
 
 					// Skip update if nothing has been changed. The reference will
 					// match the original block if `reduce` had no changed values.
@@ -591,6 +600,40 @@ export const editor = flow( [
 	} ),
 } );
 
+export function blockTransients( state = {}, action ) {
+	if ( action.type === 'UPDATE_BLOCK_ATTRIBUTES' ) {
+		if ( action.transient ) {
+			return {
+				...state,
+				[ action.clientId ]: {
+					...state[ action.clientId ],
+					...action.attributes,
+				},
+			};
+		}
+
+		if ( ! state[ action.clientId ] ) {
+			return state;
+		}
+
+		const nextTransients = omit(
+			state[ action.clientId ],
+			...keys( action.attributes ),
+		);
+
+		if ( ! Object.keys( nextTransients ).length ) {
+			return omit( state, action.clientId );
+		}
+
+		return {
+			...state,
+			[ action.clientId ]: nextTransients,
+		};
+	}
+
+	return state;
+}
+
 /**
  * Reducer returning the initial edits state. With matching shape to that of
  * `editor.edits`, the initial edits are those applied programmatically, are
@@ -1236,6 +1279,7 @@ export function previewLink( state = null, action ) {
 
 export default optimist( combineReducers( {
 	editor,
+	blockTransients,
 	initialEdits,
 	currentPost,
 	isTyping,