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

Update POT file and existing language files #964

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

laurelfulford
Copy link
Contributor

@laurelfulford laurelfulford commented Dec 9, 2021

All Submissions:

Changes proposed in this Pull Request:

This PR includes the following updates:

  1. Updates where the default editable strings in the Donate Block are set. They were originally in the block's index.js file; there, they were successfully being picked up by the .pot file npm run i18n, but not actually showing in the editor with the other translated strings. I think this is because they weren't being included with the other edit.js translation in the same way, though I don't know why this would be a problem. Moving the strings to the block's edit.js file seemed to help, but may be incorrect formatting -- I would really appreciate some smarter eyes on this.
  2. Updates each language's .po file based on the new .pot file, and regenerates their .mo and .json files. For the latter, new files were generated for each of the JS files now in the plugins.

If this approach seems okay, once it's been approved and merged, #940 can be updated and these two strings should start previewing correctly for the Polish translations, too!

How to test the changes in this Pull Request:

  1. Apply the PR.
  2. Add a few of the Newspack blocks to the editor, including at least the Homepage Posts block, Post Carousel and Donate blocks (since they all have strings translated).
  3. Navigate to WP Admin > Settings > General, and try changing your site to another supported language, like German, Spanish, Belgian French or Portuguese.
  4. View the blocks on the front-end, and in the editor, and confirm that translations are being applied.
  5. Repeat step 3 and 4 a couple times, testing out the other languages and confirming they still work.
  6. Try editing the Donate button and thank you message text in the block and save it. Confirm that it displays the updated text on the front-end, and if you exit and return to the editor, make sure it still displays your updated text there. This is to make sure moving where these default values were set doesn't affect the ability to edit them.

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

@laurelfulford laurelfulford changed the title Update POT file and existing language files WIP: Update POT file and existing language files Dec 9, 2021
@laurelfulford laurelfulford requested a review from a team December 9, 2021 20:52
@laurelfulford laurelfulford changed the title WIP: Update POT file and existing language files Update POT file and existing language files Dec 9, 2021
@laurelfulford laurelfulford marked this pull request as ready for review December 9, 2021 20:53
@adekbadek
Copy link
Member

I wanted to test this by verifying the relation of JSON files to what I see. After switching the language to Belgian French, I've updated the "A block for displaying homepage posts." string in the newspack-blocks-fr_BE-fbe7f8c598cf05d4603ba49fec909ded.json file (which should map to src/blocks/homepage-articles/index.js). After refreshing the editor, and after re-building the assets, I still see the untranslated string.

Is this an error on my part?

@laurelfulford
Copy link
Contributor Author

laurelfulford commented Jan 6, 2022

Thanks for testing, @adekbadek! In the JSON file, the translation should actually be coming from the empty string in the square brackets after "A block for displaying homepage posts.", buuuut when I tried changing that, it also did nothing 😕

Knowing I've had problems with the block translations being 'fragile' before, I also tried:

  1. Adding a placeholder translation for that string to the fr_BE PO file, and saving it to update the MO file.
  2. Re-running npm run i18n to regenerate the JSON file.
  3. Doing a couple hard refreshes in the browser

... and it still didn't work. So it definitely seems like some strings files still aren't being picked up!

I'll need to circle back and see if I can figure out what the heck is happening here! In the meantime, I'll mark this one as needing work again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants