WordPress / theme-experiments

Experimenting with themes made out of blocks.
GNU General Public License v2.0
546 stars 180 forks source link

Block Themes: Remove theme attributes from block templates #172

Closed jeyip closed 3 years ago

jeyip commented 3 years ago

Changes proposed in this Pull Request:

Remove theme attributes from block-theme block templates

Context:

This PR was created as a follow-up to WordPress/gutenberg#28088. See the aforementioned PR description and code changes for more context.

Discussion:

  1. What this means for theme designers is that, when creating block templates, the theme attribute can be omitted from block templates (like we've done in this PR). The logic for the Gutenberg full site editor will automatically add a theme attribute that matches the current, activated theme's stylesheet.
  2. New sites created for full site editing will not load template parts correctly until https://github.com/WordPress/gutenberg/pull/28088 and https://github.com/WordPress/gutenberg/pull/27910 are added to a release.
  3. Testing:

    • Apply this PR
    • Run Gutenberg locally (see instructions here)
    • Install the tt1-blocks theme by referencing it in .wp-env.override.json

      
      // .wp-env.override.json
      
      // Replace ~/work/theme-experiments/tt1-blocks with 
      // the path to your locally installed tt1-blocks theme
      {
         "mappings": {
             "wp-content/themes/tt1-blocks": "~/work/themes/tt1-blocks",
         }
      }```
    • npx wp-env start --update
    • Activate the tt1-blocks theme
    • Open the front end and the site editor. FSE templates and template parts should load without issue.
    • Reinstall the tt1-blocks theme in a subdirectory by modifying .wp-env.override.json

      
      // .wp-env.override.json
      
      // Add a test subdirectory in the mapping
      {
         "mappings": {
             "wp-content/themes/test/tt1-blocks": "~/work/themes/tt1-blocks",
         }
      }```
    • npx wp-env start --update
    • Activate tt1-blocks.
    • Open the front end and the site editor. FSE templates and template parts should still load without issue.
pattonwebz commented 3 years ago

I'm having a tough time understanding what problem this PR solves or if there might be alternative approaches.

It's the problem that when templates are created and then the theme is moved to a subdirectory and reactivated they aren't available in the reactivated theme?

jeyip commented 3 years ago

I'm having a tough time understanding what problem this PR solves or if there might be alternative approaches.

I'll update the PR description tomorrow to try and make the problem more accessible 👍

It's the problem that when templates are created and then the theme is moved to a subdirectory and reactivated they aren't available in the reactivated theme?

That's on the right track. What happens is that when the theme is initially created in or moved to any subdirectory, the template parts defined in theme-provided block templates won't load in the Full Site Editor.

The issue will look something like this:

101551397-33717400-39b1-11eb-8e31-91cfa8cd745d

Example

Take the front-page.html block template for example. It has a serialized template part that looks like this:

<!-- wp:template-part {"slug":"header","theme":"tt1-blocks","align":"full", "tagName":"header","className":"site-header"} /-->

In a recent Gutenberg PR https://github.com/WordPress/gutenberg/pull/28088, we decided that the best path forward was to remove the static theme attribute provided by block templates theme files. The Gutenberg plugin now handles assignment of the correct theme attribute (see here and here) so that we can guarantee that it will match the current theme's stylesheet.

carolinan commented 3 years ago

I am not able to use the steps listed above for testing. I am not using wp-env. But as far as I can tell the pull request is correct.

The confusion lies in the description, the pr is very straight forward and all that was needed was to refer to https://github.com/WordPress/gutenberg/pull/28088 🤷‍♀️

jeyip commented 3 years ago

The confusion lies in the description, the pr is very straight forward and all that was needed was to refer to WordPress/gutenberg#28088 🤷‍♀️

That's a wonderful suggestion! I removed the PR description since it seemed to be causing more confusion than helping and included a reference to #28088 instead. Thank you @carolinan 🙏

jeyip commented 3 years ago

Could we wait for WordPress/gutenberg#28088 to be included in a release before landing this PR, or is there a reason to ship this PR before then?

This makes sense to me 👍 We can revisit this once Gutenberg v9.9 is released

jeyip commented 3 years ago

Quick update: The Gutenberg v9.9 release is being pushed back to align with the release of WordPress v5.7 beta 1. Both are slated for release on February 2nd.

carolinan commented 3 years ago

I merged the changes for TT1 Blocks only.

jeyip commented 3 years ago

This PR originally updated twentynineteen-blocks, twentytwenty-blocks, and tt1-blocks. After giving it more thought, since Carolina already merged the changes for tt1-blocks, should we still go through the trouble of updating twentynineteen and twentytwenty block themes? I'm not sure if they're still being supported. @jffng

If not, I can close out this PR.

jeyip commented 3 years ago

Checking in again @jffng @kjellr. I'm leaning towards closing this PR, since it seems like twentynineteen and twentytwenty blocks aren't being actively supported as experiments anymore

kjellr commented 3 years ago

Yeah, I think it's fine to close, until we do a more thorough refresh of either of those two themes. Thanks, @jeyip!