-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Edit Site: Fix Template Part Auto-draft creation #23050
Conversation
Size Change: 0 B Total Size: 1.13 MB ℹ️ View Unchanged
|
Restarting the failing Travis CI job. |
Still failing. It's the Build Artifacts job, I'll investigate. |
@ockham there's a convo in core-editor as well https://wordpress.slack.com/archives/C02QB2JS7/p1591791354355800 |
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.
All template parts are showing (and saving) correctly for the 3 themes I tested.
Filed #23052 to hopefully fix the issue with the Build Artifacts job 🤞 |
- Edit Site: fix template lookup. #22954 - Fix for FSE template parts: #23050 - Fix link color style rule. #23025 - Fix for link color: it needs to be opt-in. #23049 - Revert "Image Block: add caption field to placeholder" #23027 - Cover padding: reset button + hook namespace + improve visualizer #23041 - Fix failing 'Build artifacts' CI job (by updating `package-lock.json`): #23052
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 for the quick fix!
if ( 'core/template-part' !== $block['blockName'] ) { | ||
return array(); | ||
} | ||
$template_part_ids = array(); |
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.
This doesn't need to be an array.
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.
What's the alternative? 🤔 It seemed like the most straight-forward way to allow for the inner blocks recursion to conditionally extend it.
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.
I think I just expected it to be added to the array at the end instead of declaring itself as an array, but I can see how that can be more complicated in PHP.
Description
This fixes an issue introduced by yours truly in #22143, and first reported by @carolinan in Slack:
The problem was that we were bailing early from
create_auto_draft_for_template_part_block
if the block type was notcore/template-part
. But this means that we also didn't descend into recursion of a block's inner blocks, and neglected e.g. the template part block children of agroup
block.This probably went unnoticed for a while because many of us still had some auto-generated template part auto-drafts lingering from previous runs of the site editor.
Diff best viewed with whitespace changes ignored.
Thanks @carolinan for reporting, and @nosolosw for bringing this to my attention!
How has this been tested?
wp_template
CPTs (neither published norauto-draft
s) from previous Site Editor runs. (It's best to start with a fresh install, i.e.npx wp-env clean all && npx wp-env start
. Careful, this will wipe your WordPress install's data!)/wp-admin/admin.php?page=gutenberg-experiments
must not be ticked. (Make sure the "Full Site Editing" checkbox is ticked -- since it also gets reset after a wipe.)theme-experiments
repo (see thewp-env
README).Screenshots
Before
After
Types of changes
Bug fix
Checklist: