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

Autodetect and set the language on first launch #3481

Merged
merged 7 commits into from Mar 25, 2020

Conversation

vadi2
Copy link
Member

@vadi2 vadi2 commented Mar 18, 2020

Brief overview of PR changes/additions

Autodetect and set the language to the user's desktop environment language if it's one of those that we have a quality translation for.

Motivation for adding to Mudlet

Better i18n experience out of the box

Other info (issues closed, discussion etc)

@wiploo if you could test it as well.

Needs testing on:

  • Linux
  • Windows
  • macOS

@vadi2 vadi2 added this to the 4.7.0 milestone Mar 18, 2020
@vadi2 vadi2 requested a review from a team March 18, 2020 16:56
@vadi2 vadi2 requested a review from a team as a code owner March 18, 2020 16:56
@add-deployment-links
Copy link

add-deployment-links bot commented Mar 18, 2020

Hey there! Thanks for helping Mudlet improve. 🌟

Test versions

You can directly test the changes here:

No need to install anything - just unzip and run.
Let us know if it works well, and if it doesn't, please give details.

src/mudlet.cpp Show resolved Hide resolved
src/mudlet.cpp Outdated Show resolved Hide resolved
@Kebap
Copy link
Contributor

Kebap commented Mar 18, 2020

Works as expected on Win10 with German.
Great addition for new players!

@wiploo
Copy link
Contributor

wiploo commented Mar 19, 2020

Ehmm ... how can I test this PR? Need to have a fresh install? How can I perform that?

@vadi2
Copy link
Member Author

vadi2 commented Mar 19, 2020

I've just documented that in https://wiki.mudlet.org/w/Manual:Testing

Copy link
Member

@SlySven SlySven left a comment

Choose a reason for hiding this comment

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

Would it be reasonable to ALSO bring up the preferences dialogue with the relevant control as the focus on the first launch - if necessary ON TOP of the connection dialogue - so that the non-English speaker end-user can change the setting without having to navigate to the settings blind.

When I originally came up with a GUI switching design I had actually arranged for the controlling widget to have a special non-translatable multi-lingual tool-tip which said something along the lines of "Please select the language you want Mudlet to use." in all the languages that we had translations for. This (and the International icon for "language selection" icon) should have made it blindingly obvious what the first time user needed to adjust!

@vadi2
Copy link
Member Author

vadi2 commented Mar 20, 2020

I don't think so from a user experience perspective - you launch Mudlet for the first time and you're attacked with a multitude of dialogs, one of which is very information-rich, lets say, and one that for 90% of our current userbase won't matter.

@vadi2
Copy link
Member Author

vadi2 commented Mar 20, 2020

I can't confirm this working on macOS: it's having difficulties changing the language. I change the language, it asks to reboot, and after a reboot it forgets about the language change.

src/mudlet.cpp Outdated Show resolved Hide resolved
src/mudlet.h Show resolved Hide resolved
@@ -743,9 +745,6 @@ class translation
// cases the loaded file will be a "xx" language only file even though it
// is an "xx_YY" one here:
QString mQtTranslationFileName;
// Further items like the above pair may be needed should some of the
Copy link
Member

Choose a reason for hiding this comment

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

Don't remove this comment - it is entirely true and shouldn't be forgotten - the few translations in qtkeychain are ones that we need to get around to including in the near future - the author of that library has just added a Russian (ru_RU) translation in the 0.10 release BTW - see https://github.com/frankosterfeld/qtkeychain/tree/master/translations .

That is one aspect I intended when I made the translation class - so as to hold ALL the filenames we need to load to handle a particular GUI language (and unload them if/when that was changed)!

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 don't understand what this comment means, not sure anyone else does either. Can you reword it?

Copy link
Member

Choose a reason for hiding this comment

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

We (will need) to include any bunch of .ts files that any subproject provides and that will need a QString in this class to hold the path to the selected file needed for the selected GUI language.

Specifically right now, someone (likely you or I) will need to add in the QKeyChain translations by:

  • writing a (void) mudlet::scanForQKeyChainTranslations(const QString& path) which will be like void mudlet::scanForQtTranslations(const QString& path) except that it will look for a:
    QString translationFileName(QStringLiteral("qkeychain_%1.qm").arg(languageCode));

to populate a (QString) Translation::mQKeyChainTranslationFileName member with the file that best matches the translations in ./3rdparty/qkeychains/translations

  • extend (void) mudlet::loadTranslators(const QString&) to also load that file after the Qt one and before the mudlet one...

As it happens I allowed for the need for further translation files to be wanted and any additional ones will automatically be handled provided we do the above for any subproject that includes them (as Qt .ts files).

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay - I've added it back with more understandable (for all) wording!

@SlySven
Copy link
Member

SlySven commented Mar 20, 2020

I don't think so from a user experience perspective - you launch Mudlet for the first time and you're attacked with a multitude of dialogs, one of which is very information-rich, lets say, and one that for 90% of our current userbase won't matter.

For the other 10% though they will still need to find their way to the GUI Language setting - and don't forget in the absence of a loaded profile most of the controls will be obviously inactive:

image

This first launch situation - may be one, rare I suppose - when we could conceivable have a QMessageBox pop-up interrupt the normal flow of actions and say - in a variety of languages: "Please select the language you wish Mudlet to use. You may change this later if it is not satisfactory." (or even just have that Icon which hopefully should convey the same meaning) - and have the same combo-box as is on the preferences. I think I have seen other applications do that - and it may be particularly relevant on Windows as I can't recall how good the Qt Desktop Language choice detection is on that OS. OTOH It would be some effort to do for something that the user would only see once per OS/Machine instance that they run Mudlet on.

@SlySven SlySven closed this Mar 20, 2020
@SlySven SlySven reopened this Mar 20, 2020
@SlySven
Copy link
Member

SlySven commented Mar 20, 2020

Ooops - hit the wrong button. 😊

@vadi2
Copy link
Member Author

vadi2 commented Mar 21, 2020

I insist that the first launch experience is super important to be very smooth and pleasant, interrupting it with pop-ups or big windows full of settings runs counter to a good user experience. Remember first impressions matter.

Just check out how simple it is to connect to a game with Mudlet on Windows: https://www.mudlet.org/game-admins/run-game-in-30s

Interrupting that is going backwards, not forwards. That said I don't only want this to be my opinion, can @Mudlet/ui-review pitch in as well?

@vadi2
Copy link
Member Author

vadi2 commented Mar 21, 2020

Review feedback applied.

@vadi2
Copy link
Member Author

vadi2 commented Mar 21, 2020

95% Chinese now translated! @vingi and @wiploo please test with first launch Mudlet (simulating a fresh install) with Chinese/Italian as the desktop / operating system language, it should choose the right language out of the box.

@vingi
Copy link

vingi commented Mar 22, 2020

test in simplified Chinese operation system(Windows 10), after delete/rename .config/mudlet folder, and the keys in Windows Registry, this testing version works very well...quite convenient , Awesome!

@Kebap
Copy link
Contributor

Kebap commented Mar 22, 2020

Not following SlySven's suggestion to present lots of options on first startup.
Instead, maybe think about a "Did you know?" box which can (not as first item) show a hint about changing languages.
Then again, it is completely unrelated to the issue at hand. Could you open a new issue to discuss this further?

@wiploo
Copy link
Contributor

wiploo commented Mar 23, 2020

Hi, on windows 10 italian, works very well.

I don't like language choose screen on first startup, another windows on top of another.

Go autodetect and go to preferences if needed (like now)

Best regards

@vadi2
Copy link
Member Author

vadi2 commented Mar 23, 2020

Thanks for testing! This is good for merging pending approval.

@SlySven
Copy link
Member

SlySven commented Mar 24, 2020

Not following SlySven's suggestion to present lots of options on first startup.
Instead, maybe think about a "Did you know?" box which can (not as first item) show a hint about changing languages.

And what language do you put that in? 😜

I am trying to get across how we handle the case where the user is totally unable to comprehend the language that is used on first presentation to the Mudlet application. Whilst the default locale is a good way to go, it is not infallible; for instance: for Mudlet testing purposes a while back set my Linux box to have the LANG environmental variable set en_GB;de;fr_FR and now I get gitk in French mais je ne parle pas français! Having clobbered myself like that, if I were not familiar with Mudlet I would be hard pressed to navigate to the preferences to set it to the wanted language.

So I am thinking that we could take a copy of just the QComboBox (and it's associated code) used for GUI language selection and put THAT (and that language icon) into a one-shot (only on first launch) pop-up dialogue - which could be preset to the default locale that this PR deduces...

@vadi2
Copy link
Member Author

vadi2 commented Mar 24, 2020

Would that many people be running a desktop environment they are totally unable to comprehend when launching Mudlet for the first time?

Copy link
Member

@SlySven SlySven left a comment

Choose a reason for hiding this comment

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

Okay then, I guess things are as good as they can be. :shipit: ...

@SlySven SlySven assigned vadi2 and unassigned SlySven Mar 25, 2020
@vadi2
Copy link
Member Author

vadi2 commented Mar 25, 2020

OK :D

@vadi2 vadi2 merged commit 71a774f into Mudlet:development Mar 25, 2020
@vadi2 vadi2 deleted the autodetect-language-first-launch branch March 25, 2020 17:27
dicene pushed a commit to dicene/Mudlet that referenced this pull request Mar 28, 2020
* Autodetect and set the language on first launch

* Remove unecessary duplicated languageCode key

* Add English (British) as an exception

* Review feedback

* Simplified texts
Chris7 pushed a commit to Chris7/Mudlet that referenced this pull request Jan 2, 2022
* Autodetect and set the language on first launch

* Remove unecessary duplicated languageCode key

* Add English (British) as an exception

* Review feedback

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

Successfully merging this pull request may close these issues.

None yet

5 participants