Skip to content

[EC-1002] [BEEEP] Add ability to change language in app#2299

Merged
fedemkr merged 6 commits intomasterfrom
EC-1002-change-language-in-app
Mar 1, 2023
Merged

[EC-1002] [BEEEP] Add ability to change language in app#2299
fedemkr merged 6 commits intomasterfrom
EC-1002-change-language-in-app

Conversation

@fedemkr
Copy link
Member

@fedemkr fedemkr commented Jan 16, 2023

Type of change

  • Bug fix
  • New feature development
  • Tech debt (refactoring, code cleanup, dependency upgrades, etc)
  • Build/deploy pipeline (DevOps)
  • Other

Objective

Add ability to change the language from the app. For now it requires app restart to apply the change.

Code changes

  • OptionsPage.xaml: Added language picker
  • OptionsPageViewModel: Added possibility to select a language/locale, also added system option (with null key value). This is saved into preferences.
  • MainApplication, iOSCoreHelpers: Load the app locale if any selected and set the app culture.
  • MobileI18NService: Added SetCulture(...) so that the culture can be from away the class and fixed some caps in the languages (IDK if the others should be in caps or not)
  • IPreferencesStorageService, PreferencesStorageService: Added sync methods for Get, Save, Remove and the interface so the caller can use the synchronous way instead of the async one of the IStorageService. This was done to avoid needing async code for setting the culture when the app is loading which caused some race conditions and therefore some strings weren't translated correctly because the strings were being translated before the Culture was actually updated.
  • StorageMediatorService: Added this mediator in order to interact with the storages. With this we can have sync and async methods of interaction where the logic to choose one or the other is transparent to the caller and there are also StorageMediatorOptions to pass.
  • StateService: Added get/set Locale to here to be alike to other global settings like theming. Additionally, now it uses the aforementioned mediator to interact with the storages. For now, it's only applied to Locale but afterwards it'll be refactored so that the storage services are removed from here and the mediator is used for everything. Also, marked some methods as Obsolete so the mediator is used in new additional features.
  • Others: Use the StateService to get/set the locale

Screenshots

Language option

Language option selector

Language selected message

Language option dark theme

Before you submit

  • Please check for formatting errors (dotnet format --verify-no-changes) (required)
  • Please add unit tests where it makes sense to do so (encouraged but not required)
  • If this change requires a documentation update - notify the documentation team
  • If this change has particular deployment requirements - notify the DevOps team

@fedemkr fedemkr added the hold do not merge yet label Jan 16, 2023
# Conflicts:
#	src/App/Resources/AppResources.resx
#	src/Core/Abstractions/IStateService.cs
@fedemkr fedemkr removed the hold do not merge yet label Feb 24, 2023
@fedemkr fedemkr requested a review from a team February 24, 2023 15:05
@fedemkr fedemkr requested a review from mpbw2 February 24, 2023 19:58
var clearClipboard = await _stateService.GetClearClipboardAsync();
ClearClipboardSelectedIndex = ClearClipboardOptions.FindIndex(k => k.Key == clearClipboard);

var appLocale = _synchronousStorageService.Get<string>(Core.Constants.AppLocaleKey);
Copy link
Contributor

Choose a reason for hiding this comment

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

Using the storage service directly from the app without going through _stateService goes against the pattern established with multi-account support (even for globals). We could add a Synchronous bool to StorageOptions and create a GetDefaultSynchronousStorageOptionsAsync method to StateService to be used from new AppLocale getter/setter methods, lining up nicely with the other app settings. (I'm assuming this value is global since it sounds like Locale must be set on app initialization, so check out GetThemeAsync & SetThemeAsync in StateService for a good global reference)

Copy link
Member Author

@fedemkr fedemkr Feb 28, 2023

Choose a reason for hiding this comment

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

I'll make the change, although I think that at some point we should refactor it to have the globals in some other place and leave the StateService for account related stuff only. I also believe the StateService is getting gigantic and has a lot of responsabilities.

…eMediatorService to a new way to interact with the storage. Later the StateService will only interact with this mediator instead of directly with the storage services, with this we have more control inside the mediator and we can have both sync and async methods to interact with storages handled by the mediator
@fedemkr fedemkr requested a review from mpbw2 March 1, 2023 15:30
Copy link
Contributor

@mpbw2 mpbw2 left a comment

Choose a reason for hiding this comment

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

Nice!

@fedemkr fedemkr merged commit 470e08f into master Mar 1, 2023
@fedemkr fedemkr deleted the EC-1002-change-language-in-app branch March 1, 2023 16:28
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.

2 participants