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.

Sign up for GitHub

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

Fix pasting content into inner blocks #16717

Merged
merged 3 commits into from
Jul 26, 2019
Merged

Fix pasting content into inner blocks #16717

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0c345c7
Update REPLACE_BLOCKS_AUGMENTED_WITH_CHILDREN so that it invalidates …
talldan Jul 23, 2019
2c416fd
Add jsdoc to clarify usage of getBlocksWithParentsClientIds
talldan Jul 23, 2019
8655de3
Update reducer to omit the replaced clientId, except for when it is t…
talldan Jul 23, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions packages/block-editor/src/store/reducer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,17 @@ const withBlockCache = ( reducer ) => ( state = {}, action ) => {
}
newState.cache = state.cache ? state.cache : {};

/**
* For each clientId provided, traverses up parents, adding the provided clientIds
* and each parent's clientId to the returned array.
*
* When calling this function consider that it uses the old state, so any state
* modifications made by the `reducer` will not be present.
*
* @param {Array} clientIds an Array of block clientIds.
*
* @return {Array} The provided clientIds and all of their parent clientIds.
*/
const getBlocksWithParentsClientIds = ( clientIds ) => {
return clientIds.reduce( ( result, clientId ) => {
let current = clientId;
Expand Down Expand Up @@ -264,11 +275,12 @@ const withBlockCache = ( reducer ) => ( state = {}, action ) => {
};
break;
case 'REPLACE_BLOCKS_AUGMENTED_WITH_CHILDREN':
const parentClientIds = fillKeysWithEmptyObject( getBlocksWithParentsClientIds( action.replacedClientIds ) );

newState.cache = {
...omit( newState.cache, action.replacedClientIds ),
...fillKeysWithEmptyObject(
getBlocksWithParentsClientIds( keys( flattenBlocks( action.blocks ) ) ),
),
...omit( parentClientIds, action.replacedClientIds ),
...fillKeysWithEmptyObject( keys( flattenBlocks( action.blocks ) ) ),
Copy link
Contributor Author

@talldan talldan Jul 23, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

To explain a bit what I've done here:

  1. The replacedClientIds needs to be used to find parentClientIds, but then replacedClientIds also need to be omitted, hence the second call to omit here.
  2. The new block clientId needs to be included in the cache, and there's a unit test should replace the block even if the new block clientId is the same, so that's why the last object spread is adding the new client id.

Copy link
Contributor

Choose a reason for hiding this comment

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

The logic seems good to me.

};
break;
case 'REMOVE_BLOCKS_AUGMENTED_WITH_CHILDREN':
Expand Down
8 changes: 8 additions & 0 deletions packages/block-editor/src/store/test/reducer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -759,21 +759,29 @@ describe( 'state', () => {
blocks: [ wrapperBlock ],
} );

const originalWrapperBlockCacheKey = original.cache[ wrapperBlock.clientId ];

const state = blocks( original, {
type: 'REPLACE_BLOCKS',
clientIds: [ nestedBlock.clientId ],
blocks: [ replacementBlock ],
} );

const newWrapperBlockCacheKey = state.cache[ wrapperBlock.clientId ];

expect( newWrapperBlockCacheKey ).not.toBe( originalWrapperBlockCacheKey );

expect( state.order ).toEqual( {
'': [ wrapperBlock.clientId ],
[ wrapperBlock.clientId ]: [ replacementBlock.clientId ],
[ replacementBlock.clientId ]: [],
} );

expect( state.parents ).toEqual( {
[ wrapperBlock.clientId ]: '',
[ replacementBlock.clientId ]: wrapperBlock.clientId,
} );

expect( state.cache ).toEqual( {
[ wrapperBlock.clientId ]: {},
[ replacementBlock.clientId ]: {},
Expand Down