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

fix(mobile): fixes on language change #14089

Merged
merged 5 commits into from
Nov 19, 2024

Conversation

johnstef99
Copy link
Contributor

@johnstef99 johnstef99 commented Nov 11, 2024

This pull request fixes several problems around the flow of changing language:

  1. When you change language some widgets in the pages [SettingsSubPage, SettingsPage, LibraryPage] stay in the old language.
  2. Because across the app, you don't pass the context.locale to DateFormat, by default Intl uses the system's locale. This means that even after selecting a language in the immich settings, the dates will stay in the system's language.
  3. I created a new provider localeProvider that can be used inside other providers to invalidate them whenever the locale is changed. This is needed because immich has providers that cache UI elements. I added the localeProvider only to the asset providers of the photos page but I am sure someone with more knowledge of the codebase can find other instances where it's needed.

Before fix:

not_fixed.mp4

After fix:
As you can see when changing language both settings and photo grid get updated.

fixed.mp4

This will make the make the pages to instantly refresh the correct
translated string, without the need to pop and push the settings page.
This is needed because across the app, you don't pass the context.locale
to DateFormat, so by default it uses the system's locale. This will fix
the issue without the need to refactor a lot of code.
This provider can be used to refresh providers that provide UI elements
and get cached.
This is necessary to update the locale on the already evaluated
DateFormat.
@NicholasFlamy NicholasFlamy added date-time Issues with date/time/time zone changelog:bugfix labels Nov 11, 2024
@NicholasFlamy
Copy link
Member

@johnstef99 Is PR #14035 related to one of the issues you're fixing?

@johnstef99
Copy link
Contributor Author

Is kind of related, because the user describes this:

Changing my phone to English does resolve the issue.

That means that DateFormat does not use the context.locale and instead uses the phone's locale. That problem is going to get fixed by this change here

@NicholasFlamy
Copy link
Member

Is kind of related, because the user describes this:

Changing my phone to English does resolve the issue.

That means that DateFormat does not use the context.locale and instead uses the phone's locale. That problem is going to get fixed by this change here

Okay, just wanna be sure your changes and that PR don't conflict.

@johnstef99
Copy link
Contributor Author

No conflicts for sure. The #14035 initialises the date formatter, but this PR make the date formatter use the selected locale from immich settings.

@@ -46,6 +46,7 @@ class SettingsPage extends StatelessWidget {

@override
Widget build(BuildContext context) {
context.locale;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this used to trigger the widget rebuild?

Copy link
Contributor Author

@johnstef99 johnstef99 Nov 19, 2024

Choose a reason for hiding this comment

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

Exactly! It's like doing MediaQuery.sizeOf(context) which is going to rebuild the widget when the size changes from the InheritedWidget

@alextran1502 alextran1502 merged commit 3a2e30e into immich-app:main Nov 19, 2024
33 checks passed
yosit pushed a commit to yosit/immich that referenced this pull request Nov 21, 2024
* fix(mobile): make widgets rebuild on locale changes

This will make the make the pages to instantly refresh the correct
translated string, without the need to pop and push the settings page.

* fix(mobile): set the default intl locale

This is needed because across the app, you don't pass the context.locale
to DateFormat, so by default it uses the system's locale. This will fix
the issue without the need to refactor a lot of code.

* feat(mobile): create localeProvider

This provider can be used to refresh providers that provide UI elements
and get cached.

* fix(mobile): refresh asset providers on locale change

This is necessary to update the locale on the already evaluated
DateFormat.

---------

Co-authored-by: Alex <alex.tran1502@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:bugfix date-time Issues with date/time/time zone 📱mobile
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants