Skip to content

Conversation

@Docteh
Copy link
Contributor

@Docteh Docteh commented Aug 3, 2019

Hello, Looks like anything except the GUI.log can be re-translated on the fly.

Give it a go. I'll fix the commit with a revision to locales/en/messages.json if needed. But maybe we can just drop the string that says that a restart is required.

<span i18n="userLanguageSelect" class="i18n-replaced">Sprache (Das Programm muss zur Änderung neu gestartet werden)</span>

mikeller
mikeller previously approved these changes Aug 3, 2019
Copy link
Member

@mikeller mikeller left a comment

Choose a reason for hiding this comment

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

Wow, I didn't expect this to be possible so easily.

@mikeller mikeller added this to the 10.6.0 milestone Aug 3, 2019
@mikeller
Copy link
Member

mikeller commented Aug 3, 2019

Except that the builds are failing. :-(

@Docteh
Copy link
Contributor Author

Docteh commented Aug 3, 2019

Whoops, not the only bug.
image

Also fixed.

@McGiverGim
Copy link
Member

I did a similar feature when I made the translation system, and @mikeller asked to remove this live language change because not all is changed. Some combos that are loaded in the js and other thinks are not translated. I suggested some workarounds but it was some more little work for the developers.

I will look at the discussion to see if I can find it...

@McGiverGim
Copy link
Member

McGiverGim commented Aug 3, 2019

Here is the original PR #892

@Docteh
Copy link
Contributor Author

Docteh commented Aug 3, 2019

Thanks I'll take a closer look at what doesn't update. If we can get the last percentage fixed I think we can make a strong case going forward for people to keep things translatable on the fly. I figure that the log could be left alone just because of how logs work. Sometimes these things need to sneak up in stages.

I should benchmark the retranslation compared to asking it to translate new items in the document.

@mikeller
Copy link
Member

mikeller commented Aug 3, 2019

@McGiverGim: Good point, I remember now. My point still stands that, if this requires extra coding effort and understanding from people wanting to contribute to the configurator, then this disadvantage outweighs the benefits we get from it. After all, for the majority of users, selecting the language will be a one-off operation, and they won't have any need to switch the language again after selecting the one they want, so having to restart the application for this is only a very minor inconvenience.

@Docteh: Agreed that leaving the log in whatever language it was generated is acceptable.

@Docteh
Copy link
Contributor Author

Docteh commented Aug 4, 2019

Here is an example of an item that is tricky to retranslate. The two drop downs above it are easier.

Does it really need to say "Choose version for ....."? I figure just selecting the latest version should be fine. Or just make it unspecific.
image

@mikeller
Copy link
Member

mikeller commented Aug 4, 2019

@Docteh:

Does it really need to say "Choose version for ....."? I figure just selecting the latest version should be fine. Or just make it unspecific.

That again for me turns into a matter of deciding what is more important - the capability to change languages without a restart, or the capability to add translations to whatever we want without restriction, and without having to think about what is working and what isn't.

@Docteh
Copy link
Contributor Author

Docteh commented Aug 4, 2019

How about a compromise?

We could only change the language if the GUI doesn't think its connected.

Leave the message stating that a restart is required.

Right now if something on a tab doesn't switch languages, you can just go to a different tab, and then come back.

@mikeller
Copy link
Member

mikeller commented Aug 4, 2019

Interesting point, I never looked at it this way, but since tabs are reloaded entirely on every change this will work.

Or take this in a different direction, and put the language drop down into the landing tab (this makes it easier to discover for somebody not fluent in the display language as well).

@Docteh
Copy link
Contributor Author

Docteh commented Aug 5, 2019

Put the language switcher on the landing page? I like that idea. That'll be my next move.

@Docteh Docteh changed the title switch languages on the fly, or find out why not switch languages without an app restart Aug 5, 2019
@Docteh
Copy link
Contributor Author

Docteh commented Aug 5, 2019

Looking for input, this probably needs a touch of design work. I thought about throwing the selector widget into the page, and ended up researching language choosers for a bit. I like having the list.

image

@mikeller
Copy link
Member

mikeller commented Aug 5, 2019

I like it - it shows the option in your language / alphabet right on startup - doesn't go much more straightforward than that.
Speaking from experience of clicking through a plethora of drop downs filled with Chinese characters in search of the one listing 'English', I think this is great.
I'd just add a blurb up top explaining that this is to select the language, and will be stored permanently. And maybe change 'Default' to 'System Default', or add a blurb explaining that this is whatever language your system is in (if available).

Copy link
Member

@McGiverGim McGiverGim left a comment

Choose a reason for hiding this comment

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

I'm not too sure about the change. In this way we have options in the main page, and different options to select in the menu. I like to have all of them in the same manner.

If we finally do this change, maybe is better to make it more visual, maybe add a little flag before each text. If not, I'm pretty sure that most people will not read it.

@Docteh
Copy link
Contributor Author

Docteh commented Aug 6, 2019

image

For the languages, not sure if I want like English (en) or just English. Leaning towards just English. I'll remove eg languagebr_en from the english locale file if we go that way otherwise its useful.

@McGiverGim
Copy link
Member

The locales were there to give "some order" to the texts. They are ordered by locale. I don't know if now, without them, it seems more only a mesh of languages...

@mikeller
Copy link
Member

mikeller commented Aug 6, 2019

I am a bit on the fence about adding the locale for every language to the list - my sense of ordering likes it, my sense of curiosity likes to be able to see what the languages not using the latin alphabet are, but my sense of aestethics just loves the simplicity of the list of languages without any distractions.

But one thing I think we need to address is that currently the cursor when hovering over the language names is text selection - this makes it harder than needed to discover that they are clickable.

@Docteh
Copy link
Contributor Author

Docteh commented Aug 6, 2019

I switched the spans out for links, so they'll be more obviously clickable.

We could also do something to the text on hover. Like underline, or a color change.

mikeller
mikeller previously approved these changes Aug 7, 2019
Copy link
Member

@mikeller mikeller left a comment

Choose a reason for hiding this comment

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

Looking good now. Can you rebase / squash please?

@@ -0,0 +1,6 @@
{
"language_default_pretty": {
Copy link
Member

Choose a reason for hiding this comment

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

wut?

Copy link
Member

Choose a reason for hiding this comment

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

Or to be more precise: What is this used for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In English language_default_pretty is fully defined: "System Default ($t(detectedLanguage))"

This works as a fallback as many languages have language_default translated.

messing with fallback might be a way to rename a key, and not lose the translated content.

tabLanding
defaultChangelogAction
defaultPrivacyPolicyAction
tabHelp
tabFirmwareFlasher

Copy link
Member

Choose a reason for hiding this comment

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

Not sure I understand this right, but the way fallbackLng is used it looks now like fake is the only language that will ever be used as fallback language. This is probably not what we want.

Also, should this mythical language maybe be called fallback or similar instead of fake?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe the fallback is to english. We can specify multiple fallback languages. So for defaultDonateBottom on the landing page gets english when using german. (my change to the link ended up wiping out the translations).

Copy link
Member

Choose a reason for hiding this comment

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

Have a look at the code. Only the first element of fallbackLng seems to ever be used. Is it possible that English is a hardcoded default fallback for the framework we are using?

Copy link
Contributor Author

@Docteh Docteh Aug 9, 2019

Choose a reason for hiding this comment

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

In i18next or in our code? We are injecting information (resources) into the first fallbackLng, but I figured that would be okay to ignore. I figure it'd be an annoying problem to throw out a language while running, even if its a fake one.

For this screenshot I made did fallbackLng: ['fallback','de','en'], and then loaded polish translation manually i18next.changeLanguage('pl')
image

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, my bad, I'd assumed fallbackLng was an option that we'd added (dodgy name and all).

@Docteh
Copy link
Contributor Author

Docteh commented Aug 8, 2019

Looks like I did something slightly wrong. I'll try to fix.

Let me know on the fallback fakery.

mikeller
mikeller previously approved these changes Aug 9, 2019
TABS.landing = {};
TABS.landing.initialize = function (callback) {
var self = this;
var self = this;
Copy link
Member

Choose a reason for hiding this comment

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

This file will need to have the indentation adjusted at some point (not in this pull request though).

@mikeller
Copy link
Member

mikeller commented Aug 9, 2019

@McGiverGim: Is this ok now?

@McGiverGim
Copy link
Member

My review yes, now has been fixed. But I don't understand all this fallback language. @Docteh can you explain why it is needed? I see that you have translations that recommend to translate in other place, translations that are in the fallback...

@McGiverGim
Copy link
Member

It seems it has been done in this way to maintain some actual translations. If they are not valid/needed the correct way is to remove/change them. I don't have too much time now to look deeply to the PR, so if it is needed for certain reasons sorry, I have not see them.

@Docteh
Copy link
Contributor Author

Docteh commented Aug 10, 2019

Oh, probably should have commented that the fallback modifications are pulled.

Copy link
Member

@McGiverGim McGiverGim left a comment

Choose a reason for hiding this comment

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

I think we will need to add something like a flag to make them more visible but is ok for now. Good job!

@mikeller mikeller merged commit 7e371ee into betaflight:master Aug 10, 2019
@Docteh Docteh deleted the tooslow branch August 11, 2019 20:44
@j4k0xb j4k0xb mentioned this pull request Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants