Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify the RESET_BLOCK action to fix template part focus mode content loss #37283

Merged
merged 1 commit into from Dec 13, 2021
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
106 changes: 5 additions & 101 deletions packages/block-editor/src/store/reducer.js
Expand Up @@ -14,7 +14,6 @@ import {
isEmpty,
identity,
omitBy,
pickBy,
} from 'lodash';

/**
Expand Down Expand Up @@ -122,48 +121,6 @@ function getFlattenedBlockAttributes( blocks ) {
return flattenBlocks( blocks, ( block ) => block.attributes );
}

/**
* Given a block order map object, returns *all* of the block client IDs that are
* a descendant of the given root client ID.
*
* Calling this with `rootClientId` set to `''` results in a list of client IDs
* that are in the post. That is, it excludes blocks like fetched reusable
* blocks which are stored into state but not visible. It also excludes
* InnerBlocks controllers, like template parts.
*
* It is important to exclude the full inner block controller and not just the
* inner blocks because in many cases, we need to persist the previous value of
* an inner block controller. To do so, it must be excluded from the list of
* client IDs which are considered to be part of the top-level entity.
*
* @param {Object} blocksOrder Object that maps block client IDs to a list of
* nested block client IDs.
* @param {?string} rootClientId The root client ID to search. Defaults to ''.
* @param {?Object} controlledInnerBlocks The InnerBlocks controller state.
*
* @return {Array} List of descendant client IDs.
*/
function getNestedBlockClientIds(
blocksOrder,
rootClientId = '',
controlledInnerBlocks = {}
) {
return reduce(
blocksOrder[ rootClientId ],
( result, clientId ) => {
if ( !! controlledInnerBlocks[ clientId ] ) {
return result;
}
return [
...result,
clientId,
...getNestedBlockClientIds( blocksOrder, clientId ),
];
},
[]
);
}

/**
* Returns an object against which it is safe to perform mutating operations,
* given the original object and its current working copy.
Expand Down Expand Up @@ -637,70 +594,17 @@ const withInnerBlocksRemoveCascade = ( reducer ) => ( state, action ) => {
*/
const withBlockReset = ( reducer ) => ( state, action ) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can probably now remove these HoR and just move its logic to the sub reducers.

if ( action.type === 'RESET_BLOCKS' ) {
/**
* A list of client IDs associated with the top level entity (like a
* post or template). It excludes the client IDs of blocks associated
* with other entities, like inner block controllers or reusable blocks.
*/
const visibleClientIds = getNestedBlockClientIds(
state?.order ?? {},
'',
state?.controlledInnerBlocks ?? {}
);

// pickBy returns only the truthy values from controlledInnerBlocks
const controlledInnerBlocks = Object.keys(
pickBy( state?.controlledInnerBlocks ?? {} )
);

/**
* Each update operation consists of a few parts:
* 1. First, the client IDs associated with the top level entity are
* removed from the existing state key, leaving in place controlled
* blocks (like reusable blocks and inner block controllers).
* 2. Second, the blocks from the reset action are used to calculate the
* individual state keys. This will re-populate the clientIDs which
* were removed in step 1.
* 3. In some cases, we remove the recalculated inner block controllers,
* letting their old values persist. We need to do this because the
* reset block action from a top-level entity is not aware of any
* inner blocks inside InnerBlock controllers. So if the new values
* were used, it would not take into account the existing InnerBlocks
* which already exist in the state for inner block controllers. For
* example, `attributes` uses the newly computed value for controllers
* since attributes are stored in the top-level entity. But `order`
* uses the previous value for the controllers since the new value
* does not include the order of controlled inner blocks. So if the
* new value was used, template parts would disappear from the editor
* whenever you try to undo a change in the top level entity.
*/
const newState = {
...state,
byClientId: {
...omit( state?.byClientId, visibleClientIds ),
...getFlattenedBlocksWithoutAttributes( action.blocks ),
},
attributes: {
...omit( state?.attributes, visibleClientIds ),
...getFlattenedBlockAttributes( action.blocks ),
},
order: {
...omit( state?.order, visibleClientIds ),
...omit(
mapBlockOrder( action.blocks ),
controlledInnerBlocks
ntsekouras marked this conversation as resolved.
Show resolved Hide resolved
),
},
parents: {
...omit( state?.parents, visibleClientIds ),
...mapBlockParents( action.blocks ),
},
controlledInnerBlocks: state?.controlledInnerBlocks || {},
byClientId: getFlattenedBlocksWithoutAttributes( action.blocks ),
attributes: getFlattenedBlockAttributes( action.blocks ),
order: mapBlockOrder( action.blocks ),
parents: mapBlockParents( action.blocks ),
controlledInnerBlocks: {},
};

const subTree = buildBlockTree( newState, action.blocks );
newState.tree = {
...omit( state?.tree, visibleClientIds ),
...subTree,
// Root
'': {
Expand Down