Fix Nav block editing wrong entity on creation of new Menu#36880
Merged
Fix Nav block editing wrong entity on creation of new Menu#36880
Conversation
|
Size Change: -21 B (0%) Total Size: 1.1 MB
ℹ️ View Unchanged
|
This was referenced Nov 25, 2021
talldan
reviewed
Nov 26, 2021
259db3b to
93246ac
Compare
Contributor
Author
|
Failing e2e test appears to be around dirty entities which worries me. I'll run locally to check... |
Contributor
Author
I think we've introduced a bug here with dirty entity states as the test is failing because the |
Contributor
|
I added an additional check for |
getdave
commented
Nov 26, 2021
| } | ||
| // innerBlocks are intentionally not listed as deps. This function is only concerned | ||
| // with the snapshot from the time when ref became undefined. | ||
| }, [ clientId, ref ] ); |
Contributor
Author
There was a problem hiding this comment.
Suggested change
| }, [ clientId, ref ] ); | |
| }, [ clientId, ref, innerBlocks ] ); |
Do we now need to add innerBlocks to the deps?
Contributor
There was a problem hiding this comment.
No, as the comment above states :-)
Contributor
There was a problem hiding this comment.
Oops, looks like it was added anyway in 793ff2a
Contributor
Author
There was a problem hiding this comment.
I don't even know what happened here 😕
Contributor
Author
|
@adamziel this now looks good. e2e passes. Needs a 👍 pending one final query in #36880 (comment) |
adamziel
approved these changes
Nov 26, 2021
noisysocks
pushed a commit
that referenced
this pull request
Nov 29, 2021
* Move reset of innerblocks to effect * Add e2e test that checks for updating the wrong entity record * Formatting * Unfocus focused test * Only replace innerBlocks with an empty list, if it's not already empty * Update useEffect dependencies Co-authored-by: Adam Zieliński <adam@adamziel.com> Co-authored-by: Daniel Richards <daniel.richards@automattic.com>
talldan
reviewed
Dec 14, 2021
This was referenced Dec 14, 2021
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
4 participants
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.
Description
In #36862 we learnt that if you use the
Create newbuttons within the Nav block UI to create a new Menu you can end up causing edits to the wrongwp_navigationentity.How does this happen? Scenario:
Create newdrop theSelect menudropdown on the blockwp_navigationentity for editing.replaceInnerBlocksbut it does this before the entity is set to the freshly created one. Instead it ends up calling replaceBlocks and the existing entity picks up the changes and gets marked as "dirty"Savein the Site Editor only to find they now have to save both the new and the original Nav Post entitiesThis PR fixes that by moving the reseting of the inner blocks to an effect which runs if the
refto thewp_navigationPost is reset. This way we ensure we have completely "unloaded" the current entity before we reset the blocks in the UI.Closes #36862
How has this been tested?
Select menu -> Create new.Screenshots
Types of changes
Bug fix (non-breaking change which fixes an issue)
Checklist:
*.native.jsfiles for terms that need renaming or removal).