Editor: Fix issue where createBlock in block template caused list view collapse#56666
Merged
youknowriad merged 2 commits intotrunkfrom Nov 30, 2023
Conversation
|
Size Change: +7 B (0%) Total Size: 1.72 MB
ℹ️ View Unchanged
|
ramonjd
approved these changes
Nov 30, 2023
Member
ramonjd
left a comment
There was a problem hiding this comment.
This is working well for me.
Before
2023-11-30.16.23.50.mp4
After
2023-11-30.16.22.21.mp4
Also, page editing, template preview toggling and template editing work for pages in the Site Editor as I'd expect 👍🏻
| mode, | ||
| ] ); | ||
| return null; | ||
| }, [ templateBlocks, rootLevelPost, post.type, post.id, mode ] ); |
Member
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?
Contributor
Author
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.
Contributor
Author
|
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 🙂 |
Contributor
|
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? |
Contributor
|
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 |
Contributor
|
Feel free to revert my commit if you don't like it :) |
Contributor
|
Merging this PR, as I'm opening a follow-up based on it. |
Contributor
Author
|
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 |
vcanales
pushed a commit
that referenced
this pull request
Dec 5, 2023
…w collapse (#56666) Co-authored-by: Riad Benguella <benguella@gmail.com>
Member
|
Backported to 17.2 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
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.
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