Skip to content
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

Block Library: Add core template part block. #18736

Merged
merged 2 commits into from
Dec 4, 2019

Conversation

epiqueras
Copy link
Contributor

Follows #18339

Description

This PR adds the implementation for the core/template-part block introduced in #18339.

It focuses on the server-side render callback so that block templates that use template parts begin to render correctly on the front end. The editor implementation for creating and editing template parts will come in a follow-up PR.

How has this been tested?

It was verified that the following steps make post previews render as expected:

Screen Shot 2019-11-25 at 4 11 13 PM

  • Enable the Gutenberg full site editing experiment.
  • Add the following files to your active theme's folder:

/block-templates/single.html:

<!-- wp:template-part {"slug":"header","theme":"twentytwenty"} /-->
<!-- wp:paragraph -->
<p>Single Template</p>
<!-- /wp:paragraph -->

/block-template-parts/header.html:

<!-- wp:paragraph -->
<p>Header TwentyTwenty Template Part</p>
<!-- /wp:paragraph -->

Types of Changes

New Feature: Added a new server-rendered template part block for the full site editing experiment.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR. .

@ellatrix
Copy link
Member

Looks like you still need test fixture files for this block.

@epiqueras
Copy link
Contributor Author

You're right, thanks! I added them.

@ellatrix
Copy link
Member

Why is there a theme attribute? Are these template parts meant to be used still when switching themes? You cannot have multiple themes active, so the attribute seem unnecessary to me.

@epiqueras
Copy link
Contributor Author

From the PR that introduced it, #18339

We need a theme attribute to differentiate between template parts with the same slug originating from different themes. When you save changes to a given template, it and all its nested template parts will persist across theme switches. Even though by that point those template parts will have ids and will be renderable regardless of what the new active theme provides, we need a way for the user to insert new instances of them and that is where theme comes in handy. For now, we store the value in post meta so we can do meta queries.

if ( file_exists( $template_part_file_path ) ) {
$content = file_get_contents( $template_part_file_path );
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure the logic here might need to evolve as we discover edge cases and we polish the flows. I'm not certain the current way of handing theme template parts is the best, but it's a decent one to start with.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll need to handle recursive template parts differently, maybe.

Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@youknowriad
Copy link
Contributor

I managed to make this work (with a rebase) but while testing with https://github.com/carolinan/burgers I noticed this error
Capture d’écran 2019-12-04 à 11 24 25 AM

@epiqueras epiqueras merged commit 21304f9 into master Dec 4, 2019
@epiqueras epiqueras deleted the add/core-template-part-block branch December 4, 2019 17:10
@epiqueras
Copy link
Contributor Author

What's in your line 158 after rebasing? It's probably just a missing if-guard.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Block library /packages/block-library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants