Skip to content
This repository has been archived by the owner on Jan 24, 2024. It is now read-only.

Removed themename in template parts #403

Merged

Conversation

shail-mehta
Copy link
Member

Removed "theme":"twentytwentyfour" in Template Parts as per guideline

Files:
patterns/template-home-portfolio.php
patterns/template-home-writer.php

@richtabor richtabor self-requested a review September 18, 2023 16:48
Copy link
Member

@richtabor richtabor left a comment

Choose a reason for hiding this comment

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

This is blocks at the moment.

The theme is currently required to include template parts within patterns, like we're doing in these two files. Otherwise they don't render:

CleanShot 2023-09-18 at 12 48 08

Related: WordPress/gutenberg#53194 (comment)

@MaggieCabrera
Copy link
Collaborator

I pinged @scruffian on this issue. I wonder, do the template parts still work with the theme slug present if the theme is installed in a subfolder/the folder has a different name than twentytwentyfour?

@MaggieCabrera
Copy link
Collaborator

MaggieCabrera commented Sep 22, 2023

WordPress/gutenberg#54595 has been merged and I would like to merge this by end of day today so it makes it to the first build in beta 1. This ensures that template parts will show no matter the folder your theme is installed in.

Right now to test it we need to use the latest GB trunk. I've marked the PR for backport so it needs to make it to the final GB release before beta for this to work without the plugin installed.

Lets test that this is working as intended.

@MaggieCabrera MaggieCabrera force-pushed the remove-themename-in-template-part branch from 9587473 to 176efea Compare September 22, 2023 08:52
Copy link
Contributor

@scruffian scruffian left a comment

Choose a reason for hiding this comment

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

LGTM

@MaggieCabrera
Copy link
Collaborator

When is it best to merge this? @desrosj if I include this change on the first build of the theme at the current state, the template parts will fail. I'm assuming WordPress/gutenberg#54595 will get backported during Tuesday, so maybe it's best if we sit on this and add it to that build last minute?

@desrosj
Copy link
Contributor

desrosj commented Sep 25, 2023

Let's proceed with the original PR and then create a follow up one for then. If there are any other changes that are ready, we can include them as well.

@richtabor richtabor self-requested a review September 27, 2023 14:00
@MaggieCabrera
Copy link
Collaborator

We can bring this in now, I tested with core trunk and we don't need the theme slugs anymore

@MaggieCabrera MaggieCabrera merged commit c26f827 into WordPress:trunk Sep 29, 2023
2 checks passed
@shail-mehta shail-mehta deleted the remove-themename-in-template-part branch July 27, 2024 03:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants