-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Conversation
@@ -637,70 +594,17 @@ const withInnerBlocksRemoveCascade = ( reducer ) => ( state, action ) => { | |||
*/ | |||
const withBlockReset = ( reducer ) => ( state, action ) => { |
There was a problem hiding this comment.
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.
Size Change: -151 B (0%) Total Size: 1.11 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again, Riad.
This fix works as expected.
Unrelated, but do you know why we did that? |
yes, we didn't have any "core" store, and it was just the initial naive implementation of reusable blocks. It went through several iterations since. |
@youknowriad It looks like we have slight regression after this PR - #37361. |
closes #36937
A long time ago, we used to keep the reusable blocks parsed content inside the "block-editor" store even if these were not rendered. To be able to do so, we needed to keep the blocks of these reusable blocks in state when
RESET_BLOCKS
action was called as it only concerns the "main" block list.That behavior though is a bit problematic now when we switch to template part focus mode because when we do so, a block that was "controlled" become top-level in the store so "uncontrolled", and when we come back to the old state, the "controlled" block was kept in the state but not its children, so it seemed like the content disappeared because
getBlock( id )
of these child blocks was returningnull
.This PR removes that old RESET_BLOCKS` behavior because I believe it's probably useless nowadays and potentially a source for memory leaks. I'm not certain yet that I didn't miss any hidden use case for that behavior but let's see what e2e tests say.