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: improve language selection resolution #1011

Draft
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

achou11
Copy link
Member

@achou11 achou11 commented Feb 26, 2025

Fixes #978

The issues

There were a few issues I noticed about how we handle language settings that were prone to bugs:

  1. We immediately persist the initial state of a persisted zustand store. It wasn't apparent to me at the time of implementation, but this is not a desirable behavior. Persistence should generally happen based on explicitly actions or via some kind of clearly defined effect. Doing this implicitly contributes to the issue that is detailed next.

  2. System preferences could be ignored unintentionally. Basically what's highlighted in App does not use system language preference when expected #978, but here's a sequence that highlights the various issues:

    1. Using the following conditions: Fresh install with language preference set to English.
    2. Upon opening the app, there is no persisted locale, so we defer to the default value, which is calculated here. This initial value is persisted to storage immediately due to the first issue I highlighted.
    3. App is presented in English right now, but for some reason I decide to update my system settings to use a different language. I go to the system settings and set my preferred language to Portuguese.
    4. I return to the app and since we support Portuguese as a language, I'm assuming that the app will automatically update to preset itself in Portuguese. Doesn't happen. Still in English
    5. Okay, maybe restarting the app will help. Nope, app is still in English.

    Steps (4) and (5) highlight a couple of the issues:

    1. The app does not dynamically update to respect system preferences when it's expected to
    2. The app does not respect system preferences upon subsequent openings of the app

    The main problem here lies in the first issue I mentioned. Using a more code-friendly explanation, what's happening is the following:

    1. With a fresh install of the app and assuming our system preferred language is English -> there's no selected language that's persisted, so we do a calculation and end up with 'en' as the language to use for the app.
    2. 'en' is persisted to storage because of issue 1
    3. The system settings change the preferred language to 'pt', but our calculation of the language to use is not reactive to this, as we only check the system settings when initializing Zustand (using getLocales() from expo-localization) and only in the case that something is not already persisted
    4. The React state value that's used is still 'en' even though it should be 'pt'

Terminology

Before getting into the changes that address the issues above, it's helpful to clarify some wording that I've committed to for this PR. I found it confusing to figure out what a "locale" meant versus a "language code" versus a "language tag" when it came to what's represented in code, so I made a concerted effort to distinguish between them appropriately:

  • locale: kind of this nebulous definition of a language based on many components that define how a language is presented and used. basically, the Locale type from expo-localization is a good example. given this, I found our usage of the word locale in the codebase to be a bit too vague (and somewhat misleading), so I made lots of efforts in the PR to avoid using it for naming where possible.

  • language tag: basically what's detailed here. These can either be single or multi-component strings, where each components adds a degree of specificity (e.g. regional code). For example, en and en-US are both language tags.

  • language code: in practice, it's the component of a language tag that defines the primary language, which is typically the first component of the language tag (i.e. before the first hyphen if there are any). For example, the language code for en-US is en. For all intents and purposes of our usage, every language code is a valid language tag.

Changes this PR introduces

  • uses our new approach to Zustand-based state for persisting a selected locale. What's persisted is information related to language that is explicitly selected via user action (e.g. a store action). In this case, the storage looks like { languageCode: string | null }. There is no more implicit persistence on initialization like before.

  • introduces a hook called useResolvedLanguageTag() which figures out which language tag to use for the app based on:

    • the selected language code (i.e. what's persisted in storage)
    • the system preferences

    This hook solves the issue where we aren't reactively updating based on changes to the above. In addition to that, it also accounts for multiple system preferences. Before, we'd only take the first language preference and use that, but that's prone to unideal behavior because a user could have a set of preferred languages that may not actually be supported by the app. For example, if the system preferences were something like ["foo", "bar", "pt"], the existing behavior would attempt to only use "foo" despite "pt" being available and supported in the app.

  • cleans up the implementation for intl-related utilities and resolution of language tags. I noticed that there's a noticeable distinction between the following:

    • translated languages: languages that have a non-trivial number of translated messages (basically what's found in messages/)
    • supported languages: languages that we claim to support by acknowledging their existence in the languages.json file.
    • usable languages: a subset of supported languages that can actually be used in the app. this is based on checking the intersection of translated languages and supported languages.

    Using this categorization, the useResolvedLanguageTag() hook basically returns the best fitting "usable" language

Some implementation concerns/questions

Use existing storage key or not?

As I refactored the locale state setup, I was trying to decide between two options:

  1. Use the existing key that's used for persisting the locale (set up by the usePersistedLocale() hook implementation)
  2. Use a different key

At the time of writing this, I've currently chosen (1) for the sake of backwards compatibility. (2) would be easier and less work, but in the worst case, it would mean that anyone who did explicitly select a language before would lose that selection. The app would be presented based on the system preference or the English fallback. Personally I don't find this to be a major issue but I can imagine it being quite surprising, especially for low tech users.

In choosing (1), I've updated the shape of the storage to be more forwards compatible. There's a zustand migration that i've implemented to migrate from version 0 (the default version associated with previous implementation) and version 1 (this new implementation). You may think this migration implementation is unnecessary work. In terms of what's stored, that's a valid criticism, but I would argue that we should be getting in the habit of versioning all of our persisted stores and implementing migrations where appropriate (such is the consequence of offline + local first!)

I'm not too sure what the best way to test the migrations is, as it ideally requires having the previous storage version persisted and then updating the app to use the new one in order to trigger the migration. Open to thoughts on how to go about that in a somewhat reliable and reproducible way.


Example of the app responding to changes in system preferences (no explicit choosing of language in in-app settings):

system-language-switch.mp4

@achou11 achou11 force-pushed the 978/language-preference-resolution branch from 2ac269d to 50de297 Compare February 26, 2025 23:16
@achou11 achou11 force-pushed the 978/language-preference-resolution branch from 50de297 to 71fb95f Compare February 27, 2025 18:28
@achou11 achou11 force-pushed the 978/language-preference-resolution branch from 71fb95f to 3a244a6 Compare February 27, 2025 18:49
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.

App does not use system language preference when expected
1 participant