Feature/spanish i18n - #201
Conversation
starbrightlab
left a comment
There was a problem hiding this comment.
Thanks for splitting this out onto its own branch — much easier to consider on its own. Three things, one a straightforward bug and one that's a decision rather than a defect.
The Home Assistant setup steps will render their numbers twice
MultiRoomStep(n, text) draws the number itself:
private fun MultiRoomStep(n: String, text: String) {
Row(...) {
Text("$n.", ...) // ← the number
Text(text, ...)
}
}The Spanish translations embed their own on top of that:
"In Home Assistant, add the Mosquitto broker add-on…" -> "1. En Home Assistant, añade el complemento Mosquitto broker…"
"Turn on the toggle below and enter your broker's address…" -> "2. Activa el interruptor…"
"This Portal appears automatically under Settings → Devices…" -> "3. Este Portal aparecerá automáticamente…"
So all three steps on the Home Assistant screen render as "1. 1. En Home Assistant…". The English originals correctly carry no number. Fix is to drop the 1. / 2. / 3. prefixes from those three strings. Worth grepping the rest of the map for the same pattern anywhere else a caller supplies the marker.
Needs a rebase, and it'll be a real one
The base here is 13432318. Since then SettingsDomains.kt and ImmortalSettingsActivity.kt have both changed substantially across #208 and #210 — the MQTT screen gained a sensors section and then lost a row again, and the mqtt and immortal domains both changed shape. This PR touches both files heavily, so expect genuine conflicts rather than a clean replay. The green CI here is from 11 Aug and doesn't reflect the current tree.
The architecture is worth an explicit decision
This introduces a 779-line I18n.kt that maps hardcoded English strings to translations at runtime, called as I18n.translate("Some English sentence", userLang) at each site. That's a bigger commitment than the diff suggests: every future string in the app inherits the choice, and the English text becomes a lookup key, so a later copy edit silently drops the translation back to English with nothing to catch it.
Android's own resources are the conventional alternative (strings.xml + values-es/strings.xml, which this PR already adds a small one of) — compile-time keys, no key drift, per-locale fallback handled by the platform, and no runtime map. The tradeoff is a much larger mechanical diff to extract the strings.
I'm not asking you to redo it — I'd just rather @starbrightlab make that call deliberately before it lands, since it's hard to reverse once the codebase is written against it. Flagging it as a question, not a change request.
The translations themselves read well, and the coverage of the Home Assistant screens in particular is thorough.
Generated by Claude Code
…e aplicaciones al espanol
9d9db40 to
727641b
Compare
|
Thanks — that's a fair thing to flag, and I agree The argument that convinces me isn't key drift so much as contributor cost: once Spanish lands, German/French/Portuguese will follow, and with a single 779-line One caveat on scope: the App Store strings in this PR include app descriptions. If those ever come from a remote catalog or from APK metadata they can't live in The good news is that this isn't a one-way door. On the concrete asks: the Rebased onto current One thing to flag because it goes beyond the original diff: the sensors and camera sections that landed in |
Thanks for shipping the Google Photos fix in v1.65!
As requested, I've created a dedicated feature branch
feature/spanish-i18nrebased onto current main (v1.67) with only the i18n changes, and opened a fresh PR for it.I've also rebased the screensaver transition PR (#166) on top of current main. Thanks!