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

Fixes #148: French translation #161

Merged
merged 14 commits into from
Nov 12, 2019
Merged

Fixes #148: French translation #161

merged 14 commits into from
Nov 12, 2019

Conversation

baltpeter
Copy link
Member

Thanks to @NathanBnm, we have a French translation of the website (see #148). This PR will actually expose that in the UI.

Benni and others added 6 commits October 19, 2019 12:25
Currently translated at 98.9% (631 of 638 strings)

Translation: Datenanfragen.de/website
Translate-URL: https://hosted.weblate.org/projects/datenanfragen-de/website/fr/
Currently translated at 98.9% (631 of 638 strings)

Translation: Datenanfragen.de/website
Translate-URL: https://hosted.weblate.org/projects/datenanfragen-de/website/fr/
Currently translated at 100.0% (638 of 638 strings)

Translation: Datenanfragen.de/website
Translate-URL: https://hosted.weblate.org/projects/datenanfragen-de/website/fr/
@baltpeter baltpeter changed the title WIP: French translation WIP: Fixes #148: French translation Oct 22, 2019
Copy link
Collaborator

@NathanBnm NathanBnm left a comment

Choose a reason for hiding this comment

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

I left a few comments. Let me know what you think.

.eslintrc.js Outdated Show resolved Hide resolved
content/_index.fr.md Outdated Show resolved Hide resolved
config/_default/menus/menus.fr.toml Outdated Show resolved Hide resolved
content/_index.fr.md Outdated Show resolved Hide resolved
content/_index.fr.md Outdated Show resolved Hide resolved
@baltpeter baltpeter changed the title WIP: Fixes #148: French translation Fixes #148: French translation Oct 23, 2019
@baltpeter
Copy link
Member Author

@NathanBnm Done. Huge thanks once again!

@zner0L This should now be ready for review.

@baltpeter baltpeter requested a review from zner0L October 23, 2019 20:36
@baltpeter baltpeter mentioned this pull request Oct 23, 2019
baltpeter added a commit that referenced this pull request Oct 24, 2019
That was… surprisingly not bad. Go figure, I 'spose.

Note: This will *hopefully* result in a merge conflict with #161,
as that moves the requests.json file into the other translations.
zner0L
zner0L previously approved these changes Oct 28, 2019
// Since we now support languages where we cannot guarantee that all strings are always translated, we now need to
// fallback to English for untranslated strings.
const en = require('../i18n/en.json');
data = deepmerge(en, data);
Copy link
Member

Choose a reason for hiding this comment

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

I would like to at least see what translations are missing in debug env. Or are we just going to let Weblate do the work there?

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree with the sentiment but I don't see how to implement this in a way that improves on the current implementation:

  • The only languages for which we can guarantee support are German and English anyway.
  • Seeing an English string on the German site should be pretty obvious anyway.
  • The previous implementation didn't actually alert or highlight missing translations either. It just left them blank. I feel like that is even easier to miss than a translation in the wrong language.
  • What would we change the strings to in the dev env to indicate missing translations? We can't use HTML to actually make them stand out.
  • Weblate makes missing translations pretty obvious.

* In the deploy script `deploy.sh`, make sure to also copy the companies and SVAs for the new language.
* Add the new language to the `languageFiles` for `preact-i18n` in `.eslintrc.js`.
* In `content/` create a `*.[new lang].md` file for all pages that should be available in the new language.
* Specify the menu items in `config/_default/menus.toml`. That file is unfortunately not part of the Weblate infrastructure.
Copy link
Member

Choose a reason for hiding this comment

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

Is there a way to still add the necessary translations to the files nevertheless? I mean, for French it might be possible to do it ourselves but for any other language we need those strings translated as well.

Copy link
Member Author

@baltpeter baltpeter Oct 29, 2019

Choose a reason for hiding this comment

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

I absolutely agree.

Unfortunately, I don't believe that is possible in Hugo. So our options are:

  • Either write our own preprocessor for those files, or
  • Hardcode the menus in the Hugo baseof.html template.

Copy link
Member

Choose a reason for hiding this comment

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

I was just thinking of having the translations in the translation file and copying those by hand. That leaves some work to us, but we don't need to translate them.

Copy link
Member

Choose a reason for hiding this comment

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

@baltpeter I don't know if we need to solve this here though. Merge if you like.

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, I guess that would work, too… Now that we are having our own Webpack thingies anyway, writing a preprocessor might not even be that hard, though.

Let's see. Does GitHub also have a 'Resolve as an issue' button?

NathanBnm
NathanBnm previously approved these changes Oct 29, 2019
@baltpeter
Copy link
Member Author

@zner0L ?

@baltpeter baltpeter dismissed stale reviews from NathanBnm and zner0L via cc41666 November 12, 2019 20:20
@cypress
Copy link

cypress bot commented Nov 12, 2019



Test summary

17 0 0 0


Run details

Project website
Status Passed
Commit cc41666
Started Nov 12, 2019 8:24 PM
Ended Nov 12, 2019 8:25 PM
Duration 00:52 💡
OS Linux Debian - 9.6
Browser Electron 61

View run in Cypress Dashboard ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@baltpeter baltpeter merged commit 2650079 into master Nov 12, 2019
@baltpeter baltpeter deleted the b_french branch November 12, 2019 20:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

4 participants