Editor: Fix issue where createBlock in block template caused list view collapse #56666
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
andrewserong
added
[Type] Regression
andrewserong
requested review from
youknowriad,
apeatling,
ramonjd and
tellthemachines
|
Size Change: +7 B (0%) Total Size: 1.72 MB
ℹ️ View Unchanged
|
ramonjd
approved these changes
Nov 30, 2023
| mode, | ||
| ] ); | ||
| return null; | ||
| }, [ templateBlocks, rootLevelPost, post.type, post.id, mode ] ); |
There was a problem hiding this comment.
Just checking, postBlocks was kind of redundant anyway given that post.type and post.id were used to fetch it?
There was a problem hiding this comment.
Pretty much — postBlocks was only in the dependency array because it was returned as a fallback value, so it wasn't being used for anything. By moving it out of the dependency array, it shouldn't really change anything other than reducing the number of times that the useMemo gets called unnecessarily.
|
Thanks for the quick review @ramonjd! I'll leave this PR open overnight in case @youknowriad had other ideas or concerns about this fix, but if not, I'll merge this in tomorrow 🙂 |
|
Yes, I noticed this, I had included a fix in my post editor refactor PR that is slightly different than this one. Mind if I push it here? |
|
It's a little bit different to account for more overrides in the future potentially and adds a comment. I'd love an approval for this #56542 too, it included this fix |
|
Feel free to revert my commit if you don't like it :) |
|
Merging this PR, as I'm opening a follow-up based on it. |
youknowriad
deleted the
fix/list-view-collapse-when-changing-navigation-or-content-blocks
branch
|
Nice, that's a bit clearer @youknowriad, and helps ensure each template is isolated to only those changes that mean it should be regenerated. Thanks for landing this! 👍 I think we now have an unnecessary comment below the |
andrewserong
added
the
Backport to Gutenberg RC
vcanales
pushed a commit
that referenced
this pull request
Dec 5, 2023
…w collapse (#56666) Co-authored-by: Riad Benguella <benguella@gmail.com>
|
Backported to 17.2 RC |
vcanales
removed
the
Backport to Gutenberg RC
derekblank
pushed a commit
that referenced
this pull request
Dec 7, 2023
…w collapse (#56666) Co-authored-by: Riad Benguella <benguella@gmail.com>
cbravobernal
pushed a commit
that referenced
this pull request
Dec 7, 2023
…w collapse (#56666) Co-authored-by: Riad Benguella <benguella@gmail.com>
25 tasks
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.
What?
Fixes #56662, related to #56000
This fixes an issue when editing Navigation entities in the site editor, or when editing content within the post-only mode — in both cases, rearranging blocks will cause the list view to collapse on
trunk.Why?
Within
useBlockEditorProps,createBlockwas called too frequently (every time a change was made inpostBlocks), resulting in a newclientIdfor the wrapping Navigation block, or the wrapping Post Content block in the Navigation editor or the post-only mode, respectively.How?
By updating the
useMemoto removepostBlocksfrom the dependency array, and moving the fallback topostBlocksto outside of theuseMemo, we ensure thatcreateBlockis only called when switching a mode, template, or post. This means that theclientIdof the Navigation or Post Content blocks should now be stable when editing within the Navigation editor in the site editor, or in post only mode.Testing Instructions
Screenshots or screencast
Before (Navigation)
2023-11-30.11.36.43.mp4
After (Navigation)
2023-11-30.15.57.17.mp4
Before (editing Page content)
Notice how rearranging paragraph blocks within Content unexpectedly collapses the Content block:
2023-11-30.16.01.46.mp4
After (editing Page content)
2023-11-30.16.00.17.mp4