gutenberg icon indicating copy to clipboard operation
gutenberg copied to clipboard

Feature: Set editor rendering mode by post type

Open TylerB24890 opened this issue 1 year ago • 11 comments

What?

Related Issue & PR:

  • https://github.com/WordPress/gutenberg/issues/58038
  • https://github.com/WordPress/gutenberg/pull/61844

This PR adds a filterable default_rendering_mode property to the WP_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 via register_post_type() and can be overwritten using the available filters:

  • post_type_{$post_type}_default_rendering_mode: To target an individual specific post type.
/**
 * Filters the block editor rendering for a specific post type.
 *
 * The dynamic portion of the hook name, `$post_type->name`, refers to the post type slug.
 *
 * @param string       $rendering_mode The current rendering mode set with the post type registration.
 * @param WP_Post_Type $post_type      The current post type object.
 */
$rendering_mode = apply_filters( "post_type_{$post_type->name}_default_rendering_mode", $post_type->default_rendering_mode, $post_type );
  • post_type_default_rendering_mode: To target all (or multiple) post types.
/**
 * Filters the block editor rendering for a post type.
 *
 * @param string       $rendering_mode  The current rendering mode set with the post type registration.
 * @param WP_Post_Type $post_type_name  The current post type object.
 */
$rendering_mode = apply_filters( 'post_type_default_rendering_mode', $post_type->default_rendering_mode, $post_type );

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 the WP_Post_Type object itself and provides multiple ways of overriding or setting the default for a custom (or core) post type.

Testing Instructions

  1. Create a new post post type and observe the default editor UI.
  2. In your functions.php file (or similar) use one of the available filters to set the default_rendering_mode property to template-lock:
add_filter(
    'post_type_default_rendering_mode',
    function () {
        return 'template-lock';
    }
);
  1. Refresh the post editor and confirm you are now seeing the Template UI instead of the default Post UI.
  2. Create a new page post and confirm the page editor also loads with the Template UI.
  3. Update the filter in functions.php to target only the page post type:
add_filter(
    'post_type_page_default_rendering_mode',
    function () {
        return 'template-lock';
    }
);

  1. Reload the page editor and confirm it still renders the Template UI.
  2. Refresh the post editor and confirm it now renders the default Post UI.
  3. Update the filter in functions.php to set the rendering mode for the post and page post types, but no others:
add_filter(
    'post_type_default_rendering_mode',
    function ( $mode, $post_type ) {
        if ( 'page' === $post_type || 'post' === $post_type ) {
            return 'template-lock';
        }

        return $mode;
    },
    10,
    2
)
  1. Register a custom post type using register_post_type and set the default_rendering_mode parameter to template-lock:
register_post_type(
    'my_custom_type',
    [
        'show_in_rest'           => true,
        'supports'               => [ 'title', 'editor' ],
        'default_rendering_mode' => 'template-lock',
    ]
);
  1. Visit the editor for your custom post type and confirm it loads with the Template UI.
  2. Visit the Site Editor and select Pages in the left navigation.
  3. Select a page and confirm it loads with the Template view (if the code from step 8 is still in place).
  4. Change the filter value from step 8 to output post-only and refresh the Site Editor.
  5. Confirm the Page now loads with the Post Content instead of the template UI.

TylerB24890 avatar Jun 04 '24 20:06 TylerB24890

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 props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: TylerB24890 <[email protected]>
Co-authored-by: Sidsector9 <[email protected]>
Co-authored-by: fabiankaegy <[email protected]>
Co-authored-by: youknowriad <[email protected]>
Co-authored-by: dcalhoun <[email protected]>
Co-authored-by: ramonjd <[email protected]>
Co-authored-by: mcsf <[email protected]>
Co-authored-by: jasmussen <[email protected]>
Co-authored-by: annezazu <[email protected]>
Co-authored-by: jameskoster <[email protected]>
Co-authored-by: dinhtungdu <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

github-actions[bot] avatar Jun 05 '24 14:06 github-actions[bot]

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:

showing template

Otherwise it still shows the current default:

not showing template

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 avatar Jun 06 '24 07:06 jasmussen

@jasmussen Agree that there should also be a user setting for this :) We can look into that as follow up 👍

fabiankaegy avatar Jun 06 '24 08:06 fabiankaegy

Not necessarily urgent, IMO, just something to consider for the technical review!

jasmussen avatar Jun 06 '24 08:06 jasmussen

Any thoughts on making "template-locked" default for pages for both post and site editor @jasmussen ?

youknowriad avatar Jun 06 '24 08:06 youknowriad

To make sure I read you right, you mean by default show the full template when editing pages, regardless of whether you're editing inside the site editor, or in the separate editor that's accessed from the top level Pages > Add new?

This seems something we most likely want, absolutely. As a 6.7 thing it feels reasonable to test this early.

jasmussen avatar Jun 06 '24 09:06 jasmussen

Great. Thanks for the confirmation.

In that case, I think we should move the code here https://github.com/WordPress/gutenberg/pull/62304#discussion_r1627866620 directly to the "editor" package somehow. edit-post and edit-site shouldn't have to pass the default rendering mode anymore.

youknowriad avatar Jun 06 '24 09:06 youknowriad

Great. Thanks for the confirmation.

In that case, I think we should move the code here #62304 (comment) directly to the "editor" package somehow. edit-post and edit-site shouldn't have to pass the default rendering mode anymore.

@youknowriad This has been moved to the editor package. Please take a look and let me know if this is the approach you were thinking of. Adding this here allowed me to remove the various mode props passed to the site-editor and post-editor, so now it's all defined in the main editor package. Confirmed using the filters will set the default for both the site and post editors as expected.

I have not applied the template-lock mode to the Page post type by default. If that's something we do want to do I can do so.

@mcsf

Why the difference in signatures between the two hooks? I see one takes a WP_Post_Type instance, the other takes the type's name.

I've consolidated these to be the same. Both filters will now pass the full WP_Post_Type object.

Would amending the hook name from {$post_type->name}default_rendering_mode to post_type{$post_type->name}_default_rendering_mode help developers infer and trace it back to its origin in the future? Otherwise I worry that certain post type names could make things confusing or make it sound like it's a third-party hook (e.g. woo_product_default_rendering_mode)

This has been updated to "post_type_${post_type->name}_default_rendering_mode"

TylerB24890 avatar Jun 06 '24 17:06 TylerB24890

I've consolidated these to be the same. Both filters will now pass the full WP_Post_Type object.

Ah, jinx!

mcsf avatar Jun 06 '24 17:06 mcsf

@WordPress/outreach for anyone who can help test and review this PR ❤️

annezazu avatar Jun 12 '24 23:06 annezazu

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.

youknowriad avatar Jun 28 '24 16:06 youknowriad

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!

ramonjd avatar Jul 23 '24 02:07 ramonjd

@youknowriad Thanks to the help of @Sidsector9 we finally figured out the failing tests 🚀

We'll work on the back port PR on Friday to get that added also :)

Let us know if there is anything else of if this is good to be merged

fabiankaegy avatar Jul 30 '24 19:07 fabiankaegy

All good for me, I think there's probably a remaining failure on the performance tests it seems. the subtlety for the performance test is that the same test should be able to run on two different environments (the PR environment and the trunk environment) to be able to compare.

youknowriad avatar Jul 31 '24 07:07 youknowriad

Backport PR created w/ Trac ticket.

Also updated the order of the post_type_rendering_mode filters so the post_type_${post_type}_* filter will apply after the generic filter. cc @fabiankaegy

TylerB24890 avatar Aug 02 '24 19:08 TylerB24890

Would love to see this PR move forward and land. @fabiankaegy Is that something you can help with? How can I help personally?

youknowriad avatar Sep 26 '24 09:09 youknowriad

@youknowriad We've been trying to move it forward but failed on how to best deal with the performance tests...

Most of my contributions right now are focussed on the 6.7 documentation efforts. But I can work with @TylerB24890 to get the PR rebased / updated again and if you have any insights on how we can deal with the performance test that would be incredibly helpful ❤️

fabiankaegy avatar Sep 26 '24 09:09 fabiankaegy

Based on the screenshots in the performance test failures, it seems like the the performance tests are expecting the page to be in "template-locked" (access the post content block) but for some reason, we're on "post-only" mode there.

youknowriad avatar Sep 26 '24 10:09 youknowriad

I'm also still very confused what is happening with the mobile unit tests 🤔

fabiankaegy avatar Nov 14 '24 23:11 fabiankaegy

For the mobile unit tests, what's happening is that getPostType calls don't return anything at the moment in the unit test, so the editor doesn't initialize (doesn't render the block list). The setup of these unit-tests is a bit complex so I wasn't able to fix it. We need to either mock the REST API or mock the selector call I believe. Maybe @dcalhoun could help here.

youknowriad avatar Nov 15 '24 09:11 youknowriad

