Fix content loss when ungrouping template parts or reusable blocks #37280
Conversation
This file contains 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
youknowriad
added
[Type] Bug
|
Size Change: +463 B (0%) Total Size: 1.11 MB
ℹ️ View Unchanged
|
Mamaduka
approved these changes
Dec 10, 2021
Is this a reason why my example fix was failing? We also have another issue with controlled inner blocks and now I'm curious if this might be related - #36937. |
7 tasks
Mamaduka
reviewed
Dec 10, 2021
|
It's interesting that with this PR, if we group and then ungroup, we don't have the disappearing issue(#36937) |
|
@ntsekouras Do you mean if we group/ungroup before accessing focus mode or inside the focus mode? |
Before the isolated editing view |
|
@ntsekouras potential fix for the related issue here #37283 |
|
Maybe we should add e2e tests that cover the grouping/ungrouping of inner block controllers. I've to call it a day soon, but happy to create a new test on Monday. What do you think? |
Co-authored-by: George Mamadashvili <[email protected]>
Works for me. Thanks @Mamaduka |
noisysocks
removed
the
Backport to WP 6.7 Beta/RC
noisysocks
pushed a commit
that referenced
this pull request
Dec 14, 2021
…37280) Co-authored-by: George Mamadashvili <[email protected]> Co-authored-by: ntsekouras <[email protected]>
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.
Closes #36788
Alternative to #37274
There are two low level bugs that create this issue:
onChangehandler fromuseBlockSyncis called. Ideally it shouldn't be because that component will be unmounted a few milliseconds later. This is hard to solve because the store subscriptions run from the deepest component to the top level ones. Basically, it's another kind of redux zombie bug. I'm not sure we can solve that part of the bug.blocksshouldn't be emptied in the first place when "moving" the block to another position, this happens because controlled blocks are supposed to retrieve their block again from the "core" store when they remount again but since onChange is called (first bug above) the value in "core" store is wrong (empty blocks). I'm fixing this part of the bug in this PR: I'm making sure if a controlled block moves to a new position, it retains its inner blocks from the previous position.