fix(mobile): localize Just Lift Echo-mode UI for Italian (issue #540) - #542
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces localization support for the Italian language and resolves a rendering issue on iOS where the eccentric load percentage (e.g., "110%") was clipped due to system font metrics. It adds platform-specific implementations to retrieve the current language code, introduces Italian string resources, and formats the eccentric load percentage with a non-breaking space for Italian locales. Additionally, hardcoded UI strings in JustLiftScreen are replaced with localized resources. Feedback suggests caching the retrieved language code on iOS using a lazy property to avoid repeated reads from NSUserDefaults during Compose recompositions, and using stringArrayForKey to avoid unchecked casts.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| actual fun currentLanguageCode(): String { | ||
| val defaults = NSUserDefaults.standardUserDefaults | ||
| val languages: Any? = defaults.objectForKey("AppleLanguages") | ||
| @Suppress("UNCHECKED_CAST") | ||
| val list = languages as? List<String> | ||
| val first = list?.firstOrNull().orEmpty() | ||
| // AppleLanguages values are BCP-47 tags like "en-US" / "it-IT"; we only | ||
| // need the language subtag for the percent-format decision. | ||
| return first.substringBefore('-').lowercase() | ||
| } |
There was a problem hiding this comment.
Reading from NSUserDefaults on every call to currentLanguageCode() is inefficient, especially since this function is called during Jetpack Compose recompositions (e.g., in eccentricLoadLabel), which can occur many times per second.
Since iOS terminates the app process whenever the system language is changed, we can safely cache the language code in a lazy property. Additionally, we can use stringArrayForKey to avoid the unchecked cast and the @Suppress annotation.
private val cachedLanguageCode: String by lazy {
val list = NSUserDefaults.standardUserDefaults.stringArrayForKey("AppleLanguages")
val first = list?.firstOrNull() as? String ?: ""
first.substringBefore('-').lowercase()
}
actual fun currentLanguageCode(): String = cachedLanguageCodeThere was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8343c39bf4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <?xml version="1.0" encoding="utf-8"?> | ||
| <!-- Italian (Italiano) — Beta AI-generated translation for issue #540 --> | ||
| <resources> | ||
| <string name="eccentric_load_helper">Carico durante la fase eccentrica (discesa)</string> |
There was a problem hiding this comment.
Add missing Italian Echo card titles
For Italian locales, the two titles changed in JustLiftScreen now read Res.string.eccentric_load and Res.string.echo_level, but this new values-it file does not define either key. Those lookups therefore fall back to the base English strings, so the Echo-mode section still shows “Eccentric Load” and “Echo Level” in English even after selecting/advertising Italian; the other shipped locales already define these keys.
Useful? React with 👍 / 👎.
…c percent glyph (issue #540) On Italian iPhone (app 0.9.1), the Just Lift Echo-mode block rendered fully in English and the Eccentric Load dropdown value '110%' had its '%' glyph visually collide with the trailing '0' on iOS SF Pro at bodyLarge, making the value read as '11 U%' / '11U%'. Three layered localization defects: 1. JustLiftScreen.kt Echo-mode block (lines 491-666) used hard-coded English Kotlin literals and bound the raw EccentricLoad.displayName to the dropdown value with no Locale-aware formatter. 2. EccentricLoad.displayName is '<int>%' with no separator; iOS SF Pro drops the '%' glyph with effectively zero left-advance, causing the trailing-0 / '%' collision. 3. composeResources had values-{de,es,fr,nl} but no values-it directory, and iosApp Info.plist did not declare CFBundleLocalizations so Italian could not be served even if the literals were converted. Fix (bounded to the Echo-mode block): - Convert the 6 hard-coded English labels in the Echo-mode block to stringResource(Res.string.*) lookups (Eccentric Load, Echo Level, Rep Count Timing, rep_count_timing_top/bottom, eccentric_load_helper). - Add a locale-aware percentage formatter (formatEccentricLoad + eccentricLoadLabel) that emits '110 %' with a U+00A0 NBSP for any 'it' language code and '110%' for everything else. The NBSP is the canonical Italian typographic form and resolves the iOS SF Pro glyph collision by construction. The active language code is sourced from a new expect/actual currentLanguageCode() helper (Android: java.util.Locale, iOS: NSUserDefaults AppleLanguages first entry), which is the multiplatform-friendly alternative to LocalConfiguration that is not exposed in the Compose Multiplatform iOS klib (CMP 1.10.3). - Add EchoLevel stringResource lookup (echo_level_hard / harder / hardest / epic) so the segmented buttons and FilterChips pick up the per-locale translation. - Add values-it/strings.xml with Italian translations. - Add CFBundleLocalizations to iosApp Info.plist with the 6 shipped locales (en, de, es, fr, nl, it). Bounded scope: the wire protocol is unaffected — BlePacketFactory.createEchoCommand takes the integer percentage from WorkoutParameters, not displayName. EccentricLoad / EchoLevel / WorkoutMode.Echo enum displayName values are preserved (Hard/Harder/Hardest/Epic and 0%..150%) because they are consumed by BLE, tests, and CSV consumers. The Stall Detection / Rest Timer / Auto-stop labels in JustLiftScreen.kt (lines 685, 691, 721) are valid localization debt but explicitly out of scope for issue #540. Files: - shared/src/commonMain/kotlin/.../CurrentLanguage.kt (new, expect) - shared/src/androidMain/kotlin/.../CurrentLanguage.android.kt (new, actual) - shared/src/iosMain/kotlin/.../CurrentLanguage.ios.kt (new, actual) - shared/src/commonMain/kotlin/.../EccentricLoadLabels.kt (new, helpers) - shared/src/commonMain/kotlin/.../JustLiftScreen.kt (Echo-mode block) - shared/src/commonMain/composeResources/values/strings.xml (8 new keys) - shared/src/commonMain/composeResources/values-it/strings.xml (new dir) - iosApp/.../Info.plist (CFBundleLocalizations) - shared/src/commonTest/kotlin/.../EccentricLoadDisplayNameTest.kt (10/10 passing — 4 pre-fix invariants + 6 post-fix assertions covering the locale-aware formatter for en, it, empty, case-insensitive, the per-entry cross-locale invariant, and the iOS AppleLanguages subtag-extraction invariant) Verification: - ./gradlew :shared:compileKotlinIosArm64 BUILD SUCCESSFUL - ./gradlew :shared:compileAndroidHostTest BUILD SUCCESSFUL - ./gradlew :shared:testAndroidHostTest --tests EccentricLoadDisplayNameTest 10/10 pass, 0 failures, 0 errors - plutil -lint Info.plist OK Fixes #540 Cross-references #539 (sibling Weight per Cable decimal-comma bug shares the same locale-aware formatter pattern; not addressed here). GPT-5.5/default retains final merge authority; MiniMax/phoenixworker will not merge this PR.
8343c39 to
0d9a50d
Compare
Code Review Roast 🔥Verdict: No Issues Found | Recommendation: Merge Oh wait, this PR is actually clean. I need to sit down. I had my flamethrower warmed up and everything. 📊 Overall: Like finding a unicorn in production — I didn't think clean PRs existed anymore, but here we are. Files Reviewed (9 files)
Reviewed by step-3.7-flash-20260528 · 187,317 tokens |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d9a50dac0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "PR" = PR (record personale) | ||
| Technical terms like "Echo", "TUT", "AMRAP", "1RM" stay in English. | ||
| --> | ||
| <resources> |
There was a problem hiding this comment.
Expose Italian in the language picker
Adding this values-it locale and advertising it in CFBundleLocalizations makes Italian a shipped locale, but the app’s own language dropdown still constructs only en, nl, de, es, and fr options in SettingsTab.kt (checked the languageOptions list at lines 1123-1128). In contexts where the user does not have system Italian selected, or has previously chosen another in-app language that is persisted via the existing locale helpers, these new Italian strings and the NBSP percent fix are not reachable from the app’s language setting. Please add the it option and its display string alongside this locale.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Summary
- Added Italian to the Settings language picker, including the generated resource import and the
"it"option inlanguageOptions. shared/src/commonMain/kotlin/com/devil/phoenixproject/presentation/screen/SettingsTab.ktL173-L178 shared/src/commonMain/kotlin/com/devil/phoenixproject/presentation/screen/SettingsTab.ktL1123-L1131 - Added the base
language_italianstring and the Italian resource file’s language-name entries so the picker can render Italian consistently. shared/src/commonMain/composeResources/values/strings.xmlL158-L164 shared/src/commonMain/composeResources/values-it/strings.xmlL15-L21 - Confirmed Italian Echo card titles are present in
values-itforecho_levelandeccentric_load. shared/src/commonMain/composeResources/values-it/strings.xmlL23-L30 - Updated iOS
currentLanguageCode()to cache the AppleLanguages lookup withlazyand usestringArrayForKey, avoiding repeatedNSUserDefaultsreads during recomposition. shared/src/iosMain/kotlin/com/devil/phoenixproject/domain/model/CurrentLanguage.ios.ktL15-L29 - Committed changes on the current branch with commit
c82d543and created the follow-up PR viamake_pr.
Testing
- ✅
./gradlew -Pskip.supabase.check=true :shared:compileKotlinIosArm64 - ✅
python3 - <<'PY' from pathlib import Path import xml.etree.ElementTree as ET for p in Path('shared/src/commonMain/composeResources').glob('values*/strings.xml'): ET.parse(p) print('parsed compose resource strings.xml files') PY - ✅
git diff --check ⚠️ ./gradlew :shared:compileKotlinIosArm64 :shared:compileAndroidHostTest(warning due to missing Supabase credentials in this environment)⚠️ ./gradlew -Pskip.supabase.check=true :shared:compileKotlinIosArm64 :shared:compileAndroidHostTest(warning due to missing Android SDK /ANDROID_HOMEin this environment)
Summary
Fixes #540 — iOS Italian locale: Eccentric percentage UI is clipped in Just Lift.
On Italian iPhone (app 0.9.1), the Just Lift Echo-mode block rendered fully in English and the Eccentric Load dropdown value
110%had its%glyph visually collide with the trailing0on iOS SF Pro at bodyLarge, making the value read as11 U%/11U%. Three layered localization defects:EccentricLoad.displayNameto the dropdown value with no Locale-aware formatter.EccentricLoad.displayNameis<int>%with no separator — iOS SF Pro drops the%glyph with effectively zero left-advance, causing the trailing-0 /%collision.composeResourceshadvalues-{de,es,fr,nl}but novalues-it— andiosApp/.../Info.plistdid not declareCFBundleLocalizationsso Italian could not be served even if the literals were converted.What this PR changes
stringResource(Res.string.*)lookups (Eccentric Load,Echo Level,Rep Count Timing,rep_count_timing_top/bottom,eccentric_load_helper); route the Eccentric Load dropdown value and dropdown items through a neweccentricLoadLabel()helper; route the EchoLevel FilterChip/SegmentedButton labels through a newechoLevelLabel()helper.currentLanguageCode()expect/actual (CurrentLanguage.kt+CurrentLanguage.android.kt+CurrentLanguage.ios.kt): multiplatform-friendly way to get the active app language. Android usesjava.util.Locale.getDefault().language; iOS reads the first entry ofNSUserDefaultsAppleLanguagesand strips the region subtag. This is the multiplatform alternative toLocalConfiguration.current.locales, which is not exposed in the Compose Multiplatform iOS klib (CMP 1.10.3).formatEccentricLoad(load, language)non-Composable core +eccentricLoadLabel(load)Composable wrapper: emits"110\u00A0%"(with U+00A0 NBSP) for anyitlanguage code and"110%"for every other locale. The NBSP is the canonical Italian typographic form and resolves the iOS SF Pro glyph collision by construction.values/strings.xml: 8 new keys (echo_level_hard/harder/hardest/epic,eccentric_load_helper,rep_count_timing,rep_count_timing_top/bottom). Existing keys untouched.values-it/strings.xml: full Italian translations.iosApp/.../Info.plist: addCFBundleLocalizationswith the 6 shipped locales (en, de, es, fr, nl, it) so iOS advertises Italian as a supported locale and the Compose Resources system can serve thevalues-itqualifier.EccentricLoadDisplayNameTest.ktincommonTest: 10 assertions — 4 pre-fix invariants pinning the unchanged enumdisplayNamecontract (BLE, tests, CSV rely on it) + 6 post-fix assertions covering the new locale-aware formatter (English ASCII, Italian NBSP, empty-language fallback, case-insensitive branch, per-entry cross-locale invariant, iOS AppleLanguages subtag-extraction invariant). Runs on both Android and iOS test targets.Bounded scope
BlePacketFactory.createEchoCommandtakes the integerpercentagefromWorkoutParameters, notdisplayName. BLE packets keep their existing bit layout.EccentricLoad/EchoLevel/WorkoutMode.EchoenumdisplayNamevalues are preserved (ASCIIHard/Harder/Hardest/Epicand0%..150%) because they are consumed by BLE, tests, CSV, and other code paths that depend on the raw form.JustLiftScreen.kt(lines 685, 691, 721) are valid localization debt but explicitly out of scope for issue iOS Italian locale: eccentric percentage UI is clipped in Just Lift #540. Cross-referenced inreferences/rca-issue-498-portal-leaderboard-tabs.mdpatterns and tracked as a follow-up.Verification
./gradlew :shared:compileKotlinIosArm64./gradlew :shared:compileAndroidHostTest./gradlew :shared:testAndroidHostTest --tests EccentricLoadDisplayNameTestplutil -lint iosApp/VitruvianPhoenix/VitruvianPhoenix/Info.plistPre-existing deprecation warnings in
CsvImporter.ios.kt/LocaleHelper.ios.kt/ForceCurveEngineTest.kt/DWSMRoutineFlowTest.ktare unrelated to this change.Files changed (9)
shared/src/commonMain/kotlin/.../CurrentLanguage.ktshared/src/androidMain/kotlin/.../CurrentLanguage.android.ktshared/src/iosMain/kotlin/.../CurrentLanguage.ios.ktshared/src/commonMain/kotlin/.../EccentricLoadLabels.ktshared/src/commonMain/kotlin/.../JustLiftScreen.ktshared/src/commonMain/composeResources/values/strings.xmlshared/src/commonMain/composeResources/values-it/strings.xmliosApp/VitruvianPhoenix/VitruvianPhoenix/Info.plistshared/src/commonTest/kotlin/.../EccentricLoadDisplayNameTest.ktRCA reference
Full RCA in commit history:
https://github.com/9thLevelSoftware/Project-Phoenix-MP/issues/540#issuecomment-4700342964. Recreation evidence:~/.hermes/phoenix-bug-recreations/mobile-issue-540-1515522415051145317/.GPT-5.5/default retains final merge authority; MiniMax/phoenixworker will not merge this PR.