I also just noticed a bug that I probably introduced yesterday in my rebase. For the posts post type (or likely and post type that doesn't have the preview enabled by default) the template doesn't seem to get resolved at all.

My guess is that there is some early return somewhere if the post type doesn't have the default rendering mode set to template-locked

CleanShot 2024-11-15 at 11 12 17@2x

fabiankaegy avatar Nov 15 '24 10:11 fabiankaegy

I also just noticed a bug that I probably introduced yesterday in my rebase. For the posts post type (or likely and post type that doesn't have the preview enabled by default) the template doesn't seem to get resolved at all.

My guess is that there is some early return somewhere if the post type doesn't have the default rendering mode set to template-locked

CleanShot 2024-11-15 at 11 12 17@2x

This has been fixed in https://github.com/WordPress/gutenberg/pull/62304/commits/a742e4a8c4c0f9c2f8368924967210de680df95a

fabiankaegy avatar Nov 15 '24 10:11 fabiankaegy

For the mobile unit tests, what's happening is that getPostType calls don't return anything at the moment in the unit test, so the editor doesn't initialize (doesn't render the block list). The setup of these unit-tests is a bit complex so I wasn't able to fix it. We need to either mock the REST API or mock the selector call I believe. Maybe @dcalhoun could help here.

@youknowriad this appears to be a legitimate bug in the mobile editor, not an issue in the unit test setup. The mode is always undefined, leading to the editor provider always rendering null.

https://github.com/WordPress/gutenberg/blob/a742e4a8c4c0f9c2f8368924967210de680df95a/packages/editor/src/components/provider/index.js#L327-L330

I observed that, in the mobile editor, the renderingMode reducer never runs even though the setRenderingMode action is triggered in the provider's effect—the rendering mode is never set the defaultMode value. Do have any thoughts as to why this may be the case?

dcalhoun avatar Nov 15 '24 13:11 dcalhoun

@dcalhoun As you can see here, we have this code in the editor provider

const postTypeObject = select( coreStore ).getPostType(
					post.type
				);

This code performs a REST API to fetch the post type with the rendering mode. I don't know how to test the app personally but at least in the mobile unit tests, that object was always "undefined" which means, nothing was rendered.

youknowriad avatar Nov 15 '24 14:11 youknowriad

I don't know how to test the app personally but at least in the mobile unit tests, that object was always "undefined" which means, nothing was rendered.

@youknowriad correct, but if that object is undefined, the rendering mode should fallback to the default—post-only. Currently it is not, both in the mobile editor and its automated tests.

https://github.com/WordPress/gutenberg/blob/a742e4a8c4c0f9c2f8368924967210de680df95a/packages/editor/src/components/provider/index.js#L197-L198

I will continue debugging, but wanted to ask if you had insight on that oddity.

dcalhoun avatar Nov 15 '24 14:11 dcalhoun

In case it helps, There's this condition

if ( ! isReady || ! mode || ! hasLoadedPostObject ) {

This condition means that we don't render anything if the object didn't load yet, regardless of whether there's a fallback rendering mode.

youknowriad avatar Nov 15 '24 14:11 youknowriad

@youknowriad @TylerB24890 the mobile editor breakage and failing automated tests should be resolved by the following:

  • https://github.com/TylerB24890/gutenberg/pull/2
  • https://github.com/TylerB24890/gutenberg/pull/3

The former is merged, the latter needs review and merging. 🚀

dcalhoun avatar Nov 15 '24 21:11 dcalhoun

Okay now we just have to figure out how to fix the performance tests 😅

fabiankaegy avatar Nov 15 '24 22:11 fabiankaegy

@youknowriad in my digging it appears that this line here when we load the sample HTML file for the large post

https://github.com/WordPress/gutenberg/blob/ec543c5939e8459680c948c27f78221f3f95d933/test/performance/fixtures/perf-utils.ts#L148

That doesn't update the blocks that are stored in the post content but rather replaces the blocks in the template itself 🤔

I have added a workarround that switches the rendering mode to post-only before it calls the replaceBlocks dispatch function. It works as expected now but I'm not sure if there isn't a better way ti improve the tests

fabiankaegy avatar Nov 15 '24 22:11 fabiankaegy

This seems ready to go, If I'm not wrong the main impact of this PR is that pages are going to show templates in the post editor by default as well.

So just a final heads-up before merge @jasmussen

Also can we do a rebase here to ensure that are no regressions/hidden issues with the latests updates in trunk?

youknowriad avatar Nov 19 '24 08:11 youknowriad