search

rl-institut / WAM_APP_stemp_abw

StEmp-Tool für Anhalt-Bitterfeld-Wittenberg
GNU Affero General Public License v3.0
4 stars 1 forks source link

Multi-language support #72

Closed nesnoj closed 5 years ago

nesnoj commented 5 years ago

Text is prepared by @Stefanie08 in https://github.com/rl-institut/WAM_APP_stemp_abw/tree/feature/multi_language_support

@4lm What options do we have to implement multi-language support?

4lm commented 5 years ago

Thanks for the task, I like :smiley_cat: As spoken f2f - I will have a look and report!

4lm commented 5 years ago

Hi @nesnoj,

multi-language works! In templates and views (also in the Charts :rocket:). I had to do some workarounds, because multi-language support is normally tied as global state to wam/settings.py. But we want each app to have a separate language state, so we are missing some features, which is OK IMO. The stuff we need is there!

I added a simple version with a (unstyled) language switcher in the navbar: https://github.com/rl-institut/WAM_APP_stemp_abw/commit/031e415f6cf37e8680ecc8a3f9ceff0ceb6c9215

To test it out you have to add locale as middleware in wam/settings.py:

MIDDLEWARE = [
    'django.middleware.locale.LocaleMiddleware',
    ...
]

The last problem are the labels and texts in the config files in stemp_abw/config. They are set right at the beginning when the app server of the WAM is started. So right now it's not possible to set the language at runtime, load the appropriate config files with the appropriate language and reload the page or go to an app via link. That's why we always have to restart the server, when we change some stuff in the config files ...

Maybe I find a doable solution for the config files tomorrow ...

nesnoj commented 5 years ago

As agreed yesterday f2f, you create a draft to find out if all language-related parts can be covered. Let me know if you need a review..

EDIT: BTW: In the end, pls add deployment instructions to the dev docs.

4lm commented 5 years ago

Hi @nesnoj,

enabling I18N in stemp_abw is as good as done. As far as I can see, there are only some minor things left to do (e. g. the unit attributes in stemp/abw/layers_region.cfg and temp/abw/layers_region.cfg are in German and should also be translatable).

In the end I opted for the .cfg- and .md files to use the approach you first suggested. I did this, because it was way less work, than changing the code to go the locale .po files route all the way.

So, the way to add a language to stemp_abw is relatively simple now:

  1. Make a new locale folder with .po file via python manage.py message commands in the desired language
  2. Copy all .cfg and .md files and folders from a given locale language folder to the folder from the step before
  3. Translate
  4. Add the language name as string to the LANGUAGE_STORE list in app_settings of stemp_abw. This will also automatically add the language to the language switcher.

Next week, I can explain this approach in more detail f2f, if you like.

nesnoj commented 5 years ago

Sounds great, looking forward! :)

nesnoj commented 5 years ago

Good work @4lm! I merged your PR #76.

There're some shortcomings, I'd be very happy if you could tackle them:

  1. The result chart tooltips (texts below the charts) are not translated. They are loaded correctly here but the class ResultChart is instantiated by each chart only once: at the start of the app in result_charts.py. Hence, the current locale texts are not loaded again. For the very same reason, you used callables in app_settings, I think this could be fixed the very same way here..
  2. I don't like the redundant layer files (layers_*.cfg) for each language, but I guess the units are the only reason y u opted for separate en files? What about moving the units to the labels file?
  3. There're no translation for the JS files (e.g. for loading, leave page warning, ...). Is it a hassle to add it? I know, no template tags available.. :(
  4. Warning message ("results lost..") on language switch (same as the other ones you used when switching to contect etc.)

I saw u added some structure for i18n to the docs, please complete this section :).

Branch 4lm/feature/add-multi-language-support is up-to-date, you can reuse it if you like...

Thanks a lot!

nesnoj commented 5 years ago

Beside my comments above, I discovered a bug:

When switching from DE to EN, the first accordion in energy system is not expanded on startup as expected:

image

Contrary, when switching to DE, it is expanded properly..

nesnoj commented 5 years ago

Another bug: The translation does not work on the landing page - do you know why?

nesnoj commented 5 years ago

Also, some other translations are not loaded, e.g. in the left menu and in the result charts section..

4lm commented 5 years ago

For the record, all the i18n related issues have been fixed in branch 4lm/feature/add-multi-language-support - hopefully :wink:

I will tackle now:

And if there is time:

@nesnoj, I will do a pull request for 4lm/feature/add-multi-language-support and work on the above mentioned issues in another feature branche(s).

4lm commented 5 years ago

There're no translation for the JS files (e.g. for loading, leave page warning, ...). Is it a hassle to add it? I know, no template tags available.. :(

@nesnoj, I did some changes in https://github.com/rl-institut/WAM_APP_stemp_abw/pull/88/commits/bb612dbeb11ed432272629ac06fb06d79b9cf707 and https://github.com/rl-institut/WAM_APP_stemp_abw/pull/88/commits/cc6f7dbf893bfe6324496e395619d81c0279ccb3 to fix this. Do you see another section in the code where this is needed. The approach I used is not optimal and should be refactored in the future.

nesnoj commented 5 years ago

Looks good, thx! The only remaining tasks are

When switching from DE to EN, the first accordion in energy system is not expanded on startup as expected:

Do you have an idea why this happens?

And the docs

nesnoj commented 5 years ago

@nesnoj, I will do a pull request for 4lm/feature/add-multi-language-support and work on the above mentioned issues in another feature branche(s).

:+1:

nesnoj commented 5 years ago

@4lm Urgent: I just deployed the app on WAMdev. When hitting "OK" after selecting a language, the redirect does not work correctly: I'm always redirected to the root (landing page) of the WAM instead of staying on the current page. There's a specific order in which Django tries to redirect when using the set_language()view (that u used). Strangely, it works perfectly fine in my local environment.

4lm commented 5 years ago

@nesnoj, I think I see the problem, will do fix a branch!

4lm commented 5 years ago

@nesnoj, I also can't locally reproduce this issue, can you test if this commit https://github.com/rl-institut/WAM_APP_stemp_abw/commit/2e4acba59c9f902801bc1b475af050277148fb08 on branch 'fix/broken-language-support` fixes the issue. The stemp_abw app now also uses 'de-DE' as language definition and in locale instead of just 'de'.

nesnoj commented 5 years ago

Ok, will test now

4lm commented 5 years ago

@nesnoj, I added a language switch confirm, which is only active in the map view, but not on the start page https://github.com/rl-institut/WAM_APP_stemp_abw/commit/4d1d3fd6b8635d0f9d74f4f32a777697d1784cec. The code also has all commits from #93, so please review this pull request first. Will also do a pull request for this one.

Edit: Pull request for this one is #94

nesnoj commented 5 years ago

Both merged and dev deployed on WAMdev Still redirecting to WAM landing page... :(

4lm commented 5 years ago

Mhh, Ok, will have a look ...

nesnoj commented 5 years ago

Locale Middleware moved as you proposed. Same result..

nesnoj commented 5 years ago

Maybe it's something with the webserver - caddy behaves differently than Django's (gunicorn?) May try to set a redirect target explicitly to avoid the fallback mentioned above..

4lm commented 5 years ago

@nesnoj, can you test if this commit https://github.com/rl-institut/WAM_APP_stemp_abw/commit/6e5f7c7c3fff5c5559c29807affe1e3195f84ebc fixes it, the code is on branch fix/redirect-issue: https://github.com/rl-institut/WAM_APP_stemp_abw/tree/fix/redirect-issue.

nesnoj commented 5 years ago

Works out!! :tada: :tada: :tada: :rocket:

Can you crate a PR (just for the log)

nesnoj commented 5 years ago

Can you crate a PR (just for the log)

I'll do it thx for this important fix

4lm commented 5 years ago

:tada: :tada: :tada: Cool :sunglasses:

4lm commented 5 years ago

Here comes the next PR, I fixed the accordion #97. This one was easy :wink:

nesnoj commented 5 years ago

ooops

nesnoj commented 5 years ago

@4lm Last remaining task: docs

nesnoj commented 5 years ago

Can u finish this one today too?