REST API: Simplify 'default_rendering_mode' field registration for post types #67867
+42
−66
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
Mamaduka
added
REST API Interaction
|
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. |
Mamaduka
commented
Dec 12, 2024
lib/compat/wordpress-6.8/class-gutenberg-rest-post-types-controller-6-8.php
Outdated
Show resolved
Hide resolved
swissspidy
reviewed
Dec 12, 2024
| */ | ||
| $rendering_mode = apply_filters( "post_type_{$item->name}_default_rendering_mode", $rendering_mode, $item ); | ||
| // Property will only exist if the post type supports the block editor. | ||
| if ( ! $post_type_object || ! isset( $post_type_object->default_rendering_mode ) ) { |
There was a problem hiding this comment.
Still not a fan of a) artificially adding properties to the WP_Post_Type and b) the name
There was a problem hiding this comment.
What do you mean artificially? This is the only way to expose new properties in response.
Agreed on the name, I'll create a new discussion for it + if we should migrate to supports. This PR here is just refactoring.
There was a problem hiding this comment.
What do you mean
artificially? This is the only way to expose new properties in response.
When I worked on WP_Post_Type it wasn't really the intention to allow passing non-default fields to post type registration.
There was a problem hiding this comment.
I like your posed idea in the original PR to use post type supports - 'editor-rendering-mode' => 'post-only', and I guess we can make that change here.
It will be a conditional support value, like the autosave you recently implemented.
@fabiankaegy, @TylerB24890, @youknowriad, what do you think?
Edit: I think the key difference compared to other support values is that you can't really remove it. Removal would mean fallback to default rendering mode - post-only.
Mamaduka
commented
Dec 12, 2024
| return { | ||
| hasLoadedPostObject: _hasLoadedPostObject, | ||
| editorSettings: getEditorSettings(), | ||
| isReady: __unstableIsEditorReady(), | ||
| mode: getRenderingMode(), | ||
| defaultMode: | ||
| postTypeObject?.default_rendering_mode ?? 'post-only', | ||
| postTypeObject?.default_rendering_mode || 'post-only', |
There was a problem hiding this comment.
Just extra hardening to provide correct fallback on falsy values like empty string
|
Size Change: -44 B (0%) Total Size: 1.84 MB
ℹ️ View Unchanged
|
Mamaduka
force-pushed
the
update/rest-post-type-rendering-mode
branch
from
3f047eb to
547a7cc
Compare
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?
A follow-up to #62304.
Simplifies the registration of the
default_rendering_modepost-type REST property by adding it viaregister_rest_fieldinstead of a controller override. It also adds a schema for the new property.I've removed the new filters introduced in the original PR. Why:
default_rendering_modefor REST API responses. This could cause some unexpected results.register_post_type_argsorrest_prepare_post_typefilters.In the future, we could provide API like
add_post_type_support, but first, let's see how these configurations mature.cc @TylerB24890
Testing Instructions
template-lockedmode.Testing Instructions for Keyboard
Same.