Skip to content

chore: extract strings#245

Open
camrun91 wants to merge 2 commits into
mainfrom
YPE-1958-react-sdk-extract-strings-to-string-files
Open

chore: extract strings#245
camrun91 wants to merge 2 commits into
mainfrom
YPE-1958-react-sdk-extract-strings-to-string-files

Conversation

@camrun91
Copy link
Copy Markdown
Collaborator

@camrun91 camrun91 commented May 26, 2026

This PR puts in place the localization strings in the react repo

Greptile Summary

This PR extracts all hardcoded UI strings in the packages/ui package into react-i18next translation keys, wiring each component up with useTranslation(undefined, { i18n }) and adding the full English string catalog to en.json.

  • All 13 changed files consistently use the hook-based t() for reactivity; LoaderIcon and BibleVersionPickerLanguageTrigger were specifically fixed to drop their previous static defaults in favor of hook-derived translations.
  • getBibleTextErrorMessage in bible-text-error.ts is refactored to accept a TFunction argument, cleanly decoupling the error-message logic from any i18n singleton.
  • Two SVG icon components (BibleAppLogoLockup, Votd) were converted from arrow-function constants to named function components to accommodate the hook call.

Confidence Score: 5/5

Safe to merge — changes are purely additive string extraction with no behavioral modifications.

Every component correctly uses the hook-based t() for reactivity, the utility function refactor is cleanly scoped and TypeScript-enforced, and all previously flagged review issues (loader aria-label, share errorMessage default, and the server error key name) have been properly addressed.

No files require special attention.

Important Files Changed

Filename Overview
packages/ui/src/i18n/locales/en.json Adds 56 new translation keys for all extracted strings. Error keys use correct names (serverError, not serverErrorError). allLanguagesTab correctly uses {{count}} for future pluralization support.
packages/ui/src/lib/bible-text-error.ts Removes 7 hardcoded string constants; function now accepts TFunction parameter and delegates to t() for each error branch. Error status check ordering is unchanged.
packages/ui/src/components/YouVersionAuthButton.tsx Adds useTranslation hook; replaces hardcoded Sign in/out strings and mixed-content spans with t() and Trans component. t is correctly added to useMemo dependency array.
packages/ui/src/components/bible-version-picker.tsx Adds useTranslation to Trigger, BibleVersionPickerLanguageTrigger, Content, and BibleLanguagePickerContent; correctly replaces the static default aria-label parameter with a reactive t() fallback.
packages/ui/src/components/verse-of-the-day.tsx Refactors share() to accept errorMessage as a required parameter (removing the prior static default); the component passes t('unableToShare') reactively. useTranslation hook is correctly set up in VerseOfTheDay body.
packages/ui/src/components/icons/loader.tsx Now uses useTranslation hook for the aria-label, resolving the prior non-reactive i18n issue; allows aria-label prop override via nullish coalescing.

Reviews (2): Last reviewed commit: "fix: address greptile issues" | Re-trigger Greptile

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 26, 2026

⚠️ No Changeset found

Latest commit: 85c769c

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/icons/loader.tsx Outdated
Comment thread packages/ui/src/components/verse-of-the-day.tsx
Comment thread packages/ui/src/i18n/locales/en.json Outdated
Copy link
Copy Markdown
Collaborator

@bmanquen bmanquen left a comment

Choose a reason for hiding this comment

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

Just need to ensure that 'YouVersion' does not get translated. Besides that it looks good!

return isSignOut ? (
<div className="yv:font-normal">
Sign out of <span className="yv:font-bold">YouVersion</span>
<Trans
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We want to ensure that YouVersion does not get translated.

We should be able to add this prop to ensure YouVersion is not translated,
values={{ brand: 'YouVersion' }}.

You would also need to interpolate this variable in the strings file.

) : (
<div className="yv:font-normal">
Sign in with <span className="yv:font-bold">YouVersion</span>
<Trans
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Same as above.

Comment on lines +14 to +15
"signInWithYouVersion": "Sign in with <bold>YouVersion</bold>",
"signOutOfYouVersion": "Sign out of <bold>YouVersion</bold>",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Put 'YouVersion' as a varialble that you pass so that it is not translated.

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