-
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
Site Editor: Make Code Editor reflect block conversions #42081
Site Editor: Make Code Editor reflect block conversions #42081
Conversation
Size Change: +22 B (0%) Total Size: 1.25 MB
ℹ️ View Unchanged
|
Yeah, I think this behavior is normal. Maybe it should be different, but if you try e.g. #40506 Fortunately, it’s not too much of a problem — we expect the un-converted block to continue to work, so if the user doesn’t end up making any changes to the template (or post) and thus cannot save it, it will remain the same on the frontend (which should continue to work) — basically until the user finally makes some change and saves the updated template (or post). FWIW, on a technical level, I believe this happens because we're not creating a new undo level upon block parsing (from where deprecations are run); I think the relevant LOC is this:
|
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.
LGTM! 💪
It would be nice to get some feedback from @jorgefilipecosta. |
? contentStructure( { blocks } ) | ||
: contentStructure; | ||
|
||
// Replicates the logic found in getEditedPostContent(). |
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.
Specifically:
gutenberg/packages/editor/src/store/selectors.js
Lines 895 to 901 in 6465552
if ( typeof record.content === 'function' ) { | |
return record.content( record ); | |
} else if ( record.blocks ) { | |
return __unstableSerializeAndClean( record.blocks ); | |
} else if ( record.content ) { | |
return record.content; | |
} |
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.
It would be nice to get some feedback from @jorgefilipecosta.
The change looks good to me 👍
What?
Fix #41944 (and potentially unblock #40506).
Why?
If a block is undergoing a legacy block conversion upon loading in the Site Editor, and the user then switches to the Code Editor, that conversion is not reflected there. See #41944 for details.
(This might also affect block deprecations.)
How?
By using the readily parsed (and thus, converted) blocks as the source of truth for the Code Editor, rather than the
content
state.Testing Instructions
This requires a bit of fiddling to test, since in order to run a legacy block conversion, we need a template that includes a legacy block. Try the following:
First of all, verify that the problem exists on
trunk
by using the instructions from #41944.Now, verify that this PR solves the problem:
wp:comments
), save the template, and switch back totrunk
.wp:comments-query-loop
.update/rename-comments-query-loop
branch.wp:comments
there ✅