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

I18n/fr: fix some errors #748

Merged
merged 9 commits into from Sep 28, 2020
Merged

I18n/fr: fix some errors #748

merged 9 commits into from Sep 28, 2020

Conversation

Letiste
Copy link
Contributor

@Letiste Letiste commented Sep 25, 2020

No description provided.

@codecov
Copy link

codecov bot commented Sep 25, 2020

Codecov Report

Merging #748 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #748   +/-   ##
=======================================
  Coverage   88.44%   88.44%           
=======================================
  Files         457      457           
  Lines       14983    14983           
  Branches     1310     1310           
=======================================
  Hits        13252    13252           
  Misses       1648     1648           
  Partials       83       83           

@bastimeyer
Copy link
Member

Thanks a lot for the fixes, looks good!

Since I can't do it myself, would you please be so kind and take a look at the remaining missing French translations and add them real quick? New stuff has been added to the app over the past months and only the fr locale is currently still missing new translations because nobody has added them yet.

The missing translation keys can be seen by running yarn run grunt webpack:i18n or in the output of the i18n step of Github's test runners:
https://github.com/streamlink/streamlink-twitch-gui/runs/1165021180#step:9:13

These translation keys are compared against the en locale and they need to be copied from the respective yml files from the en locale to the yml files of the fr locale, at the same relative positions where they are in the en files.

I would really appreciate if you could do this, so that there are no more missing translations. Thank you!

@Letiste
Copy link
Contributor Author

Letiste commented Sep 26, 2020

Sure ! I saw some translations missing but didn't know how to add them.
Do I need to create a new pull request or can I just add the changes to this pull request?

@bastimeyer
Copy link
Member

If you prefer opening a new PR for the missing translations, then you can do it of course. I would also be fine with adding them here. I will squash the commits on merge though, so if you want to keep the fixes and addition of missing translations separate, open a new PR.

Copy link
Member

@bastimeyer bastimeyer left a comment

Choose a reason for hiding this comment

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

Thanks a lot! The changes are looking good and all missing translations have been added. There's one tiny issue though which needs to be fixed before I can merge this. Could you please take a look?

streams: Streams
chat: Chat
languages: Langages
hotkeys: Raccourcis claviers
Copy link
Member

Choose a reason for hiding this comment

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

This unfortunately breaks the settings menu's submenu, because the text is too long and the submenu doesn't support overflowing texts/buttons yet.

Is there a shorter translation available? If not, I guess you'll have to use the English menu name. The word "Hotkeys" shouldn't be too bad though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"Raccourcis" should be fine and doesn't break the submenu (at least for me).

@bastimeyer
Copy link
Member

Yep, that fixes the settings submenu.

I'll leave the PR up for another day or so, just in case somebody wants to review or if you find something else to fix. Then I'm going to merge.

@bastimeyer bastimeyer merged commit 5a4db9d into streamlink:master Sep 28, 2020
@bastimeyer
Copy link
Member

Thanks!

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

Successfully merging this pull request may close these issues.

None yet

2 participants