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

Use display language when available #114

Merged
merged 1 commit into from
Sep 22, 2020

Conversation

CarlosOlivo
Copy link
Contributor

@CarlosOlivo CarlosOlivo commented Sep 11, 2020

In the next order:

  1. User settings
  2. Server settings
  3. Android settings

For now, it's necessary to restart the application manually when the language is changed, as long as no events are handled.

Closes #83

Copy link
Member

@Maxr1998 Maxr1998 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!
Apart from my review comments, could you maybe move those function to another file/class as well? Maybe even with dependency injection, scoped to the Activity. See the Koin docs for more info on that.

@Maxr1998
Copy link
Member

Maxr1998 commented Sep 11, 2020

Thanks for the changes! I'll review this later today (after sleeping).

EDIT: was busy with other stuff the last days, will still take some time.

@Maxr1998 Maxr1998 added the enhancement New feature or request label Sep 19, 2020
@Maxr1998 Maxr1998 force-pushed the display-language branch 3 times, most recently from e90a7c9 to dbeb8be Compare September 20, 2020 22:49
@Maxr1998
Copy link
Member

Maxr1998 commented Sep 20, 2020

@CarlosOlivo did some more tweaks, please take another look at those and lmk if they are ok, then I'll merge.
@nielsvanvelzen would also appreciate a review from you 😁

@CarlosOlivo
Copy link
Contributor Author

@CarlosOlivo did some more tweaks, please take another look at those and lmk if they are ok, then I'll merge.
@nielsvanvelzen would also appreciate a review from you 😁

Yep, it works fine, you just have to remove all the code that loads the locale from the API [serverSettings], I noticed after the next test that it is not necessary and even counterproductive.

  1. Set client setting locale [userSettings] to Auto or clear App Data
  2. Set android system locale to English (United States) [es_US]
  3. Set jellyfin server locale [uiCulture] to Spanish (México) [es-MX]

With serverSettings code:

  • 2020-09-21 00:43:40.491 3417-3417/org.jellyfin.mobile.debug D/LocaleUtilsKt: Updated locale from [serverSettings], locale: 'es_MX'
  • webView: English (United States) [es_US]
  • settingsActivity: Spanish (México) [es_MX]

Without serverSettings code:

  • 2020-09-21 00:49:29.693 3422-3422/org.jellyfin.mobile.debug D/LocaleUtilsKt$initLocale: Couldn't acquire locale from config, keeping current
  • webView: English (United States) [es_US]
  • settingsActivity: English (United States) [es_US]

@Maxr1998
Copy link
Member

@CarlosOlivo So I should just keep the user settings source for locale?

@CarlosOlivo
Copy link
Contributor Author

@CarlosOlivo So I should just keep the user settings source for locale?

Yep, when no language is set in user settings the web UI fallback to system locale, not to uiCulture locale.

https://github.com/jellyfin/jellyfin-web/blob/7f8019d01f73a235edbd3e7872cf44310ce013e1/src/scripts/globalize.js#L39

https://github.com/jellyfin/jellyfin-web/blob/7f8019d01f73a235edbd3e7872cf44310ce013e1/src/scripts/globalize.js#L20

That is the default behavior.

@Maxr1998 Maxr1998 force-pushed the display-language branch 2 times, most recently from 11f6fad to 1cb5346 Compare September 21, 2020 22:49
@nielsvanvelzen nielsvanvelzen merged commit e514f2e into jellyfin:master Sep 22, 2020
@CarlosOlivo CarlosOlivo deleted the display-language branch September 22, 2020 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Client settings" parameter page not following the user display language
3 participants