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

Fix/un transposed patterns #567

Merged
merged 3 commits into from Apr 18, 2024
Merged

Fix/un transposed patterns #567

merged 3 commits into from Apr 18, 2024

Conversation

pbking
Copy link
Contributor

@pbking pbking commented Apr 17, 2024

When a pattern is created during the cloning action (for example if the source theme has templates with text content and the template is processed to localize the text) then the patterns that are created have the SOURCE theme's namespace rather than the new theme.

This change passes that new slug value when creating the pattern so that it is used instead.

To test:

  • Activate the Programme theme (which has un-localized content in the index template)
  • Create a CLONE of Programme using CBT. When the cloned theme is activated it should work as expected.
  • Observe all of the created patterns and note that the namespace is correct (the slug based on the name given when cloning the theme)

Also the slug generated from the new theme name is no longer using -'s to fix the issue noted below. Fixes: #566

Copy link
Member

@mikachan mikachan left a comment

Choose a reason for hiding this comment

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

I've just given this a test using Programme, naming the cloned theme Programme Two, but I'm seeing a few issues:

  • The cloned theme's slug used in patterns is programme-two-two, I think it should be programme-two
image

Looks like programme-two is being used as the theme name in the functions.php file, when I guess it should be programme_two:

image

@pbking
Copy link
Contributor Author

pbking commented Apr 18, 2024

This has been updated to address what you mentioned here:

Looks like programme-two is being used as the theme name in the functions.php file, when I guess it should be programme_two

which was reported in #566

This is ready for another test.

Copy link
Member

@mikachan mikachan left a comment

Choose a reason for hiding this comment

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

Thanks! I've given this another test.

I'm now seeing programmetwotwo as the theme slug in the pattern block:
image

I was expecting this to be programme-two or programmetwo. I can see the new theme directory name is programmetwo, which looks correct.

The issue with the function names in functions.php has been fixed 🎉

I noticed that the hyphen has been removed rather than replaced with an underscore. I think this is fine, just wanted to check if this was the expected behaviour.

image

@pbking
Copy link
Contributor Author

pbking commented Apr 18, 2024

I noticed that the hyphen has been removed rather than replaced with an underscore. I think this is fine, just wanted to check if this was the expected behaviour.

Yes, though it's unfortunate. It's impossible (difficult? outside of my ability?) to determine which strings get the - and which get the _ so we just have to use neither. :(

This was the original behavior of CBT. It would be nice to improve it in the future though.

@pbking
Copy link
Contributor Author

pbking commented Apr 18, 2024

I'm now seeing programmetwotwo as the theme slug in the pattern block:

This was weird. The problem was because the NEW slug (programmetwo) contained the OLD slug (programme). And the "paternize" function happened BEFORE the namespace swap so when doing a string replace of programme>programmetwo the slug that was in the paternized template programmetwo became programmetwotwo.

I fixed it by making sure that the patternization of the template happens LAST (after the namespace swap).

Copy link
Member

@vcanales vcanales left a comment

Choose a reason for hiding this comment

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

Tested:
✅ Fix for the problem with themes such as "Program Two"
✅ Spaces don't create broken function names

Something to keep an eye out for is that if the theme is given a name that includes an actual underscore, those are not being replaced with dashes, creating Text Domains with underscores.

eg. this theme was given the name "Alter_2"
image

This is important because the text domain has to match the Theme's slug in the wp.org theme directory.

@pbking pbking force-pushed the fix/un-transposed-patterns branch from ff1b364 to adeebad Compare April 18, 2024 15:58
Copy link
Member

@mikachan mikachan left a comment

Choose a reason for hiding this comment

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

Looks like this is working well for me now:

image

I used slightly different testing steps, as since the rebase I think most of the patternisation is done optionally. So instead of just cloning the theme, I saved the theme with all save options selected, and then cloned it. I think this is ready to bring in.

@pbking pbking merged commit 2fec309 into trunk Apr 18, 2024
2 checks passed
@pbking pbking deleted the fix/un-transposed-patterns branch April 18, 2024 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

Cloning a theme causes critical error
3 participants