Block Library: Add Post Content and Title Blocks.#18461
Conversation
epiqueras
added
[Package] Block library
epiqueras
requested review from
Soean,
TimothyBJacobs,
ajitbohra,
jorgefilipecosta and
talldan
as code owners
There was a problem hiding this comment.
The logic of the condition is not clear. maybe expand on the comment. Also if this is temporary what's the ideal fix?
There was a problem hiding this comment.
@felixarntz It's because in the admin views and in REST requests there is no post loop right? So the rest of this function becomes infinitely recursive?
Is there even a fix for this?
There was a problem hiding this comment.
I suppose this should be necessary for all post related blocks too.
For me, this is another reason the Post block is clearer as the condition becomes "is there a parent post context".
There was a problem hiding this comment.
That wouldn't solve this and there is not currently a way for a parent post block to provide context for children during server rendering. You would be relying on the order in which the block tree is traversed while serializing which would be extremely fragile, specially when you start nesting post contexts.
Solving that shouldn't block this PR.
There was a problem hiding this comment.
I noticed that this code is duplicated between post title and post content, As we discussed before, it seems to me that this code is better in a container "post" block. I'm ok moving forward without it and see where the limits of this approach lead us.
That said, we'll probably want to share that code across all post related blocks and avoid duplicating it.
There was a problem hiding this comment.
We'll see. I still think it's valuable to support top level post children blocks like these. The post wrapper block should only be used to overwrite the current post context.
There was a problem hiding this comment.
Can we extract the common code at least?
|
I don't think it's totally related to this PR but I got this warning while testing this PR (when previewing a post using a template containing these blocks)
|
youknowriad
left a comment
youknowriad
left a comment
There was a problem hiding this comment.
Excited to see this land.
It's related to block template directory loading.
Me too 🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉! |
epiqueras
force-pushed
the
add/post-content-and-title-blocks
branch
from
32bdf92 to
15eaeab
Compare
|
@epiqueras do you know how this PR was landed despite the tests fail? |
|
Here the fix: #18543 |
|
Oops sorry, we have occasional timing related transform failures on PRs and I thought that was the case here so I ignored them. |
|
it's bad when we can't trust tests :( |
|
Maybe we should just disable fickle tests for now, since they seem to cause more harm than good. |
| } | ||
| add_filter( 'pre_render_block', 'gutenberg_provide_render_callback_with_block_object', 10, 2 ); | ||
|
|
||
| /** |
There was a problem hiding this comment.
Should we have a separate file for template-related PHP logic? This compat.php file is at high risk for becoming a dumping ground of various utility functions, so should be limited to functions which can be very closely associated with a specific Trac ticket to incorporate a given change. It doesn't seem like we have such a Trac ticket we could link to with this?
There was a problem hiding this comment.
Yes, that would be good. Maybe even template-utils.php.
There was a problem hiding this comment.
Yes, that would be good. Maybe even
template-utils.php.
Would template.php be sufficient to describe the grouping of this related functionality?
There was a problem hiding this comment.
I think it would be confused with templates.php. Maybe a templates directory makes sense at this point? Then, this file could just be called utils.php, templates.php would become create-cpt.php and so on.
There was a problem hiding this comment.
Oh, I missed that there was a file already. In what way does this function differ that makes it not suitable to be placed in templates.php ?
There was a problem hiding this comment.
templates.php just sets up the CPT.
There was a problem hiding this comment.
This is a utility function for server rendered block implementations that use the post loop.
You could even argue it's not template specific and belongs in compat.php.
6 participants
Description
This PR adds the blocks from #17263, limiting the implementation to their front end rendering logic so that they can quickly be merged and used for building block templates without waiting for all the design considerations of #17263.
How has this been tested?
The blocks were added to a template and it was verified that previewing a post renders the template, filling in the placeholders appropriately.
Types of Changes
New Feature: Full Site Editing now has a Post Title and a Post Content Block.
Checklist: