Fix pasting content into inner blocks#16717
Merged
Merged
Conversation
…the correct parent blocks
…he same as the new client id. Update tests to ensure wrapper generates new key
talldan
commented
Jul 23, 2019
| getBlocksWithParentsClientIds( keys( flattenBlocks( action.blocks ) ) ), | ||
| ), | ||
| ...omit( parentClientIds, action.replacedClientIds ), | ||
| ...fillKeysWithEmptyObject( keys( flattenBlocks( action.blocks ) ) ), |
Contributor
Author
There was a problem hiding this comment.
To explain a bit what I've done here:
- The
replacedClientIdsneeds to be used to find parentClientIds, but thenreplacedClientIdsalso need to be omitted, hence the second call toomithere. - 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.
Contributor
There was a problem hiding this comment.
The logic seems good to me.
tellthemachines
approved these changes
Jul 26, 2019
Contributor
tellthemachines
left a comment
There was a problem hiding this comment.
Tested with columns and nested columns, by pasting content in, saving and previewing, and it's working fine.
Code looks good, thanks for documenting everything so thoroughly!
A thought about the reducer unit tests in general: I wonder if we shouldn't be more granular with what we're testing. There's a huge amount of assertions inside some of those tests, so if they fail, it's not immediately obvious why. E.g. should we have a test to check caching in block replacement, instead of having that check be part of the block replacement test?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #16645
Description
Resolves an issue where content pasted into inner blocks is not saved correctly.
The issue was caused by a small caching error introduced in #16407.
When pasting content into an empty paragraph, a block replacement is triggered. When this action was being carried out in an inner block, cached parent blocks were not being invalidated.
This is the code that attempts to invalidate parents of a replaced block:
https://github.com/WordPress/gutenberg/blob/master/packages/block-editor/src/store/reducer.js#L269-L271
The issue is that
getBlocksWithParentsClientIdsis called with the incorrect clientId.getBlocksWithParentsClientIdsoperates on the old state rather than the new state, so it needs to be called with the block that's about to be removed rather than the block that's about to be added: https://github.com/WordPress/gutenberg/blob/master/packages/block-editor/src/store/reducer.js#L228-L237This PR resolves the issue.
How has this been tested?
Updated the relevant unit test to ensure the parent block is invalidated.
Manual testing:
Types of changes
Bug fix (non-breaking change which fixes an issue)
Checklist: