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 6 commits into
base: develop
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/frontend/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {getSentryUserId} from './metrics/getSentryUserId';
import {AppDiagnosticMetrics} from './metrics/AppDiagnosticMetrics';
import {DeviceDiagnosticMetrics} from './metrics/DeviceDiagnosticMetrics';
import {createDraftObservationStore} from './contexts/PersistedStores/DraftObservationStore';
import {createSelectedLocaleStore} from './contexts/SelectedLocaleContext';

type SentryEnvironment = 'development' | 'qa' | 'production';

Expand Down Expand Up @@ -77,6 +78,10 @@ const persistedDraftObservationStore = createDraftObservationStore({
persist: true,
});

const persistedSelectedLocaleStore = createSelectedLocaleStore({
persist: true,
});

const App = () => {
const [permissionsAsked, setPermissionsAsked] = React.useState(false);
React.useEffect(() => {
Expand All @@ -96,7 +101,8 @@ const App = () => {
mapeoApi={mapeoApi}
appMetrics={appDiagnosticMetrics}
deviceMetrics={deviceDiagnosticMetrics}
persistedDrafObservationStore={persistedDraftObservationStore}>
persistedDrafObservationStore={persistedDraftObservationStore}
selectedLocaleStore={persistedSelectedLocaleStore}>
<AppNavigator permissionAsked={permissionsAsked} />
</AppProviders>
);
Expand Down
34 changes: 34 additions & 0 deletions src/frontend/__mocks__/expo-localization.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import {
getLocales as _getLocales,
useLocales as _useLocales,
type Locale,
} from 'expo-localization';

import {extractLanguageCode} from '../lib/intl';

export const getLocales: typeof _getLocales = () => {
return [createBaseLocale('en-US')];
};

// eslint-disable-next-line @eslint-react/hooks-extra/no-useless-custom-hooks
export const useLocales: typeof _useLocales = () => {
return [createBaseLocale('en-US')];
};

function createBaseLocale(languageTag: string): Locale {
return {
languageTag,
languageCode: extractLanguageCode(languageTag),
langageCurrencyCode: null,
langageCurrencySymbol: null,
languageRegionCode: null,
regionCode: null,
currencyCode: null,
currencySymbol: null,
decimalSeparator: null,
digitGroupingSeparator: null,
textDirection: null,
measurementSystem: null,
temperatureUnit: null,
};
}
78 changes: 44 additions & 34 deletions src/frontend/contexts/AppProviders.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ import {AppDiagnosticMetrics} from '../metrics/AppDiagnosticMetrics';
import {DeviceDiagnosticMetrics} from '../metrics/DeviceDiagnosticMetrics';
import {DraftObservationProvider} from './DraftObservationContext';
import {DraftObservationStore} from './PersistedStores/DraftObservationStore';
import {
SelectedLocaleStore,
SelectedLocaleStoreProvider,
} from './SelectedLocaleContext';

type AppProvidersProps = {
children: React.ReactNode;
Expand All @@ -37,6 +41,7 @@ type AppProvidersProps = {
appMetrics: AppDiagnosticMetrics;
deviceMetrics: DeviceDiagnosticMetrics;
persistedDrafObservationStore: DraftObservationStore;
selectedLocaleStore: SelectedLocaleStore;
};

const queryClient = new QueryClient();
Expand All @@ -49,42 +54,47 @@ export const AppProviders = ({
appMetrics,
deviceMetrics,
persistedDrafObservationStore,
selectedLocaleStore,
}: AppProvidersProps) => {
return (
<IntlProvider>
<QueryClientProvider client={queryClient}>
<SafeAreaProvider>
<GestureHandlerRootView style={styles.flex}>
<TrackTimerContextProvider>
<GPSModalContextProvider>
<ServerLoading messagePort={messagePort}>
<LocalDiscoveryProvider value={localDiscoveryController}>
<ClientApiProvider clientApi={mapeoApi}>
<MetricsProvider
appMetrics={appMetrics}
deviceMetrics={deviceMetrics}>
<ActiveProjectProvider>
<BottomSheetModalProvider>
<PhotoPromiseProvider>
<DraftObservationProvider
draftObservationStore={
persistedDrafObservationStore
}>
<SecurityProvider>{children}</SecurityProvider>
</DraftObservationProvider>
</PhotoPromiseProvider>
</BottomSheetModalProvider>
</ActiveProjectProvider>
</MetricsProvider>
</ClientApiProvider>
</LocalDiscoveryProvider>
</ServerLoading>
</GPSModalContextProvider>
</TrackTimerContextProvider>
</GestureHandlerRootView>
</SafeAreaProvider>
</QueryClientProvider>
</IntlProvider>
<SelectedLocaleStoreProvider value={selectedLocaleStore}>
<IntlProvider>
<QueryClientProvider client={queryClient}>
<SafeAreaProvider>
<GestureHandlerRootView style={styles.flex}>
<TrackTimerContextProvider>
<GPSModalContextProvider>
<ServerLoading messagePort={messagePort}>
<LocalDiscoveryProvider value={localDiscoveryController}>
<ClientApiProvider clientApi={mapeoApi}>
<MetricsProvider
appMetrics={appMetrics}
deviceMetrics={deviceMetrics}>
<ActiveProjectProvider>
<BottomSheetModalProvider>
<PhotoPromiseProvider>
<DraftObservationProvider
draftObservationStore={
persistedDrafObservationStore
}>
<SecurityProvider>
{children}
</SecurityProvider>
</DraftObservationProvider>
</PhotoPromiseProvider>
</BottomSheetModalProvider>
</ActiveProjectProvider>
</MetricsProvider>
</ClientApiProvider>
</LocalDiscoveryProvider>
</ServerLoading>
</GPSModalContextProvider>
</TrackTimerContextProvider>
</GestureHandlerRootView>
</SafeAreaProvider>
</QueryClientProvider>
</IntlProvider>
</SelectedLocaleStoreProvider>
);
};

Expand Down
26 changes: 14 additions & 12 deletions src/frontend/contexts/IntlContext.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import * as React from 'react';
import {IntlProvider as ReactIntlProvider, CustomFormats} from 'react-intl';
import {CustomFormats, IntlProvider as ReactIntlProvider} from 'react-intl';
import {StyleSheet, Text} from 'react-native';

import messages from '../../../translations/messages.json';
import {usePersistedLocale} from '../hooks/persistedState/usePersistedLocale';
import {TranslatedLocale} from '../lib/intl';
import {useResolvedLanguageTag} from '../hooks/useResolvedLanguageTag';
import {extractLanguageCode, type TranslatedLanguageTag} from '../lib/intl';

export const formats: CustomFormats = {
date: {
Expand All @@ -25,20 +25,22 @@ const DEFAULT_RICH_TEXT_MAPPINGS: NonNullable<
};

export const IntlProvider = ({children}: {children: React.ReactNode}) => {
const appLocale = usePersistedLocale(store => store.locale);
const resolvedLanguageTag = useResolvedLanguageTag();

const languageCode = appLocale.split('-')[0];
const messagesToUse = React.useMemo(() => {
const languageCode = extractLanguageCode(resolvedLanguageTag.value);

// Add fallbacks for non-regional locales (e.g. "en" for "en-GB")
const localeMessages = {
...messages[languageCode as TranslatedLocale],
...(messages[appLocale as TranslatedLocale] || {}),
};
return {
// Add fallbacks for non-regional tags (e.g. "en" for "en-GB")
...(messages[languageCode as TranslatedLanguageTag] || {}),
...(messages[resolvedLanguageTag.value as TranslatedLanguageTag] || {}),
};
}, [resolvedLanguageTag.value]);

return (
<ReactIntlProvider
locale={appLocale}
messages={localeMessages}
locale={resolvedLanguageTag.value}
messages={messagesToUse}
formats={formats}
onError={onError}
wrapRichTextChunksInFragment
Expand Down
103 changes: 103 additions & 0 deletions src/frontend/contexts/SelectedLocaleContext.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
import {createContext, useContext} from 'react';
import {createStore, useStore, type StoreApi} from 'zustand';
import {
createJSONStorage,
persist as createPersistedState,
} from 'zustand/middleware';

import {MMKVZustandStorage} from '../hooks/persistedState/createPersistedState';

export const STORAGE_KEY = 'MapeoLocale';
Copy link
Member Author

Choose a reason for hiding this comment

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

just noting that this is the same storage key that is currently used for persisting the locale in the app.


export type SelectedLocaleState = {
/**
* Value consisting of a language tag (see https://en.wikipedia.org/wiki/IETF_language_tag)
* Represents the language that is explicitly chosen via a user action within the app. If null, it means that either:
*
* 1. The user has never chosen the language explicitly.
* 2. The user has unset the language (e.g. to defer to system preferences)
*/
languageTag: string | null;
};

function createInitialState() {
return {
languageTag: null,
};
}

export function createSelectedLocaleStore({persist} = {persist: false}) {
let store: StoreApi<SelectedLocaleState>;

if (persist) {
store = createStore(
createPersistedState(createInitialState, {
name: STORAGE_KEY,
storage: createJSONStorage(() => MMKVZustandStorage),
version: 1,
migrate: (persistedState, version) => {
/**
* Version 0 stores the state as `{ locale: string, setLocale: (locale: string) => void }`.
* We only need to handle the `locale` field, which is more specifically a language tag.
*/
if (version === 0) {
// Ensure that the persisted state for version has expected shape before attempting to migrate
if (
typeof persistedState === 'object' &&
persistedState !== null &&
'locale' in persistedState &&
typeof persistedState.locale === 'string'
) {
// TODO: log to Sentry to help understand how often this is happening?
return {languageTag: persistedState.locale};
}
}

return {languageTag: null};
},
Comment on lines +38 to +57
Copy link
Member Author

Choose a reason for hiding this comment

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

Highlighting this migration implementation since it's important to confirm that it's doing the right thing (since it's kind of hard to test...).

also would help to get some input on whether to log to sentry (see todo comments here)

Copy link
Member Author

Choose a reason for hiding this comment

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

i have an idea of how to go about writing tests for this, but would require a decent chunk of additional work I think. might wait for input before trying.

}),
);
} else {
store = createStore(createInitialState);
}

const actions = {
setLanguageTag: (languageTag: string | null) => {
store.setState({languageTag});
},
};

return {instance: store, actions};
}

export type SelectedLocaleStore = ReturnType<typeof createSelectedLocaleStore>;

const SelectedLocaleContext = createContext<SelectedLocaleStore | null>(null);

export const SelectedLocaleStoreProvider = SelectedLocaleContext.Provider;

function useSelectedLocaleContext() {
const value = useContext(SelectedLocaleContext);

if (!value) {
throw new Error('Must set up SelectedLocaleStoreProvider first');
}

return value;
}

export function useSelectedLocaleState(): SelectedLocaleState;
export function useSelectedLocaleState<T>(
selector: (state: SelectedLocaleState) => T,
): T;
export function useSelectedLocaleState<T>(
selector?: (state: SelectedLocaleState) => T,
) {
const {instance} = useSelectedLocaleContext();
return useStore(instance, selector!);
}

export function useSelectedLocaleActions() {
const {actions} = useSelectedLocaleContext();
return actions;
}
2 changes: 1 addition & 1 deletion src/frontend/hooks/persistedState/createPersistedState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ type PersistedStoreKey =
| 'ActiveProjectId'
| 'Settings'
| 'MetricDiagnosticsPermission';
const MMKVZustandStorage: StateStorage = {
export const MMKVZustandStorage: StateStorage = {
Copy link
Member Author

Choose a reason for hiding this comment

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

probably should either:

  1. do some work to clean up how we're setting up the persistence stuff
  2. move this variable elsewhere that feels easier to discover

kind of taking the lazy route for now

setItem: (name, value) => {
return storage.set(name, value);
},
Expand Down
26 changes: 0 additions & 26 deletions src/frontend/hooks/persistedState/usePersistedLocale.ts

This file was deleted.

4 changes: 2 additions & 2 deletions src/frontend/hooks/server/fields.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import {useQuery} from '@tanstack/react-query';
import {useActiveProject} from '../../contexts/ActiveProjectContext';
import {usePersistedLocale} from '../persistedState/usePersistedLocale';
import {useResolvedLanguageTag} from '../useResolvedLanguageTag';

export const FIELDS_KEY = 'fields';

export const useFieldsQuery = () => {
const {projectId, projectApi} = useActiveProject();
const lang = usePersistedLocale(store => store.locale);
const lang = useResolvedLanguageTag().value;

return useQuery({
queryKey: [FIELDS_KEY, projectId, lang],
Expand Down
8 changes: 4 additions & 4 deletions src/frontend/hooks/server/presets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,18 @@ import {
import {PresetValue} from '@comapeo/schema';

import {useActiveProject} from '../../contexts/ActiveProjectContext';
import {usePersistedLocale} from '../persistedState/usePersistedLocale';
import {useResolvedLanguageTag} from '../useResolvedLanguageTag';

export const PRESETS_KEY = 'presets';

export function usePresetsQuery() {
const {projectId, projectApi} = useActiveProject();
const locale = usePersistedLocale(store => store.locale);
const lang = useResolvedLanguageTag().value;

return useSuspenseQuery({
queryKey: [PRESETS_KEY, projectId, locale],
queryKey: [PRESETS_KEY, projectId, lang],
queryFn: async () => {
return await projectApi.preset.getMany({lang: locale});
return await projectApi.preset.getMany({lang});
},
});
}
Expand Down
Loading
Loading