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 i18n issues #295

Merged
merged 5 commits into from
Aug 6, 2024
Merged

Fix i18n issues #295

merged 5 commits into from
Aug 6, 2024

Conversation

pedro-mendonca
Copy link
Contributor

Description of the Change

  • Fix wrong textdomains
  • Add missing textdomains
  • Add missing comment to translators in string with placeholder
  • Fix some simple coding standard issues

Please see issue #294 for a full issue description and screenshots.

Closes #294

How to test the Change

Install a language that the plugin is 100% translated
Go to Settings > Discussion and see all the plugin related strings translated.

Changelog Entry

Fixed - I18n issues

Credits

Props @pedro-mendonca

Checklist:

  • I agree to follow this project's Code of Conduct.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests pass.

@jeffpaul jeffpaul added this to the 2.8.0 milestone Aug 5, 2024
@jeffpaul jeffpaul requested review from faisal-alvi and removed request for dkotter and jeffpaul August 5, 2024 20:17
Copy link
Collaborator

@dkotter dkotter left a comment

Choose a reason for hiding this comment

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

@pedro-mendonca Thanks for the PR, this looks and tests great!

@dkotter dkotter merged commit ada8b1d into 10up:develop Aug 6, 2024
11 checks passed
@faisal-alvi
Copy link
Member

@dkotter We've removed the text domain from avatar_ratings in #175 to fix issues #88 and #157. This PR adds them back, so we should've verified that it doesn't cause the issues to reoccur.

@pedro-mendonca
Copy link
Contributor Author

@faisal-alvi if the site language is set to Language A, and user-language is set to Language B, all these strings are presented in Language B.
The strings appear in the translation table on w.org, so they should be loaded from the .po file and not from core.

@dkotter
Copy link
Collaborator

dkotter commented Aug 6, 2024

@dkotter We've removed the text domain from avatar_ratings in #175 to fix issues #88 and #157. This PR adds them back, so we should've verified that it doesn't cause the issues to reoccur.

That's a good callout and is something I tested. I see the proper translations both if a site language is set and if a user language is set when testing this PR. Not sure if there are other changes we've made that helped fix this since the original issue was reported but I'm not seeing any regression, though feel free to test yourself to ensure that is the case

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

Successfully merging this pull request may close these issues.

i18n - Some strings aren't translatable and others need small fixes
4 participants