-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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(@aws-amplify/aws-amplify-react): Improve french translations #5166
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5166 +/- ##
=======================================
Coverage 73.60% 73.60%
=======================================
Files 203 203
Lines 11879 11879
Branches 2325 2239 -86
=======================================
Hits 8743 8743
- Misses 2954 2974 +20
+ Partials 182 162 -20
Continue to review full report at Codecov.
|
@ChristopheBougere Thanks for your PR! I ran into the same issue in #4979 as part of our UI Refactor (#3279). You can see an example here: The bad news is that there are more strings available in the refactor than I'd love to work with you on getting proper translations into #3279. Your points in #3092 are spot on. How could I help? |
Hi @ericclemmons |
@ChristopheBougere That's right. In #4979, I tried to carry forward existing translations, but there the implementations weren't 1-to-1. What we'll need to do is revert a bit of dc26381 so that translations are automatically bootstrapped. If I setup a PR & lay out how to continue the implementation, would you be able to bring your work over? |
Sure I could do that. Correct me if I'm wrong, but basically it should consist in a translation of all the strings in this file? |
@ChristopheBougere I'll need a bit to setup the PR, but you're right: it's effectively just a map of those translations: import { Translations } from './common/Translations';
export const fr: Record<Translations, string> = {
...
[Translations.SIGN_OUT]: 'Déconnexion',
...
} |
@ericclemmons sounds good to me, let me know when I can help |
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.
Thanks for the improvements! Hopefully we will be able to re-use this in addition to our I18n efforts here: #4979
@ericclemmons and @ChristopheBougere I'm not sure if it's the right place to post 😕 |
This pull request has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs. Looking for a help forum? We recommend joining the Amplify Community Discord server |
Description of changes:
Just improved or added a bunch of french translations.
I must say that it is quite a cumbersome task, and I probably missed some of them because there is no official list of all translations. There are some dynamic strings, or messages depending on thrown Error that makes it very difficult to anticipate everything. I already filed this as a feature request in #3092
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.