-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Feature: Set editor rendering mode by post type #62304
base: trunk
Are you sure you want to change the base?
Feature: Set editor rendering mode by post type #62304
Conversation
…deringMode value into the Editor rendering function.
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 you work on this @TylerB24890 🚀
This is working exactly as I hoped it would. :) I left some minor notes and will request some additional reviews :)
lib/compat/wordpress-6.6/class-gutenberg-rest-post-types-controller-6-6.php
Outdated
Show resolved
Hide resolved
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 is looking very good to me. I'd appreciate broader review as well. @jameskoster @jasmussen @mcsf
…perty to read default_rendering_mode.
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. |
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 is working great for me for what it's worth. Nice addition.
I also checked what would happen if I logged in as an Author with template-locked
enabled - it activates the "Show template" option 👍🏻
Just left a comment about value checking, though even if it's a valid remark, I don't think it should block the first version of this.
Nice PR. Took it for a quick spin. In a very superficial test—this could use broader opinions—this seems to work as intended. If I change the default post-editor rendering mode to show templates, it does: Otherwise it still shows the current default: I think of these defaults as valid user settings to set as well at some point, user settings that are reasonable to persist, so if we add this, it would be good that a future user-setting could also unset whatever a plugin or theme might do. I.e. if my theme sets my page editor to omit the template preview, but I really prefer editing with that, I should be able to override that in my user settings. Make sense? |
@jasmussen Agree that there should also be a user setting for this :) We can look into that as follow up 👍 |
Not necessarily urgent, IMO, just something to consider for the technical review! |
@@ -161,18 +165,30 @@ export const ExperimentalEditorProvider = withRegistryProvider( | |||
BlockEditorProviderComponent = ExperimentalBlockEditorProvider, | |||
__unstableTemplate: template, | |||
} ) => { | |||
const { editorSettings, selection, isReady, mode } = useSelect( | |||
const mode = useSelect( |
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.
Any reason for separating the two useSelect calls?
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.
Nope! I've merged them now 🙂
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.
At the moment, the defaultRenderingMode for pages is "post-only", I guess we should make the update to "template-locked" as discussed above.
}; | ||
}, | ||
[] | ||
[ post ] |
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.
Can we use post.type here, it seems it's the only thing we need.
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 set!
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.
Would love some help testing this properly. (Maybe @fabiankaegy if you find some time) but this is looking good to me.
@WordPress/outreach for anyone who can help test and review this PR ❤️ |
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 is working well for me 🎉
If @fabiankaegy is happy that it satisfies #58038 then I suppose the next step is the accompanying Core patch?
Happy to help test that as well. Cheers!
lib/compat/wordpress-6.7/class-gutenberg-rest-post-types-controller-6-7.php
Outdated
Show resolved
Hide resolved
lib/compat/wordpress-6.7/class-gutenberg-rest-post-types-controller-6-7.php
Outdated
Show resolved
Hide resolved
lib/compat/wordpress-6.7/post.php
Outdated
* | ||
* @return array Array of available rendering modes. | ||
*/ | ||
function gutenberg_rendering_modes() { |
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.
Not a blocker, just a question - I'm assuming that this will be wp_rendering_modes
or rendering_modes
when it eventually makes it to Core.
What about making the name be more specific, e.g., gutenberg_post_type_rendering_modes
or even gutenberg_get_post_type_rendering_modes
?
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.
That's a good question, and I assume you're correct about it being renamed to wp_rendering_modes
in Core.
I think it makes sense to prefix it with post_type_ so the final function would be wp_post_type_rendering_modes()
I'm also not opposed to using get_
in the prefix, if the general that's the general consensus. In the meantime I've updated it to use gutenberg_post_type_rendering_modes()
-- happy to update again if necessary!
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.
For me this is exactly doing what I had in mind and testing perfectly! :)
I'm not sure about the failing tests though. They don't seem to be related but I'm not sure.
@youknowriad would love to get your eyes on that if you have a moment <3
I looked at this earlier today, and it's good to ship to me as well. I think tests are somewhat stable these days, so if restart (or rebasing) doesn't really fix them than there must be a relationship with this PR. I think the switch to "template by default" for pages is probably impactful for e2e tests for instance. |
I've kicked off the tests again. 🤞🏻 🤞🏻 🤞🏻 🤞🏻 The final thing to do is create a Core backport PR. See: https://github.com/WordPress/gutenberg/blob/trunk/backport-changelog/readme.md Getting a draft PR up would be enough, and then I think the CI tests will be happy and the discussion about how to integrate into Core can move there. Thanks a lot! |
What?
Related Issue & PR:
block_editor_settings_all
hook in PHP to allow customizing it #61844This PR adds a filterable
default_rendering_mode
property to theWP_Post_Type
object allowing users to define the default rendering mode for the editor for that post type.The
default_rendering_mode
property can be added to the arguments when registering the post type viaregister_post_type()
and can be overwritten using the available filters:post_type_{$post_type}_default_rendering_mode
: To target an individual specific post type.post_type_default_rendering_mode
: To target all (or multiple) post types.Why?
Currently there is no way to set the default rendering mode of the block editor. You can select the mode while in the block editor, but upon refreshing that mode is reset back to the default
post-only
. With this update developers have more control over the editing experience and provides a means of setting the default view for the block editor.How?
The linked PR has a discussion that mentions this setting should be applied at the post type level, allowing for a difference editing mode per post type. This PR applies the
default_rendering_mode
property to theWP_Post_Type
object itself and provides multiple ways of overriding or setting the default for a custom (or core) post type.Testing Instructions
post
post type and observe the default editor UI.functions.php
file (or similar) use one of the available filters to set thedefault_rendering_mode
property totemplate-lock
:post
editor and confirm you are now seeing the Template UI instead of the default Post UI.page
post and confirm thepage
editor also loads with the Template UI.functions.php
to target only thepage
post type:page
editor and confirm it still renders the Template UI.post
editor and confirm it now renders the default Post UI.functions.php
to set the rendering mode for thepost
andpage
post types, but no others:register_post_type
and set thedefault_rendering_mode
parameter totemplate-lock
:post-only
and refresh the Site Editor.