Skip to content

Add Language detection and additional language support#254

Open
camrun91 wants to merge 3 commits into
mainfrom
localization
Open

Add Language detection and additional language support#254
camrun91 wants to merge 3 commits into
mainfrom
localization

Conversation

@camrun91
Copy link
Copy Markdown
Collaborator

@camrun91 camrun91 commented Jun 3, 2026

This puts into place the initial localized strings.

Greptile Summary

This PR introduces browser language detection and adds French and Spanish locale bundles to the YouVersion platform SDK, wiring up i18next to auto-select a locale on mount via syncBrowserLanguageFromNavigator.

  • Language detection (detectLanguage.ts) reads navigator.languages/navigator.language, resolves to a supported locale via exact-then-base BCP 47 matching, and is fully SSR-guarded.
  • i18n module (index.ts) adds fr and es resource bundles, calls getInitialLanguage() at module load for CSR paths, and exports syncBrowserLanguageFromNavigator as the runtime correction hook.
  • YouVersionProvider calls syncBrowserLanguageFromNavigator in a useEffect([]) on mount (already updated from a previous review to avoid the SSR useLayoutEffect warning).

Confidence Score: 5/5

The change is safe to merge — detection logic is SSR-guarded, locale bundles are complete, and the two-phase init + effect approach is correct.

The detection and resolution logic is well-structured with proper SSR guards and fallback handling. Bundled resources mean init completes before the effect fires, and the idempotent check in syncBrowserLanguageFromNavigator prevents unnecessary language changes.

No files require special attention.

Important Files Changed

Filename Overview
packages/ui/src/i18n/detectLanguage.ts New utility: SSR-safe browser language reading and BCP 47 resolution with exact-then-base fallback. Logic is clean and well-tested.
packages/ui/src/i18n/index.ts Adds fr/es bundles, getInitialLanguage() at module load, and syncBrowserLanguageFromNavigator export; changeLanguage errors are silently discarded via void.
packages/ui/src/components/YouVersionProvider.tsx Adds useEffect([]) to call syncBrowserLanguageFromNavigator on mount; straightforward and SSR-safe.
packages/ui/src/i18n/index.test.ts Comprehensive tests covering resolveBrowserLanguage, getBrowserLanguages, and full i18n instance initialisation per-language; uses vi.resetModules() correctly for isolation.
packages/ui/src/i18n/locales/es.json New Spanish locale; all 56 keys present, matching en.json structure.
packages/ui/src/i18n/locales/fr.json New French locale; all 56 keys present, matching en.json structure.

Sequence Diagram

sequenceDiagram
    participant Browser
    participant Module as i18n/index.ts (module load)
    participant Detect as detectLanguage.ts
    participant i18next
    participant Provider as YouVersionProvider
    participant Effect as useEffect([])

    Browser->>Module: "import @/i18n"
    Module->>Detect: "getInitialLanguage() -> getBrowserLanguages()"
    Detect-->>Module: navigator.languages (or undefined in SSR)
    Module->>Detect: resolveBrowserLanguage(langs, supportedLngs, 'en')
    Detect-->>Module: resolved locale (e.g. 'fr')
    Module->>i18next: ".init({ lng: 'fr', resources, supportedLngs, fallbackLng })"
    i18next-->>Module: initialized (async, resources are inline)

    Browser->>Provider: mount YouVersionProvider
    Provider->>Effect: schedule useEffect
    Effect->>Detect: "syncBrowserLanguageFromNavigator() -> getBrowserLanguages()"
    Detect-->>Effect: navigator.languages
    Effect->>Detect: resolveBrowserLanguage(...)
    Detect-->>Effect: resolved locale
    Effect->>i18next: "changeLanguage(locale) if i18n.language != locale"
Loading

Fix All in Claude Code Fix All in Cursor Fix All in Codex

Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
packages/ui/src/i18n/index.ts:31
`void i18n.changeLanguage(detected)` silently swallows any rejection. With bundled resources this is unlikely to fail, but if init is still in progress or i18next is in a bad state the language will silently remain wrong with no diagnostic. A minimal `.catch` keeps the code non-blocking while at least surfacing the failure in dev.

```suggestion
    i18n.changeLanguage(detected).catch((err: unknown) => {
      console.error('[youversion-sdk] Failed to change language:', err);
    });
```

Reviews (3): Last reviewed commit: "Merge branch 'main' into localization" | Re-trigger Greptile

@camrun91 camrun91 requested a review from cameronapak June 3, 2026 21:07
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Jun 3, 2026

⚠️ No Changeset found

Latest commit: 0ddd088

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Comment thread packages/ui/src/components/YouVersionProvider.tsx Outdated
Comment thread packages/ui/src/components/YouVersionProvider.tsx Outdated
Comment thread packages/ui/src/i18n/index.ts
cameronapak
cameronapak previously approved these changes Jun 5, 2026
Copy link
Copy Markdown
Collaborator

@cameronapak cameronapak left a comment

Choose a reason for hiding this comment

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

I like this PR. Good work. I am waiting on one response from Greptile

Comment thread packages/ui/src/i18n/index.ts
Comment thread packages/ui/src/components/YouVersionProvider.tsx Outdated
@camrun91
Copy link
Copy Markdown
Collaborator Author

camrun91 commented Jun 5, 2026

@cameronapak I got the greptile things addressed

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