diff --git a/docs/reference-guides/data/data-core.md b/docs/reference-guides/data/data-core.md index 1ee04e09550e2d..8db3a26dd09772 100644 --- a/docs/reference-guides/data/data-core.md +++ b/docs/reference-guides/data/data-core.md @@ -358,6 +358,8 @@ _Returns_ ### getRedoEdit +> **Deprecated** since 6.3 + Returns the next edit from the current undo offset for the entity records edits history, if any. _Parameters_ @@ -401,6 +403,8 @@ _Returns_ ### getUndoEdit +> **Deprecated** since 6.3 + Returns the previous edit from the current undo offset for the entity records edits history, if any. _Parameters_ diff --git a/package-lock.json b/package-lock.json index 5833c451c403be..43903adae1486b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -29227,7 +29227,7 @@ "co": { "version": "4.6.0", "resolved": "https://registry.npmjs.org/co/-/co-4.6.0.tgz", - "integrity": "sha1-bqa989hTrlTMuOR7+gvz+QMfsYQ=", + "integrity": "sha512-QVb0dM5HvG+uaxitm8wONl7jltx8dqhfU33DcqtOZcLSVIKSDDLDi7+0LbAKiyI8hD9u42m2YxXSkMGWThaecQ==", "dev": true }, "code-point-at": { @@ -30804,7 +30804,7 @@ "css.escape": { "version": "1.5.1", "resolved": "https://registry.npmjs.org/css.escape/-/css.escape-1.5.1.tgz", - "integrity": "sha512-YUifsXXuknHlUsmlgyY0PKzgPOr7/FjCePfHNt0jxm83wHZi44VDMQ7/fGNkjY3/jV1MC+1CmZbaHzugyeRtpg==", + "integrity": "sha1-QuJ9T6BK4y+TGktNQZH6nN3ul8s=", "dev": true }, "cssesc": { @@ -41365,7 +41365,7 @@ "lz-string": { "version": "1.4.4", "resolved": "https://registry.npmjs.org/lz-string/-/lz-string-1.4.4.tgz", - "integrity": "sha512-0ckx7ZHRPqb0oUm8zNr+90mtf9DQB60H1wMCjBtfi62Kl3a7JbHob6gA2bC+xRvZoOL+1hzUK8jeuEIQE8svEQ==", + "integrity": "sha1-wNjq82BZ9wV5bh40SBHPTEmNOiY=", "dev": true }, "macos-release": { diff --git a/packages/core-data/README.md b/packages/core-data/README.md index dddc3550e03b26..d2ac90a4a5165b 100644 --- a/packages/core-data/README.md +++ b/packages/core-data/README.md @@ -535,6 +535,8 @@ _Returns_ ### getRedoEdit +> **Deprecated** since 6.3 + Returns the next edit from the current undo offset for the entity records edits history, if any. _Parameters_ @@ -578,6 +580,8 @@ _Returns_ ### getUndoEdit +> **Deprecated** since 6.3 + Returns the previous edit from the current undo offset for the entity records edits history, if any. _Parameters_ diff --git a/packages/core-data/src/actions.js b/packages/core-data/src/actions.js index ffae417a83cd13..cfab95aae9f8fc 100644 --- a/packages/core-data/src/actions.js +++ b/packages/core-data/src/actions.js @@ -18,6 +18,7 @@ import { receiveItems, removeItems, receiveQueriedItems } from './queried-data'; import { getOrLoadEntitiesConfig, DEFAULT_ENTITY_KEY } from './entities'; import { createBatch } from './batch'; import { STORE_NAME } from './name'; +import { getUndoEdits, getRedoEdits } from './private-selectors'; /** * Returns an action object used in signalling that authors have been received. @@ -406,14 +407,14 @@ export const editEntityRecord = export const undo = () => ( { select, dispatch } ) => { - const undoEdit = select.getUndoEdit(); + // Todo: we shouldn't have to pass "root" here. + const undoEdit = select( ( state ) => getUndoEdits( state.root ) ); if ( ! undoEdit ) { return; } dispatch( { - type: 'EDIT_ENTITY_RECORD', - ...undoEdit, - meta: { isUndo: true }, + type: 'UNDO', + stackedEdits: undoEdit, } ); }; @@ -424,14 +425,14 @@ export const undo = export const redo = () => ( { select, dispatch } ) => { - const redoEdit = select.getRedoEdit(); + // Todo: we shouldn't have to pass "root" here. + const redoEdit = select( ( state ) => getRedoEdits( state.root ) ); if ( ! redoEdit ) { return; } dispatch( { - type: 'EDIT_ENTITY_RECORD', - ...redoEdit, - meta: { isRedo: true }, + type: 'REDO', + stackedEdits: redoEdit, } ); }; diff --git a/packages/core-data/src/index.js b/packages/core-data/src/index.js index 43fa4a0b3cd074..c2b491fa8c1ea1 100644 --- a/packages/core-data/src/index.js +++ b/packages/core-data/src/index.js @@ -62,7 +62,6 @@ const storeConfig = () => ( { * @see https://github.com/WordPress/gutenberg/blob/HEAD/packages/data/README.md#createReduxStore */ export const store = createReduxStore( STORE_NAME, storeConfig() ); - register( store ); export { default as EntityProvider } from './entity-provider'; diff --git a/packages/core-data/src/private-selectors.ts b/packages/core-data/src/private-selectors.ts new file mode 100644 index 00000000000000..0ac2c750239692 --- /dev/null +++ b/packages/core-data/src/private-selectors.ts @@ -0,0 +1,30 @@ +/** + * Internal dependencies + */ +import type { State, UndoEdit } from './selectors'; + +type Optional< T > = T | undefined; + +/** + * Returns the previous edit from the current undo offset + * for the entity records edits history, if any. + * + * @param state State tree. + * + * @return The edit. + */ +export function getUndoEdits( state: State ): Optional< UndoEdit[] > { + return state.undo.list[ state.undo.list.length - 1 + state.undo.offset ]; +} + +/** + * Returns the next edit from the current undo offset + * for the entity records edits history, if any. + * + * @param state State tree. + * + * @return The edit. + */ +export function getRedoEdits( state: State ): Optional< UndoEdit[] > { + return state.undo.list[ state.undo.list.length + state.undo.offset ]; +} diff --git a/packages/core-data/src/reducer.js b/packages/core-data/src/reducer.js index f04d543919b8c8..b7dd9d73df15a7 100644 --- a/packages/core-data/src/reducer.js +++ b/packages/core-data/src/reducer.js @@ -183,6 +183,30 @@ export function themeGlobalStyleVariations( state = {}, action ) { return state; } +const withMultiEntityRecordEdits = ( reducer ) => ( state, action ) => { + if ( action.type === 'UNDO' || action.type === 'REDO' ) { + const { stackedEdits } = action; + + let newState = state; + stackedEdits.forEach( + ( { kind, name, recordId, property, from, to } ) => { + newState = reducer( newState, { + type: 'EDIT_ENTITY_RECORD', + kind, + name, + recordId, + edits: { + [ property ]: action.type === 'UNDO' ? from : to, + }, + } ); + } + ); + return newState; + } + + return reducer( state, action ); +}; + /** * Higher Order Reducer for a given entity config. It supports: * @@ -196,6 +220,8 @@ export function themeGlobalStyleVariations( state = {}, action ) { */ function entity( entityConfig ) { return compose( [ + withMultiEntityRecordEdits, + // Limit to matching action type so we don't attempt to replace action on // an unhandled action. ifMatchingAction( @@ -411,8 +437,9 @@ export const entities = ( state = {}, action ) => { /** * @typedef {Object} UndoStateMeta * - * @property {number} offset Where in the undo stack we are. - * @property {Object} [flattenedUndo] Flattened form of undo stack. + * @property {number} list The undo stack. + * @property {number} offset Where in the undo stack we are. + * @property {Object} cache Cache of unpersisted transient edits. */ /** @typedef {Array & UndoStateMeta} UndoState */ @@ -422,10 +449,7 @@ export const entities = ( state = {}, action ) => { * * @todo Given how we use this we might want to make a custom class for it. */ -const UNDO_INITIAL_STATE = Object.assign( [], { offset: 0 } ); - -/** @type {Object} */ -let lastEditAction; +const UNDO_INITIAL_STATE = { list: [], offset: 0 }; /** * Reducer keeping track of entity edit undo history. @@ -436,107 +460,114 @@ let lastEditAction; * @return {UndoState} Updated state. */ export function undo( state = UNDO_INITIAL_STATE, action ) { + const omitPendingRedos = ( currentState ) => { + return { + ...currentState, + list: currentState.list.slice( + 0, + currentState.offset || undefined + ), + offset: 0, + }; + }; + + const appendCachedEditsToLastUndo = ( currentState ) => { + if ( ! currentState.cache ) { + return currentState; + } + + let nextState = { + ...currentState, + list: [ ...currentState.list ], + }; + nextState = omitPendingRedos( nextState ); + const previousUndoState = nextState.list.pop(); + const updatedUndoState = currentState.cache.reduce( + appendEditToStack, + previousUndoState + ); + nextState.list.push( updatedUndoState ); + + return { + ...nextState, + cache: undefined, + }; + }; + + const appendEditToStack = ( + stack = [], + { kind, name, recordId, property, from, to } + ) => { + const existingEditIndex = stack?.findIndex( + ( { kind: k, name: n, recordId: r, property: p } ) => { + return ( + k === kind && n === name && r === recordId && p === property + ); + } + ); + const nextStack = [ ...stack ]; + if ( existingEditIndex !== -1 ) { + // If the edit is already in the stack leave the initial "from" value. + nextStack[ existingEditIndex ] = { + ...nextStack[ existingEditIndex ], + to, + }; + } else { + nextStack.push( { + kind, + name, + recordId, + property, + from, + to, + } ); + } + return nextStack; + }; + switch ( action.type ) { - case 'EDIT_ENTITY_RECORD': case 'CREATE_UNDO_LEVEL': - let isCreateUndoLevel = action.type === 'CREATE_UNDO_LEVEL'; - const isUndoOrRedo = - ! isCreateUndoLevel && - ( action.meta.isUndo || action.meta.isRedo ); - if ( isCreateUndoLevel ) { - action = lastEditAction; - } else if ( ! isUndoOrRedo ) { - // Don't lose the last edit cache if the new one only has transient edits. - // Transient edits don't create new levels so updating the cache would make - // us skip an edit later when creating levels explicitly. - if ( - Object.keys( action.edits ).some( - ( key ) => ! action.transientEdits[ key ] - ) - ) { - lastEditAction = action; - } else { - lastEditAction = { - ...action, - edits: { - ...( lastEditAction && lastEditAction.edits ), - ...action.edits, - }, - }; - } - } + return appendCachedEditsToLastUndo( state ); - /** @type {UndoState} */ - let nextState; - - if ( isUndoOrRedo ) { - // @ts-ignore we might consider using Object.assign({}, state) - nextState = [ ...state ]; - nextState.offset = - state.offset + ( action.meta.isUndo ? -1 : 1 ); - - if ( state.flattenedUndo ) { - // The first undo in a sequence of undos might happen while we have - // flattened undos in state. If this is the case, we want execution - // to continue as if we were creating an explicit undo level. This - // will result in an extra undo level being appended with the flattened - // undo values. - // We also have to take into account if the `lastEditAction` had opted out - // of being tracked in undo history, like the action that persists the latest - // content right before saving. In that case we have to update the `lastEditAction` - // to avoid returning early before applying the existing flattened undos. - isCreateUndoLevel = true; - if ( ! lastEditAction.meta.undo ) { - lastEditAction.meta.undo = { - edits: {}, - }; - } - action = lastEditAction; - } else { - return nextState; - } - } + case 'UNDO': + case 'REDO': { + const nextState = appendCachedEditsToLastUndo( state ); + return { + ...nextState, + offset: state.offset + ( action.type === 'UNDO' ? -1 : 1 ), + }; + } + case 'EDIT_ENTITY_RECORD': { if ( ! action.meta.undo ) { return state; } - // Transient edits don't create an undo level, but are - // reachable in the next meaningful edit to which they - // are merged. They are defined in the entity's config. - if ( - ! isCreateUndoLevel && - ! Object.keys( action.edits ).some( - ( key ) => ! action.transientEdits[ key ] - ) - ) { - // @ts-ignore we might consider using Object.assign({}, state) - nextState = [ ...state ]; - nextState.flattenedUndo = { - ...state.flattenedUndo, - ...action.edits, + const isCachedChange = Object.keys( action.edits ).every( + ( key ) => action.transientEdits[ key ] + ); + + const edits = Object.keys( action.edits ).map( ( key ) => { + return { + kind: action.kind, + name: action.name, + recordId: action.recordId, + property: key, + from: action.meta.undo.edits[ key ], + to: action.edits[ key ], }; - nextState.offset = state.offset; - return nextState; - } + } ); - // Clear potential redos, because this only supports linear history. - nextState = - // @ts-ignore this needs additional cleanup, probably involving code-level changes - nextState || state.slice( 0, state.offset || undefined ); - nextState.offset = nextState.offset || 0; - nextState.pop(); - if ( ! isCreateUndoLevel ) { - nextState.push( { - kind: action.meta.undo.kind, - name: action.meta.undo.name, - recordId: action.meta.undo.recordId, - edits: { - ...state.flattenedUndo, - ...action.meta.undo.edits, - }, - } ); + if ( isCachedChange ) { + return { + ...state, + cache: edits.reduce( appendEditToStack, state.cache ), + }; } + + let nextState = omitPendingRedos( state ); + nextState = appendCachedEditsToLastUndo( nextState ); + nextState = { ...nextState, list: [ ...nextState.list ] }; // When an edit is a function it's an optimization to avoid running some expensive operation. // We can't rely on the function references being the same so we opt out of comparing them here. const comparisonUndoEdits = Object.values( @@ -546,16 +577,11 @@ export function undo( state = UNDO_INITIAL_STATE, action ) { ( edit ) => typeof edit !== 'function' ); if ( ! isShallowEqual( comparisonUndoEdits, comparisonEdits ) ) { - nextState.push( { - kind: action.kind, - name: action.name, - recordId: action.recordId, - edits: isCreateUndoLevel - ? { ...state.flattenedUndo, ...action.edits } - : action.edits, - } ); + nextState.list.push( edits ); } + return nextState; + } } return state; diff --git a/packages/core-data/src/selectors.ts b/packages/core-data/src/selectors.ts index 7513d918109673..a6b7774d37094c 100644 --- a/packages/core-data/src/selectors.ts +++ b/packages/core-data/src/selectors.ts @@ -22,6 +22,7 @@ import { setNestedValue, } from './utils'; import type * as ET from './entity-types'; +import { getUndoEdits, getRedoEdits } from './private-selectors'; // This is an incomplete, high-level approximation of the State type. // It makes the selectors slightly more safe, but is intended to evolve @@ -73,9 +74,18 @@ interface EntityConfig { kind: string; } -interface UndoState extends Array< Object > { - flattenedUndo: unknown; +export interface UndoEdit { + name: string; + kind: string; + recordId: string; + from: any; + to: any; +} + +interface UndoState { + list: Array< UndoEdit[] >; offset: number; + cache: UndoEdit[]; } interface UserState { @@ -884,24 +894,38 @@ function getCurrentUndoOffset( state: State ): number { * Returns the previous edit from the current undo offset * for the entity records edits history, if any. * - * @param state State tree. + * @deprecated since 6.3 + * + * @param state State tree. * * @return The edit. */ export function getUndoEdit( state: State ): Optional< any > { - return state.undo[ state.undo.length - 2 + getCurrentUndoOffset( state ) ]; + deprecated( "select( 'core' ).getUndoEdit()", { + since: '6.3', + } ); + return state.undo.list[ + state.undo.list.length - 2 + getCurrentUndoOffset( state ) + ]?.[ 0 ]; } /** * Returns the next edit from the current undo offset * for the entity records edits history, if any. * - * @param state State tree. + * @deprecated since 6.3 + * + * @param state State tree. * * @return The edit. */ export function getRedoEdit( state: State ): Optional< any > { - return state.undo[ state.undo.length + getCurrentUndoOffset( state ) ]; + deprecated( "select( 'core' ).getRedoEdit()", { + since: '6.3', + } ); + return state.undo.list[ + state.undo.list.length + getCurrentUndoOffset( state ) + ]?.[ 0 ]; } /** @@ -913,7 +937,7 @@ export function getRedoEdit( state: State ): Optional< any > { * @return Whether there is a previous edit or not. */ export function hasUndo( state: State ): boolean { - return Boolean( getUndoEdit( state ) ); + return Boolean( getUndoEdits( state ) ); } /** @@ -925,7 +949,7 @@ export function hasUndo( state: State ): boolean { * @return Whether there is a next edit or not. */ export function hasRedo( state: State ): boolean { - return Boolean( getRedoEdit( state ) ); + return Boolean( getRedoEdits( state ) ); } /** @@ -1142,11 +1166,7 @@ export const hasFetchedAutosaves = createRegistrySelector( export const getReferenceByDistinctEdits = createSelector( // This unused state argument is listed here for the documentation generating tool (docgen). ( state: State ) => [], - ( state: State ) => [ - state.undo.length, - state.undo.offset, - state.undo.flattenedUndo, - ] + ( state: State ) => [ state.undo.list.length, state.undo.offset ] ); /** diff --git a/packages/core-data/src/test/reducer.js b/packages/core-data/src/test/reducer.js index 63caf5fb83b177..4f7d9b9c0d2aec 100644 --- a/packages/core-data/src/test/reducer.js +++ b/packages/core-data/src/test/reducer.js @@ -143,28 +143,34 @@ describe( 'entities', () => { } ); describe( 'undo', () => { - let lastEdits; + let lastValues; let undoState; let expectedUndoState; - const createEditActionPart = ( edits ) => ( { + + const createExpectedDiff = ( property, { from, to } ) => ( { kind: 'someKind', name: 'someName', recordId: 'someRecordId', - edits, + property, + from, + to, } ); const createNextEditAction = ( edits, transientEdits = {} ) => { let action = { - ...createEditActionPart( edits ), + kind: 'someKind', + name: 'someName', + recordId: 'someRecordId', + edits, transientEdits, }; action = { type: 'EDIT_ENTITY_RECORD', ...action, meta: { - undo: { ...action, edits: lastEdits }, + undo: { edits: lastValues }, }, }; - lastEdits = { ...lastEdits, ...edits }; + lastValues = { ...lastValues, ...edits }; return action; }; const createNextUndoState = ( ...args ) => { @@ -172,17 +178,17 @@ describe( 'undo', () => { if ( args[ 0 ] === 'isUndo' || args[ 0 ] === 'isRedo' ) { // We need to "apply" the undo level here and build // the action to move the offset. - lastEdits = - undoState[ - undoState.length + - undoState.offset - - ( args[ 0 ] === 'isUndo' ? 2 : 0 ) - ].edits; + const lastEdits = + undoState.list[ + undoState.list.length - + ( args[ 0 ] === 'isUndo' ? 1 : 0 ) + + undoState.offset + ]; + lastEdits.forEach( ( { property, from, to } ) => { + lastValues[ property ] = args[ 0 ] === 'isUndo' ? from : to; + } ); action = { - type: 'EDIT_ENTITY_RECORD', - meta: { - [ args[ 0 ] ]: true, - }, + type: args[ 0 ] === 'isUndo' ? 'UNDO' : 'REDO', }; } else if ( args[ 0 ] === 'isCreate' ) { action = { type: 'CREATE_UNDO_LEVEL' }; @@ -192,10 +198,9 @@ describe( 'undo', () => { return deepFreeze( undo( undoState, action ) ); }; beforeEach( () => { - lastEdits = {}; + lastValues = {}; undoState = undefined; - expectedUndoState = []; - expectedUndoState.offset = 0; + expectedUndoState = { list: [], offset: 0 }; } ); it( 'initializes', () => { @@ -208,19 +213,41 @@ describe( 'undo', () => { // Check that the first edit creates an undo level for the current state and // one for the new one. undoState = createNextUndoState( { value: 1 } ); - expectedUndoState.push( - createEditActionPart( {} ), - createEditActionPart( { value: 1 } ) - ); + expectedUndoState.list.push( [ + createExpectedDiff( 'value', { from: undefined, to: 1 } ), + ] ); expect( undoState ).toEqual( expectedUndoState ); // Check that the second and third edits just create an undo level for // themselves. undoState = createNextUndoState( { value: 2 } ); - expectedUndoState.push( createEditActionPart( { value: 2 } ) ); + expectedUndoState.list.push( [ + createExpectedDiff( 'value', { from: 1, to: 2 } ), + ] ); expect( undoState ).toEqual( expectedUndoState ); undoState = createNextUndoState( { value: 3 } ); - expectedUndoState.push( createEditActionPart( { value: 3 } ) ); + expectedUndoState.list.push( [ + createExpectedDiff( 'value', { from: 2, to: 3 } ), + ] ); + expect( undoState ).toEqual( expectedUndoState ); + } ); + + it( 'stacks multi-property undo levels', () => { + undoState = createNextUndoState(); + + undoState = createNextUndoState( { value: 1 } ); + undoState = createNextUndoState( { value2: 2 } ); + expectedUndoState.list.push( + [ createExpectedDiff( 'value', { from: undefined, to: 1 } ) ], + [ createExpectedDiff( 'value2', { from: undefined, to: 2 } ) ] + ); + expect( undoState ).toEqual( expectedUndoState ); + + // Check that that creating another undo level merges the "edits" + undoState = createNextUndoState( { value: 2 } ); + expectedUndoState.list.push( [ + createExpectedDiff( 'value', { from: 1, to: 2 } ), + ] ); expect( undoState ).toEqual( expectedUndoState ); } ); @@ -229,11 +256,10 @@ describe( 'undo', () => { undoState = createNextUndoState( { value: 1 } ); undoState = createNextUndoState( { value: 2 } ); undoState = createNextUndoState( { value: 3 } ); - expectedUndoState.push( - createEditActionPart( {} ), - createEditActionPart( { value: 1 } ), - createEditActionPart( { value: 2 } ), - createEditActionPart( { value: 3 } ) + expectedUndoState.list.push( + [ createExpectedDiff( 'value', { from: undefined, to: 1 } ) ], + [ createExpectedDiff( 'value', { from: 1, to: 2 } ) ], + [ createExpectedDiff( 'value', { from: 2, to: 3 } ) ] ); expect( undoState ).toEqual( expectedUndoState ); @@ -255,17 +281,22 @@ describe( 'undo', () => { // Check that another edit will go on top when there // is no undo level offset. undoState = createNextUndoState( { value: 4 } ); - expectedUndoState.push( createEditActionPart( { value: 4 } ) ); + expectedUndoState.list.push( [ + createExpectedDiff( 'value', { from: 3, to: 4 } ), + ] ); expect( undoState ).toEqual( expectedUndoState ); // Check that undoing and editing will slice of // all the levels after the current one. undoState = createNextUndoState( 'isUndo' ); undoState = createNextUndoState( 'isUndo' ); + undoState = createNextUndoState( { value: 5 } ); - expectedUndoState.pop(); - expectedUndoState.pop(); - expectedUndoState.push( createEditActionPart( { value: 5 } ) ); + expectedUndoState.list.pop(); + expectedUndoState.list.pop(); + expectedUndoState.list.push( [ + createExpectedDiff( 'value', { from: 2, to: 5 } ), + ] ); expect( undoState ).toEqual( expectedUndoState ); } ); @@ -277,10 +308,15 @@ describe( 'undo', () => { { transientValue: true } ); undoState = createNextUndoState( { value: 3 } ); - expectedUndoState.push( - createEditActionPart( {} ), - createEditActionPart( { value: 1, transientValue: 2 } ), - createEditActionPart( { value: 3 } ) + expectedUndoState.list.push( + [ + createExpectedDiff( 'value', { from: undefined, to: 1 } ), + createExpectedDiff( 'transientValue', { + from: undefined, + to: 2, + } ), + ], + [ createExpectedDiff( 'value', { from: 1, to: 3 } ) ] ); expect( undoState ).toEqual( expectedUndoState ); } ); @@ -292,10 +328,9 @@ describe( 'undo', () => { // transient edits. undoState = createNextUndoState( { value: 1 } ); undoState = createNextUndoState( 'isCreate' ); - expectedUndoState.push( - createEditActionPart( {} ), - createEditActionPart( { value: 1 } ) - ); + expectedUndoState.list.push( [ + createExpectedDiff( 'value', { from: undefined, to: 1 } ), + ] ); expect( undoState ).toEqual( expectedUndoState ); // Check that transient edits are merged into the last @@ -305,18 +340,19 @@ describe( 'undo', () => { { transientValue: true } ); undoState = createNextUndoState( 'isCreate' ); - expectedUndoState[ - expectedUndoState.length - 1 - ].edits.transientValue = 2; + expectedUndoState.list[ expectedUndoState.list.length - 1 ].push( + createExpectedDiff( 'transientValue', { from: undefined, to: 2 } ) + ); expect( undoState ).toEqual( expectedUndoState ); - // Check that undo levels are created with the latest action, - // even if undone. + // Check that create after undo does nothing. undoState = createNextUndoState( { value: 3 } ); undoState = createNextUndoState( 'isUndo' ); undoState = createNextUndoState( 'isCreate' ); - expectedUndoState.pop(); - expectedUndoState.push( createEditActionPart( { value: 3 } ) ); + expectedUndoState.list.push( [ + createExpectedDiff( 'value', { from: 1, to: 3 } ), + ] ); + expectedUndoState.offset = -1; expect( undoState ).toEqual( expectedUndoState ); } ); @@ -328,10 +364,10 @@ describe( 'undo', () => { { transientValue: true } ); undoState = createNextUndoState( 'isUndo' ); - expectedUndoState.push( - createEditActionPart( {} ), - createEditActionPart( { value: 1, transientValue: 2 } ) - ); + expectedUndoState.list.push( [ + createExpectedDiff( 'value', { from: undefined, to: 1 } ), + createExpectedDiff( 'transientValue', { from: undefined, to: 2 } ), + ] ); expectedUndoState.offset--; expect( undoState ).toEqual( expectedUndoState ); } ); @@ -341,7 +377,6 @@ describe( 'undo', () => { undoState = createNextUndoState(); undoState = createNextUndoState( { value } ); undoState = createNextUndoState( { value: () => {} } ); - expectedUndoState.push( createEditActionPart( { value } ) ); expect( undoState ).toEqual( expectedUndoState ); } ); } ); diff --git a/packages/core-data/src/test/selectors.js b/packages/core-data/src/test/selectors.js index 0ea9e26e505437..84fecc7d07cda9 100644 --- a/packages/core-data/src/test/selectors.js +++ b/packages/core-data/src/test/selectors.js @@ -838,20 +838,20 @@ describe( 'getCurrentUser', () => { describe( 'getReferenceByDistinctEdits', () => { it( 'should return referentially equal values across empty states', () => { - const state = { undo: [] }; + const state = { undo: { list: [] } }; expect( getReferenceByDistinctEdits( state ) ).toBe( getReferenceByDistinctEdits( state ) ); - const beforeState = { undo: [] }; - const afterState = { undo: [] }; + const beforeState = { undo: { list: [] } }; + const afterState = { undo: { list: [] } }; expect( getReferenceByDistinctEdits( beforeState ) ).toBe( getReferenceByDistinctEdits( afterState ) ); } ); it( 'should return referentially equal values across unchanging non-empty state', () => { - const undoStates = [ {} ]; + const undoStates = { list: [ {} ] }; const state = { undo: undoStates }; expect( getReferenceByDistinctEdits( state ) ).toBe( getReferenceByDistinctEdits( state ) @@ -866,9 +866,9 @@ describe( 'getReferenceByDistinctEdits', () => { describe( 'when adding edits', () => { it( 'should return referentially different values across changing states', () => { - const beforeState = { undo: [ {} ] }; + const beforeState = { undo: { list: [ {} ] } }; beforeState.undo.offset = 0; - const afterState = { undo: [ {}, {} ] }; + const afterState = { undo: { list: [ {}, {} ] } }; afterState.undo.offset = 1; expect( getReferenceByDistinctEdits( beforeState ) ).not.toBe( getReferenceByDistinctEdits( afterState ) @@ -878,9 +878,9 @@ describe( 'getReferenceByDistinctEdits', () => { describe( 'when using undo', () => { it( 'should return referentially different values across changing states', () => { - const beforeState = { undo: [ {}, {} ] }; + const beforeState = { undo: { list: [ {}, {} ] } }; beforeState.undo.offset = 1; - const afterState = { undo: [ {}, {} ] }; + const afterState = { undo: { list: [ {}, {} ] } }; afterState.undo.offset = 0; expect( getReferenceByDistinctEdits( beforeState ) ).not.toBe( getReferenceByDistinctEdits( afterState ) diff --git a/test/e2e/specs/editor/various/undo.spec.js b/test/e2e/specs/editor/various/undo.spec.js index 29b34ea416ff29..a4b68e1036dcf5 100644 --- a/test/e2e/specs/editor/various/undo.spec.js +++ b/test/e2e/specs/editor/various/undo.spec.js @@ -455,6 +455,35 @@ test.describe( 'undo', () => { }, ] ); } ); + + // @see https://github.com/WordPress/gutenberg/issues/12075 + test( 'should be able to undo and redo property cross property changes', async ( { + page, + pageUtils, + } ) => { + await page.getByRole( 'textbox', { name: 'Add title' } ).type( 'a' ); // First step. + await page.keyboard.press( 'Backspace' ); // Second step. + await page.getByRole( 'button', { name: 'Add default block' } ).click(); // third step. + + // Title should be empty + await expect( + page.getByRole( 'textbox', { name: 'Add title' } ) + ).toHaveText( '' ); + + // First undo removes the block. + // Second undo restores the title. + await pageUtils.pressKeys( 'primary+z' ); + await pageUtils.pressKeys( 'primary+z' ); + await expect( + page.getByRole( 'textbox', { name: 'Add title' } ) + ).toHaveText( 'a' ); + + // Redoing the "backspace" should clear the title again. + await pageUtils.pressKeys( 'primaryShift+z' ); + await expect( + page.getByRole( 'textbox', { name: 'Add title' } ) + ).toHaveText( '' ); + } ); } ); class UndoUtils {