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

improve internationalization #1037

Merged
merged 5 commits into from
Sep 9, 2021
Merged

Conversation

rbicker
Copy link
Contributor

@rbicker rbicker commented Aug 30, 2021

As discussed in #930:

  • I have added i18next-browser-languagedetector to automatically detect the browser's language. If the requested language json file cannot be found a fallback to the language defined as in REACT_APP_FALLBACK_LANGUAGE ("en" by default) happens. This enables users to run multilingual installations of yopass.
  • I have restructured the translations to use descriptive identifiers instead of the text fragments as keys. I have structured the translations based on the files where the text fragments are being used. These changes improve the maintainability for custom translations. Basically it means that people do not need to change their translations if you do changes to the wording of an existing text fragment.
  • I have made some minor adjustments in Attribution.tsx so people doing translations can get acknowledged for their work by adding "translatorName" and "translatorLink" to their language json file. I hope this is ok for you.

As these are rather fundamental changes it would be great to merge them before doing further work on the user interface in case you agree with this pull request. If you have suggestions on how to further improve the provided code, please let us know!

I am also willing to do further work, like providing instructions on how people can write their own translations, once this pull request is being merged.

Thanks!

@codecov
Copy link

codecov bot commented Aug 31, 2021

Codecov Report

Merging #1037 (dc141c7) into master (99807f4) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1037   +/-   ##
=======================================
  Coverage   80.00%   80.00%           
=======================================
  Files           6        6           
  Lines         390      390           
=======================================
  Hits          312      312           
  Misses         52       52           
  Partials       26       26           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 688863c...dc141c7. Read the comment docs.

@jhaals
Copy link
Owner

jhaals commented Sep 1, 2021

Looks great overall 👍
One thing missing is instructions for how to run with translations.

@rbicker
Copy link
Contributor Author

rbicker commented Sep 1, 2021

Thanks for your feedback! I have added the instructions in the website's README.

@jhaals
Copy link
Owner

jhaals commented Sep 7, 2021

@rbicker I believe this is good to go except for the failing test. Please run yarn format and commit the updates to README.md.

@jhaals jhaals merged commit 7fd294d into jhaals:master Sep 9, 2021
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.

2 participants