-
Notifications
You must be signed in to change notification settings - Fork 368
Make theme available for translation as much as possible #6484
Conversation
I think that any of the Blockbase children could be handled by Blockbase itself. This seemed to work from there to load translations for anything Blockbase.
But during testing I noticed that the localized resources that were from Blockbase weren't getting provided, only those from the child (Zoologist, etc). This might be related to how the patterns are created (via code rather than via a template as most other themes do). Switching the 404.php to a template seemed to get the expected value rendered. |
Sorry for the late reply. I'll check your way when I have the time! Looks like it's possible to add the function call in single place 👍 |
I did some tinkering and (I'm not convinced this is the way to do it but) I found that for Blockbase CHILD themes any pattern that comes from the PARENT (blockbase) isn't translated unless the function is called for BOTH domains:
I've been doing my testing with Geologist mostly. |
Nice, I'll have a try! thanks 👍 |
I've added the function call to the "blockbase" theme so that the patterns from the "blockbase" will be translated. It looks like lots of themes are still not the child theme of the blockbase, so my opinion is to add the function call to every theme and then we won't forget to call it for the newly created theme. What do you think? |
…not the children)
…location to simplify the call
e2085c4
to
c58bb66
Compare
I started with a rebase to bring everything current. I've updated the logic in Blockbase to handle the child themes so they don't need to include that logic. I've also removed the location from the calls since the value that was being passed was the default; no need to define it and it simplifies the method call. I've added the call to a few additional themes, including Block Canvas so any future themes based on that should include that logic. I believe that this is ready for review and hopefully we can get it shipped. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like you've covered all Blockbase + children, and all our current block themes 👍
I don't seem to get the same results locally and on dotcom.
This is what I'm seeing on Skatepark's 404 on wpcom:
Before | After |
---|---|
But this is what I'm seeing on my local site:
I tested in Swedish, with the same translation files in both. I copied the contents of the languages folder over from the theme on wpcom to make sure I definitely had the same translations.
I'm confident this is working on wpcom, so I'm happy for this to be merged for this use case.
I may be testing this incorrectly locally, so I'd appreciate either some help testing locally (or someone else to test!) or perhaps we should look into a local fix next.
Thanks for testing! I believe this is good to ship 🙂 But I'm not familiar with the flow of deploying the PR in this repo, could you help to do it? thanks! |
I tinkered around with this as much as I could to see why things weren't getting translated in wporg-land and came up empty. I'm keen to get this into wpcom, and there's no negative side to it existing as far as I can tell so I'm going to go ahead and bring this in and get it shipped. But I agree that we need to look into:
|
Changes proposed in this Pull Request:
load_theme_textdomain
function call to load the translation so that we could get the translated theme patterns 🙂Related issue(s): 118-gh-Automattic/ganon-issues