-
Notifications
You must be signed in to change notification settings - Fork 45
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
[locales/fr.json] correct some typos on french strings #953
Conversation
* add missing non breakable spaces before double ponctuations * correct some accords (like "la vidéo téléchargé" -> "la vidéo téléchargée") * use curvy quote (see http://cesarx.free.fr/de_la_typo.html)
+ string "Language" can now be localised
Hello There. This PR has been tested and is ready for review. Have a nice day ;) |
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.
Hi there and thanks for your PR! As with all languages that I don't know, I obviously can't verify these changes, but it doesn't look like you sneaked any swearwords into this 😁
I only have one comment below. After that is resolved, I'm happy to merge.
src/ui/settings/language.js
Outdated
<SettingsSection title="Language"> | ||
<SettingsSection title={i18n.t('settings-general-header')}> | ||
<label | ||
htmlFor='studio-lang' | ||
sx={{ variant: 'styles.label' }} | ||
> | ||
{i18n.t('settings-language-label')} | ||
</label> | ||
<select | ||
id="studio-lang" |
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.
We actually explicitly kept "Language" untranslated. If you set Studio to some weird language, you still want to find the language selection, and we figured most people know what "language" means. We plan on improving this a lot in the future via some redesign.
So, could you remove this part of the change (including the new translation strings).
Regarding adding a label to this and other drop-down menus: is this mainly for accessibility? I'm not quite sure what this change is for. I'm fully on board with the change for the audio and video device (as it doesn't change anything about what's visible), but I'm not sure it's useful in this language case?
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.
Ok ! It make sense, yes ;) Maybe we can keep both : translated string + "Language" between parenthesis ?
Adding the labels is mandatory for accessibility yes.
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.
to evaluate accessibility level of a web page, I recommend the "Wave" tool : https://wave.webaim.org/ which can be installed as a plugin in your web browser.
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 do know Wave and have it permanently installed actually. I guess I just never checked these Studio things, probably because due to the lack of time, but likely also because not long ago, Studio had way more serious accessibility concerns :D
…e weird language, you still want to find the language selection, and we figured most people know what "language" means. We plan on improving this a lot in the future via some redesign.)
I just made a commit which display "Language" string both translated + untranslated. Is it ok for you ? |
Yep, keeping both sounds good! But can you hide the second |
You're right : it looks weird in english ;) I try to hide it on this case. |
+ use t instead of i18n.t for translating (makes it consistent with the existing code.)
Here's the new commit including :
|
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.
Terribly sorry to request changes again :D But for languages where "language" is not translated, it falls back to English, so they still have the "Language / Language" problem. As shown below, I would just literally check whether the string is "English".
I was so free as to just apply my suggestion. Now I'm happy :D Thank you again for the PR! |
Thank you for the merge and your review : all your remarks are pertinent and it's how we make things better ;) Have a nice day ! |
Corrections on french strings :