Skip to content

i18n: extract 3 StrandDesign a11y labels via existing translations (#920) - #1038

Open
digitalerdude wants to merge 2 commits into
ryanbr:mainfrom
digitalerdude:fix/920-french-extraction
Open

i18n: extract 3 StrandDesign a11y labels via existing translations (#920)#1038
digitalerdude wants to merge 2 commits into
ryanbr:mainfrom
digitalerdude:fix/920-french-extraction

Conversation

@digitalerdude

Copy link
Copy Markdown

Summary

Scoped subset of #920: 3 of the 209 pending strings whose exact English key already has full, reviewed translations elsewhere in the repo, so they can be extracted with zero new translation work.

  • Next day / Previous day (DayNavBar.swift) and Trend (TrendChart.swift) were unextracted literals in StrandDesign's catalog (accessibility labels).
  • Each key already exists, fully translated (de/es/fr/pt-PT plus it/ru/zh-Hans/zh-Hant), in Strand/Resources/Localizable.xcstrings.
  • Copied the existing localizations blocks 1:1 into Packages/StrandDesign/Sources/StrandDesign/Resources/Localizable.xcstrings — no invented or machine-translated values.
  • Removed the 3 corresponding entries from Tools/i18n_audit_baseline.json.
  • Tools/i18n_extra_locale_baseline.txt untouched: all non-focus locales StrandDesign ships (it/ru/zh-Hans/zh-Hant) were carried over too, so the ratcheting allowance doesn't need to change.

Why not the full 209 from #920/#884: the rest need real de/es/pt-PT translations before they can pass the focus-locale gate — the #884 attachment only supplies French. That's separate follow-up work, not something this PR attempts.

Cross-platform: no Android change — this is a StrandDesign accessibility-label catalog fix, not an analytics/schema/stored-value change, so the byte-parity contract doesn't apply here.

Test plan

  • python3 Tools/i18n_audit.py --ci HEAD → exit 0, no FAIL, all four focus locales OK for the touched catalog
  • python3 Tools/test_i18n_audit.py → 36 tests OK
  • Verified JSON validity of both changed files
  • Verified git status touches only the 2 intended files

…yanbr#920)

Next day/Previous day (DayNavBar) and Trend (TrendChart) were unextracted
literals in StrandDesign's catalog even though the exact same English key
already has full de/es/fr/pt-PT (+it/ru/zh) translations in the Strand
catalog. Copy those existing, already-reviewed values into StrandDesign's
Localizable.xcstrings instead of inventing new ones.

Scoped to the subset of ryanbr#920 that needs zero new translation: 3 of the
209 pending strings whose key already exists elsewhere, fully translated.
The rest still needs real de/es/pt-PT input before it can pass the focus-
locale gate, since the source (ryanbr#884) only supplies French.

Removes the 3 corresponding entries from Tools/i18n_audit_baseline.json.
i18n_extra_locale_baseline.txt untouched: all shipped non-focus locales
(it/ru/zh-Hans/zh-Hant) were carried over too, so no allowance count grows.
@digitalerdude
digitalerdude marked this pull request as ready for review August 2, 2026 08:11
@ryanbr

ryanbr commented Aug 2, 2026

Copy link
Copy Markdown
Owner

The approach is right — reusing reviewed translations instead of generating new ones is exactly how to carve a shippable slice out of #920, and the audit accounting (baseline entries, extra-locale allowance untouched) is careful. But the three strings won't resolve from the catalog this adds them to, and for one of them the PR removes the only thing tracking a real gap.

The catalog entries won't be consulted

All three call sites resolve against Bundle.main, not StrandDesign's bundle:

DayNavBar.swift:58    .accessibilityLabel("Previous day")
DayNavBar.swift:100   .accessibilityLabel("Next day")
TrendChart.swift:341  .accessibilityLabel(Text(accessibilityLabel ?? "Trend"))

accessibilityLabel(_ key: LocalizedStringKey) has no bundle parameter, so SwiftUI looks in the main bundle. StrandDesign's catalog ships to the module bundle (Package.swift: .process("Resources")), so it is never searched from these sites.

The convention is already established in the same file — DayNavBar.swift:63 gets it right:

Text(label, bundle: .module)

as does Appearance.swift (String(localized: "Default", bundle: .module)).

Two of them already work; the third cannot

Next day, Previous day and Trend are all already present and translated in Strand/Resources/Localizable.xcstrings — 8, 8 and 9 locales. That IS the main bundle. So Next day / Previous day are already rendering in German today, resolved from the app catalog, and this PR doesn't change what a user sees.

Trend is the one that matters, and it goes the other way. accessibilityLabel is declared String? (line 53), so Text(accessibilityLabel ?? "Trend") binds to Text.init<S: StringProtocol> — the non-localizing initializer. That string cannot be localized from any catalog, in any bundle, without a code change.

So after this PR: two entries duplicate translations that nothing reads, and the third has its baseline entry removed while remaining genuinely un-localized. That is the part I'd push back on — the backlog stops tracking the one string that is actually broken.

What would make it real

Three call-site edits, and the catalog entries become the resolution source:

// DayNavBar.swift
.accessibilityLabel(Text("Previous day", bundle: .module))
.accessibilityLabel(Text("Next day", bundle: .module))

// TrendChart.swift
.accessibilityLabel(accessibilityLabel.map(Text.init) ?? Text("Trend", bundle: .module))

That also makes StrandDesign self-contained rather than borrowing its host app's catalog, which is the point of the audit rule the baseline was tracking. With those, the 162 added lines earn their keep and the baseline removals are honest.

Worth knowing generally

i18n_audit treats "present in a catalog" as extracted, which is the right check for the backlog but does not imply "localized at runtime". A package string needs BOTH the catalog entry and a bundle: .module lookup. Green CI here means the first, not the second — worth keeping in mind for the remaining 206 of #920, since the same pattern would repeat.

…#920)

The catalog entries added earlier don't resolve at runtime from these call
sites: .accessibilityLabel(LocalizedStringKey) and Text(_:) look in
Bundle.main, not StrandDesign's module bundle (Package.swift ships it via
.process("Resources")). So Next day/Previous day only rendered localized
because the host app's catalog happens to carry them, and Trend bound to the
non-localizing Text(_ content: StringProtocol) initializer via the ?? String
fallback — unlocalizable from any catalog without a code change.

Route all three through the module bundle, matching the convention already
used one line over (DayNavBar Text(label, bundle: .module)) and in
Appearance.swift. This makes StrandDesign self-contained and makes the
baseline removals honest — the strings are now both extracted and localized.
@digitalerdude

Copy link
Copy Markdown
Author

Confirmed all three, fixed in e5d4ac7 — the catalog half alone was resolving against Bundle.main, not StrandDesign's module bundle, so it was dead weight (and worse for Trend).

Call sites now route through the module bundle, matching the convention one line over in DayNavBar.swift:63 and in Appearance.swift:

// DayNavBar.swift
.accessibilityLabel(Text("Previous day", bundle: .module))
.accessibilityLabel(Text("Next day", bundle: .module))

// TrendChart.swift
.accessibilityLabel(accessibilityLabel.map(Text.init) ?? Text("Trend", bundle: .module))

For Trend: when a caller passes a series name it's shown verbatim via Text.init; when nil it now falls back to the localizing Text("Trend", bundle: .module) instead of the ?? String path that bound to the non-localizing initializer. So the baseline removal is honest now — the three strings are both extracted and resolved from the package's own catalog, which makes StrandDesign self-contained rather than borrowing the host app's catalog.

Verified: swift build on StrandDesign compiles (the .map(Text.init) reference isn't ambiguous), and i18n_audit.py --ci stays green. Left Pick a date (DayNavBar:86) alone — it still needs real de/es/pt-PT before it can be extracted, separate PR. Good point on the runtime-vs-catalog distinction for the remaining strings in #920; same bundle: .module requirement will apply to any package-owned ones.

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