a11y: Chart VoiceOver descriptors + localize chart-mark strings#2121
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughChart accessibility metadata is localized across metrics, weather, detection, altitude, PAX, and power charts. Shared chart series now format readings with optional units and localized fallback text, while decorative chart marks are hidden from VoiceOver. ChangesAccessibility localization
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
# Conflicts: # Localizable.xcstrings
garthvh
left a comment
There was a problem hiding this comment.
Approved after detailed review (CodeRabbit was rate-limited and did not run). Chart VoiceOver descriptors are data-accurate and localization keys verified; CI green. Merged current main to resolve the Localizable.xcstrings key conflict (union of both key sets, JSON validated).
What changed?
Makes the Swift Charts metric graphs navigable by VoiceOver (they previously had no per-point accessibility) and localizes the hardcoded English chart-mark strings.
MetricsChartSeries— every series mark gets a per-mark.accessibilityLabel(localized metric name + timestamp) and.accessibilityValue(reading + units) via the centralchartBodyClosure. Added an optionalunitfield to the series model (defaultnil), populated by the AirQuality/Environment series."Line Series"/"Bar Series"/"X: … Y: …"strings with per-metric localized labels/values; localized legend/foreground-style keys so legend and marks stay in sync;PositionAltitudeChartaltitude is now unit-aware (m/ft) and locale-formatted; hid internal threshold/selectionRuleMarks and decorative weather marks.Localizable.xcstringskeys.Design choice: per-mark labels/values instead of
.accessibilityChartDescriptor, because the shared charts render an arbitrary user-configurable mix of series where a static AXChart descriptor would be brittle.Why did it change?
The metric charts were opaque to VoiceOver — no way to read individual data points — and the deferred chart-mark strings were hardcoded English. This makes the data readable and the strings translatable.
How is this tested?
Built the
Meshtasticscheme for iOS Simulator (iPhone 17 Pro, Xcode 26) →** BUILD SUCCEEDED **. SwiftLint clean. Passed a focused Swift/SwiftUI review confirming the shared-modelunitchange is safe for every consumer (unitless path exercised), allString(localized:)keys resolve, number/measurement formatting is locale-correct, and the catalog edits are clean-additive. A live VoiceOver pass over the charts is recommended before merge.Screenshots/Videos (when applicable)
N/A — no visual change; adds VoiceOver descriptors to existing charts.
Checklist
docs/user/ordocs/developer/. No doc update is needed (VoiceOver descriptors only, no UI/flow change) — please apply theskip-docs-checklabel.Summary by CodeRabbit