Add feature to set application language. #902

manavmehta · 2020-03-21T08:18:00Z

What's this PR do?
Fixes: #855
Adds feature that lets user to change Zulip desktop client language without requiring to change the system language as was the case earlier

Any background context you want to provide?
Earlier, Zulip client used to fetch system locale thus setting default language to the client. Now we enable user to set custom language to the client.

Screenshots?

You have tested this PR on:
[✓ ] macOS Catalina 10.15.2

manavmehta · 2020-03-21T08:18:32Z

@akashnimare @timabbott @abhigyank Please review

app/renderer/js/pages/preference/general-section.ts

app/renderer/css/preference.css

app/renderer/js/pages/preference/general-section.ts

manavmehta · 2020-03-21T14:30:25Z

@abhigyank thanks, refactored according to your review.

abhigyank · 2020-03-21T16:47:30Z

LGTM!
Just fix the commit messages. Checkout Zulip commit guidelines for detailed information on how to create coherent commits.

app/renderer/js/pages/preference/general-section.ts

akashnimare · 2020-03-22T10:34:36Z

A couple of doubts -

Electron also started supporting spellchecker in v8, can you check what's the status of that?
I see only a couple of options in the supported languages. Are we just supporting the web app languages or what?

abhigyank · 2020-03-22T13:10:22Z

@akashnimare This PR for the app language ( to be used in menus and settings mostly), not related to spellchecker.
I suppose the list of languages are the ones for which have translation available on the desktop app. @manavmehta can you confirm this?

manavmehta · 2020-03-22T15:09:47Z

@abhigyank that's right
@akashnimare, As @abhigyank mentioned we use translations/'lang'.json for this
Initially we used to fetch system language which would be done manually here for end-user's convenience. Spellchecker comes into play in chats which was not the purpose hereby.
We may work on that separately forth.
2. We use all of the languages for which the translations are available in the desktop client. Further addition of supported languages in app can be appropriately done.

akashnimare · 2020-03-23T11:20:50Z

A couple of changes -

The border looks weird on focus, can you update it?

Would it make sense to move this setting to just above the add custom CSS?

manavmehta · 2020-03-23T12:44:15Z

@akashnimare updated the border color to transparent

Would it make sense to move this setting to just above the add custom CSS?

Yeah. I thought the user would use it often so kept it above but now it looks much coherent.

Have a look!

akashnimare · 2020-04-12T14:14:24Z

@manavmehta can you fix the merge conflicts?

app/renderer/js/pages/preference/general-section.ts

app/renderer/js/utils/translation-util.ts

akashnimare · 2020-04-17T08:01:30Z

Looks good. Left a few comments. @abhigyank GTG?

abhigyank · 2020-04-18T17:27:59Z

LGTM.

User can now select application language without changing the language in the operating system Dynamically create locales object for dropdown Method: Store supported locales on disk in /translations/supported-locales.json and read in general-section.ts Why?: Avoids hardcoding of locales object and eases the process of adding next supported locales

akashnimare · 2020-04-22T12:39:03Z

Merged. Thanks, @manavmehta.

akashnimare · 2020-04-22T14:25:33Z

@manavmehta as a follow-up can you take care of the translation thing we talked about?

manavmehta · 2020-04-22T15:57:21Z

@akashnimare On it

andersk · 2020-04-22T19:12:18Z

app/renderer/js/pages/preference/general-section.ts

+		let langIndex = ConfigUtil.getConfigItem('languageIndex');
+		langIndex = (langIndex === null) ? 4 : langIndex;


Storing this as an index is extremely fragile—it means we can never reorder, insert, or delete languages without disturbing other languages.

#939

Agreed. Thanks @andersk :)

…ip#909 The comment was created to document a piece of code in zulip#902 but zulip#909 addressed code quality The comment is no longer essential and hence ought to be removed

The comment was created to document a piece of code in #902 but #909 addressed code quality The comment is no longer essential and hence ought to be removed

zulipbot added the size: XL label Mar 21, 2020

zulipbot added size: L and removed size: XL labels Mar 21, 2020

manavmehta force-pushed the lang branch from f272467 to 8a7af89 Compare March 21, 2020 10:47

