-
Notifications
You must be signed in to change notification settings - Fork 43
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
feat(i18n): add polish translations #940
base: master
Are you sure you want to change the base?
Conversation
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.
I still need to dig into this further, but I noticed the editor also doesn't seem to be picking up the translations for this PR, but the cause is less obvious to me 😕 I wanted to flag in case anyone else had a chance to look before I get to circle back to it. A couple things I noticed/tried:
- This PR includes more than the usual number of translation JSON files compared to the other languages.
- I did try briefly spot-checking the Spanish translation, and it seems to be picked up by the editor, but I'm not sure if all of the strings are -- I didn't test it that extensively.
- I tried regenerating the JSON files by running
npm run i18n
; it only generated three files for Polish, and multiple messages likeThe string "XXXX" has 2 different translator comments.
So it could that the strings marked for translation/the POT file also needs to be updated. - I also got messages like this for each language, though they were followed by a
Success: Created X files.
line:
./bin/update-translations.sh: line 8: msgmerge: command not found
mv: newspack-blocks-de_DE.po.out: No such file or directory
./bin/update-translations.sh: line 10: msgfmt: command not found
... I'm not sure if this is just me though (I've run this command successfully before, but it's been a while! It may have happened all along and I just didn't make note of it because it worked as expected).
On Would you be willing to provide some documentation on this subject? Namely –
|
Oh strange! It didn't seem to add more files for me when I ran it, but the are an inconsistent number of files per language (some had 4, some had 3), which makes me think something is up.
Absolutely! We have some older docs on the P2 here: PamTN9-1Tk-p2 -- I can get that moved to the Fieldguide with the rest of the documentation, as it's probably a lot less find-able now on our P2 [edited to add: the docs have been moved, but I added a redirect so going from the original P2 link will still work!]. I did update these at one point to include the newer |
I did a little more testing, and it seems like the older POT file may be the main culprit. I updated the POT file locally, updated the Polish PO and MO files so the line numbers matched and re-ran The strings that aren't translated are defaults set in the index.js file, rather than strings in the edit.js file -- that's the only reason I can think of how they're different from the rest 🤔 I tried updating the Spanish files from the new POT the same way, and I can recreate this issue. Looking at the file history, the POT file was last updated 9 months ago, before the 'Donate Now!', and 'Thank you for your contribution.' became editable. 'Load More Posts' is displayed using the 'placeholder' attribute, rather than setting it as a default in the block's index.js, so that seems to be why it works compared to the ones in the donate block. I think the next steps here would be to get the POT file updated, and then figure out why the default strings set in index.js are not applied -- it will probably be necessary to update the POT file and language files in the same release, in case updating one ahead of the other stops translations from working 😕 Let me know what you think! |
After that, the steps from the P2 you've linked to will enable me to prepare these translations correctly? Also asking in a different context – in an upcoming PR, a new translatable string will be added, and I'd like to add it to i18n files in the same PR. In principle, this should only add a few lines in |
Yes, they should! I think whatever you did this time would have worked, too (minus the strings coming from index.js), if the POT file was up-to-date :)
I'm definitely not an expert in this, but I think each time we update the translations we should be regenerating the POT file in full, not just adding the new strings -- that way, we can make sure no string changes are missed. So with that step, there will be the new string to translate, and potentially line number shifts throughout both the .pot and .po files, assuming other code changes have happened since the last time it was done. But it should be pretty painless! Then yes, the .mo and .json files will need to be regenerated with those changes! :) |
@adekbadek I think I've figured out a way to get the translated strings for the donate button and thank you text displaying in the editor! I've created a PR with a fix for that, and updated the block .pot file, along with the other po, mo, and json files. If that's a good approach, once that PR is merged you should be able to update this using the steps in the Fieldguide here: PamTN9-1Tk-p2 -- you should only need to do steps 1-4 under "Update the .po files" and steps 1-2 under "Update the .json files" in the block section. Just let me know if you have any questions at all in the meantime! |
All Submissions:
Changes proposed in this Pull Request:
Adds polish translations.
How to test the changes in this Pull Request:
Other information: