-
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
Fix: Post_type template is not used when creating a page in site editor. #62488
Fix: Post_type template is not used when creating a page in site editor. #62488
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress. If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged. If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack. Thank you! ❤️ View changed files❔ lib/rest-api.php |
Size Change: +57 B (0%) Total Size: 1.76 MB
ℹ️ View Unchanged
|
lib/rest-api.php
Outdated
if ( ! empty( $post_type ) && ! empty( $post_type->template_lock ) ) { | ||
return $post_type->template_lock; | ||
} | ||
return null; |
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.
According to this other logic, it should return false
.
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.
Hi @oandregal, setting a value of false is the same as not having template_lock, I guess we can ignore the false values, and return null/not return when locking is not set either because there is no locking referred at all or because it is false. I don't see any downside to that and this would allow us to use an enum as a type with only array( 'all', 'insert', 'contentOnly' ). We can not really have a type that is enum or boolean. What do you think?
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 about nested content? Is there a difference between false
or null
in that scenario? I'm thinking about the locking lookup: if I remember this correctly, if a block (wp_navigation
or wp_block
types, for example) doesn't have a lock, we look it up above in the parents. I don't remember the details of "does not have a lock": was that checking for false
or for null
/non existence (or both)?
Other than double checking that scenario, the current approach makes sense to me.
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.
The nested locking is something at the block level, this template lock is only used now on edit-post, and the template locks of CPTs can not be nested. The locking of templates on CPTs is always top level there is never a need to overite with false as may happen on the block level. So I think we are safe not returning the false value at all.
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.
Ah, that sounds good. Thanks for the braindump.
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've left a comment about template_lock
valid values we need to address. Other than that, this looks good and it's working so I'm pre-approving to speed up the merge process given the time-constrains (beta period for 6.6).
Co-authored-by: André <583546+oandregal@users.noreply.github.com>
22a9f0a
to
f51bceb
Compare
); | ||
} | ||
} | ||
add_action( 'rest_api_init', 'gutenberg_register_wp_rest_post_types_controller_fields' ); |
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.
Should we be worried about changes to the REST API this late in the release cycle? This PR will need a backport PR asap.
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 are only adding two fields that will for sure be required, so I don't think we have back-compatibility problems or unexpected issues. I'm preparing a backport PR, but I don't have a strong option. we can opt for not include this fix in WP 6.6 if you think it is safer.
}, | ||
'schema' => array( | ||
'type' => 'array', | ||
'description' => __( 'The template associated with the post type.', 'gutenberg' ), |
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.
Should the phrasing reflect that the template is optional and refers to block templates as opposed to a wp_template
? For example:
-The template associated with the post type.
+The block template associated with the post type, if it exists.
…or. (#62488) Co-authored-by: jorgefilipecosta <jorgefilipecosta@git.wordpress.org> Co-authored-by: oandregal <oandregal@git.wordpress.org> Co-authored-by: ellatrix <ellatrix@git.wordpress.org> Co-authored-by: colorful-tones <colorful-tones@git.wordpress.org> Co-authored-by: fabiankaegy <fabiankaegy@git.wordpress.org> Co-authored-by: bacoords <bacoords@git.wordpress.org>
Manually resolved this in 5afddc8 |
…or. (#62488) Co-authored-by: jorgefilipecosta <jorgefilipecosta@git.wordpress.org> Co-authored-by: oandregal <oandregal@git.wordpress.org> Co-authored-by: ellatrix <ellatrix@git.wordpress.org> Co-authored-by: colorful-tones <colorful-tones@git.wordpress.org> Co-authored-by: fabiankaegy <fabiankaegy@git.wordpress.org> Co-authored-by: bacoords <bacoords@git.wordpress.org>
Fixes: #60710
The site editor now allows to create a new page but if the page post type has a template assigned to it the template is ignored.
This PR fixes the issue.
To fix the issue, the Rest API has been extended for the types endpoint. Two new fields were added template and template_lock.
Testing Instructions
I added the following test code to scrip-loader.php.
I created a new page on the site editor and verified the template was used as expected.