abhigyank suggested changes Mar 21, 2020

View reviewed changes

abhigyank reviewed Mar 21, 2020

View reviewed changes

app/renderer/js/pages/preference/general-section.ts Outdated Show resolved Hide resolved

akashnimare reviewed Mar 21, 2020

View reviewed changes

app/renderer/js/pages/preference/general-section.ts Outdated Show resolved Hide resolved

manavmehta force-pushed the lang branch from 1d02564 to 576fa3c Compare March 22, 2020 08:01

akashnimare reviewed Apr 12, 2020

View reviewed changes

app/renderer/js/pages/preference/general-section.ts Outdated Show resolved Hide resolved

akashnimare reviewed Apr 12, 2020

View reviewed changes

app/renderer/js/pages/preference/general-section.ts Outdated Show resolved Hide resolved

manavmehta force-pushed the lang branch from 3117f3a to 24cfecb Compare April 16, 2020 14:06

zulipbot added size: XL and removed size: L labels Apr 16, 2020

manavmehta force-pushed the lang branch from e738869 to da404cb Compare April 16, 2020 17:50

zulipbot added size: L and removed size: XL labels Apr 16, 2020

manavmehta requested review from akashnimare and abhigyank April 16, 2020 18:06

akashnimare reviewed Apr 17, 2020

View reviewed changes

app/renderer/js/utils/translation-util.ts Show resolved Hide resolved

manavmehta force-pushed the lang branch from 126d9a8 to caeea59 Compare April 22, 2020 12:27

manavmehta force-pushed the lang branch from caeea59 to 6289c5b Compare April 22, 2020 12:34

akashnimare changed the title ~~Add feature to set application language without changing system language~~ Add feature to set application language. Apr 22, 2020

akashnimare merged commit b2f4af0 into zulip:master Apr 22, 2020

manavmehta deleted the lang branch April 22, 2020 12:41

andersk reviewed Apr 22, 2020

View reviewed changes

manavmehta mentioned this pull request May 6, 2020

document: Remove inessential comment inherited from #902 and #909 #954

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add feature to set application language. #902

Add feature to set application language. #902

manavmehta commented Mar 21, 2020

manavmehta commented Mar 21, 2020

manavmehta commented Mar 21, 2020

abhigyank commented Mar 21, 2020

akashnimare commented Mar 22, 2020

abhigyank commented Mar 22, 2020

manavmehta commented Mar 22, 2020 •

edited

Loading

akashnimare commented Mar 23, 2020

manavmehta commented Mar 23, 2020 •

edited

Loading

akashnimare commented Apr 12, 2020

akashnimare commented Apr 17, 2020

abhigyank commented Apr 18, 2020

akashnimare commented Apr 22, 2020

akashnimare commented Apr 22, 2020

manavmehta commented Apr 22, 2020

andersk Apr 22, 2020 •

edited

Loading

manavmehta Apr 23, 2020

		let langIndex = ConfigUtil.getConfigItem('languageIndex');
		langIndex = (langIndex === null) ? 4 : langIndex;

Add feature to set application language. #902

Add feature to set application language. #902

Conversation

manavmehta commented Mar 21, 2020

manavmehta commented Mar 21, 2020

manavmehta commented Mar 21, 2020

abhigyank commented Mar 21, 2020

akashnimare commented Mar 22, 2020

abhigyank commented Mar 22, 2020

manavmehta commented Mar 22, 2020 • edited Loading

akashnimare commented Mar 23, 2020

manavmehta commented Mar 23, 2020 • edited Loading

akashnimare commented Apr 12, 2020

akashnimare commented Apr 17, 2020

abhigyank commented Apr 18, 2020

akashnimare commented Apr 22, 2020

akashnimare commented Apr 22, 2020

manavmehta commented Apr 22, 2020

andersk Apr 22, 2020 • edited Loading

Choose a reason for hiding this comment

manavmehta Apr 23, 2020

Choose a reason for hiding this comment

manavmehta commented Mar 22, 2020 •

edited

Loading

manavmehta commented Mar 23, 2020 •

edited

Loading

andersk Apr 22, 2020 •

edited

Loading