Editor: Introduce a new dynamic templates mode and add basic post title and content blocks.#17263
Editor: Introduce a new dynamic templates mode and add basic post title and content blocks.#17263epiqueras wants to merge 12 commits into
Conversation
| function render_block_core_post_title() { | ||
| rewind_posts(); | ||
| the_post(); | ||
| return the_title( '<h1>', '</h1>', false ); |
There was a problem hiding this comment.
Shouldn't the heading level be variable? In a singular context, it should be h1. But on an archive context where there is more than one post in the loop, should it not be h2?
There was a problem hiding this comment.
Perhaps this should behave like the Heading block where you can pick from among the heading levels. There could be an “auto” setting which does this, but which you could also override as desired.
There was a problem hiding this comment.
Perhaps this should behave like the Heading block where you can pick from among the heading levels. There could be an “auto” setting which does this, but which you could also override as desired.
Yeah I like that.
This was more of a first step to get something on the screen. We need a major design effort to come up with how these new blocks will behave. See the questions at the end of this overview issue: #17284
| array( | ||
| 'post_type' => 'wp_template', | ||
| 'post_name' => 'single-post', | ||
| 'post_content' => '<!-- wp:post-title /--><!-- wp:post-content /-->', |
There was a problem hiding this comment.
Should the featured image be factored in here?
There was a problem hiding this comment.
If that's something we want in the default "post" post type template, sure.
This shouldn't be in Core or the plugin though. We should provide the framework and UI for users to create new templates, but default templates, if there are any, should be provided by themes.
| $template_query = new WP_Query( | ||
| array( | ||
| 'post_type' => 'wp_template', | ||
| 'name' => 'single-post', |
There was a problem hiding this comment.
Naturally, this logic will need to be fleshed out with looking up the specific template for a given post. For example, if editing a movie post type, it should look for single-movie before single. And if single does not exist, then singular, and so on.
| */ | ||
| function render_block_core_post_content() { | ||
| rewind_posts(); | ||
| the_post(); |
There was a problem hiding this comment.
Should the post not be provided as an argument?
There was a problem hiding this comment.
Not really, this block renders whatever post its context dictates.
We do need to add support for pages with multiple posts/post blocks.
| function render_block_core_post_content() { | ||
| rewind_posts(); | ||
| the_post(); | ||
| return apply_filters( 'the_content', str_replace( ']]>', ']]>', get_the_content() ) ); |
There was a problem hiding this comment.
Should this not receive a wrapping element (like many themes put a div.entry-content around it)?
There was a problem hiding this comment.
Yeah, I've seen that, but I'm not very familiar with everything it's used for.
Is that something we should hardcode or expect themes to add it in a filter?
There was a problem hiding this comment.
I think we would need to align on a wrapping div with a certain class name. I assume based on what I've seen, one of the most common classes used for this is entry-content. An alternative would be to use a typical block class, e.g. wp-block-post-content.
This leads to a broader discussion on how the class naming scheme of Gutenberg should evolve when it comes to entire templates. Is the wp-block-... naming sufficient, should it expand to template blocks as well? Or should we constrain the typical block classes to in-content blocks and define a set of more traditional class names?
Finding a consistent solution here is crucial for themes styling Gutenberg templates.
There was a problem hiding this comment.
We'll probably have to use entry-content for backwards compatibility.
There was a problem hiding this comment.
I think that depends on whether we can get the approach in its entirety to be backward-compatible or not. I think a site-wide block based editing experience will require massive changes in how themes are developed either way. It shouldn't break existing themes, but I think certain features would just not be supported with an old theme.
There was a problem hiding this comment.
Yeah, but keeping it as entry-content wouldn't hurt.
There was a problem hiding this comment.
Yes, any existing class names from Microformats should be preserved, as they relate to semantics. And entry-content is one such example from hAtom. Use of such legacy Microformats should not preclude using Schema.org microdata.
| onInput={ useCallback( () => { | ||
| setContent( ( { blocks: blocksForSerialization = [] } ) => | ||
| serializeBlocks( blocksForSerialization ) | ||
| ); |
There was a problem hiding this comment.
Not a concern with this code, but I'm curious how this would translate to an archive context. It absolutely makes sense to start with the single post context for this exploration, and in that context combining editing of the template blocks and post blocks should work fine. But once we get to a place where there are multiple posts in one page (and the list would change over time), the UI might fill up quite a bit. This could be reduced by some kind of "zoom-in" mode where post content is mostly hidden and can be expanded on click and/or with a link to "edit this specific post" (I believe this was considered before somewhere). Another consideration might be performance - having blocks from let's say 10+ posts (plus the template blocks) in one view could be a lot, especially when assuming most people would only use the archive view for editing the template, but not individual posts.
There was a problem hiding this comment.
Yeah, if the editor is not operating on a single post, the post block can easily infer that from useEntityId returning undefined, and bail out of loading anything, showing a placeholder instead.
| const saveProps = [ 'title', 'slug' ]; | ||
| export default function PostTitleEdit() { | ||
| const [ title, setTitle ] = useEntityProp( 'postType', 'post', 'title' ); | ||
| const [ , setSlug ] = useEntityProp( 'postType', 'post', 'slug' ); |
There was a problem hiding this comment.
The hard-coding of post here is probably just temporary, but we need to figure out a way to do it right, since currently this only works for posts of post type 'post' and causes an error on other post types.
The problem is that the overall current post being edited appears to be overridden by this implementation, causing getCurrentPost() to return the wp_template instead of the actual post being edited. Otherwise we could have used withSelect() to get the current post type, but that wouldn't work.
I'm a bit wary of overriding the current post context in Gutenberg to become a wp_template post when actually editing a "real" post, since it could break expectations by other plugins. We should probably think about a way we expose the current wp_template through a set of new API functions (e.g. getCurrentTemplate() etc.), where we could transform the editor accordingly based on whether these include a wp_template post or not (if not, it would be just like today).
There was a problem hiding this comment.
The editor already supports editing multiple post types and the editor store has a getPostType selector.
There was a problem hiding this comment.
Are you referring to getCurrentPostType()? It still returns wp_template here when I use it. How do I get the post type of the actual post being edited? It's hard-coded to post here, so we need to replace it with a function that dynamically gets the correct post type.
There was a problem hiding this comment.
Sorry, I thought you were asking how to get the post type of the template. (The template is the actual post being edited after all.)
You can get the post that uses the template through editor settings:
const { postId, postType, templateId } = getEditorSettings()|
@epiqueras I made two more small changes:
|
5107ba2 to
009c304
Compare
There was a problem hiding this comment.
Let's add inline comments as to how this fixes the infinite loop and why it occurs.
There was a problem hiding this comment.
Put in a (temporary) fix for a possible infinite loop. This is not a proper solution, and we might need to find a better way to fix it. I haven't been able to dig to its roots, but basically the block parser continues to iterate the wp_template endlessly when processing the content in a REST API request (which also happens during admin initialization because of preloading).
Interesting, are you sure you didn't have nested post content blocks?
What are the repercussions of this fix. Will returning an empty string like this affect anything? If not, I guess we can keep it.
48e9d97 to
6832752
Compare
009c304 to
ef1b9d6
Compare
6832752 to
24e7a1f
Compare
… selecting inside the callback.
89db348 to
42d9b6d
Compare
|
It's all coming together: |
|
@epiqueras Can we extract the post title and post content blocks from this PR into their own PR. These could be useful on their own to build templates in the temporary UI. |
|
They can't really be tested without a template mode. |
|
@epiqueras How so, I mean their "edit" version can't but their "placeholder" version and frontend rendering are very useful even without the "template mode" |
|
Most of their code is the edit part though, and it would be strange to merge that in a PR where you can't test it and it could be completely broken. |
|
I get that and maybe we can leave that part for this PR. That said, in several occasions, I found myself trying to see if I can build a block-based template and I don't really need the "edit" functions for this. I feel these two blocks are the missing pieces to achieve a first raw block-based theme. |
|
I can open a PR for that, but I'm worried that it will just encourage this one to go stale. Unless, your idea is to postpone any sort of "mode" switching in the near future. |
|
I think this one has still a lot of design considerations to be taken care of before landing while the other one seems easier. Also I think since it reduces the changes from this PR to the template mode, it will be easier to review and land. |
|
@youknowriad that's fair. Here: #18461. |
|
Closing this as it will be merged in more granular PRs. |
Depends on #17207
Description
This PR implements a new custom post type for implementing a dynamic template hierarchy like in #4659. Then it makes a few changes to the editor store to load in a "template mode" when it detects that the current post has an attached template.
It also brings in the post title and post content blocks of #16565, but using the new declarative APIs introduced in #17153 and #17207.
All of this new functionality is enabled only when you enable the full site editing experiment in the experiments tab. This way we can iterate it on it, until it's ship-able.
If you try the experiment on this branch, you should see posts load in template mode where you can edit the top level template, the post title and content blocks, preview the post, etc, etc. Blocks have their own save buttons for now, until we design all the different save flows we should have and how they should interact with each other.
How has this been tested?
It hasn't 😬
Screenshots
Types of Changes
New Feature: Introduce a new dynamic templates mode and add basic post title and content blocks.
Checklist: