uhryniuk
4c634354bd
V3-04 through V3-07, V3-10, V3-16 shipped complete; V3-11/V3-12/V3-14 shipped code-complete pending device/account verification; V3-08/V3-09 deferred behind a new V3-17 (self-hosted OSRM investigation). 316 tests passing, up from 221. The APK is a fresh release build (debug-signed, no release signing config exists yet) with two build fixes applied: core library desugaring enabled for flutter_local_notifications, and sentry_flutter bumped to 9.27.0 (8.14.2's bundled Kotlin plugin was incompatible with this project's Kotlin 2.4.0 toolchain).
88 lines
4.0 KiB
Markdown
88 lines
4.0 KiB
Markdown
# V3-03 — Distance and speed units
|
|
|
|
**Phase** Foundations · **Depends on** V3-02 · **Size** S · **Status** Done
|
|
|
|
## Goal
|
|
Imperial as well as metric, chosen once and applied everywhere.
|
|
|
|
## Context
|
|
Everything is hardcoded metric: `formatDistance` switches m/km at 1000, `formatSpeed`
|
|
prints km/h, elevation prints metres. Fine in Canada, useless to anyone in the US or UK.
|
|
|
|
Cheap, and the kind of thing that makes an app feel unfinished when missing.
|
|
|
|
## Design
|
|
A `UnitSystem` enum (`metric`, `imperial`) in `Config`, defaulting from the device locale
|
|
on first launch.
|
|
|
|
**Conversion belongs in formatting only.** Storage stays SI — metres, km/h, metres of
|
|
altitude — forever. Converting at the storage layer would corrupt every existing ride and
|
|
break the parity harness.
|
|
|
|
| Value | Metric | Imperial |
|
|
|---|---|---|
|
|
| Distance | m / km | ft / mi |
|
|
| Speed | km/h | mph |
|
|
| Elevation | m | ft |
|
|
|
|
## Implementation
|
|
1. `UnitSystem` in `Config`; default from `Platform.localeName`
|
|
2. Extend `ui/format.dart` — every formatter takes the unit system
|
|
3. Thread it through: record screen, trips list, trip detail, charts, map legend
|
|
4. Exports stay SI regardless. GPX is metres by specification; changing that breaks
|
|
consumers.
|
|
|
|
## Acceptance criteria
|
|
- [ ] Switching units updates every screen immediately
|
|
- [ ] Stored values are unchanged — verified by exporting before and after
|
|
- [ ] GPX/GeoJSON output is byte-identical across the two settings
|
|
- [ ] First launch picks a sensible default from the locale
|
|
|
|
## Tests
|
|
- Formatter tests for both systems, including the m→km and ft→mi boundaries
|
|
- **A test asserting export output does not change with the unit setting**
|
|
- Widget test toggling units and checking a rendered label
|
|
|
|
## Risks
|
|
The obvious trap is converting too deep in the stack. Guard it with the export test.
|
|
|
|
## Out of scope
|
|
Temperature, pace (min/km) — pace is arguably right for running, revisit after V3-01.
|
|
|
|
|
|
## Outcome
|
|
|
|
Built ahead of V3-02 in execution order, despite the ticket table listing it as
|
|
depending on V3-02 — the Settings screen needed something real to control, and the
|
|
formatting/`Config` plumbing itself has zero dependency on a screen existing. Both are
|
|
done; the numbering is unchanged.
|
|
|
|
`UnitSystem` lives in `domain/models.dart`, not `ui/format.dart` as first drafted —
|
|
`Config` (a data/preferences-layer class) needed the enum too, and having it depend on
|
|
`ui/` read backwards. Moved to the domain layer alongside `Activity`, which every other
|
|
cross-cutting preference-like enum in this codebase already does.
|
|
|
|
Reactivity reuses the exact pattern `mapEnabledProvider` already established —
|
|
`unitSystemProvider`, a `StateProvider<UnitSystem>` seeded from `Config` once and then
|
|
read/written directly by the UI — rather than introducing the heavier `ConfigNotifier`
|
|
class the ticket's implementation notes proposed. `Config` mutates its own backing
|
|
`SharedPreferences` in place, so a widget re-assigning the same `Config` instance to
|
|
`configProvider` was never going to notify anything; this sidesteps that without a new
|
|
abstraction.
|
|
|
|
Threaded through all three screens plus the speed histogram's bucket labels, which
|
|
convert-and-round for display (`formatSpeedRangeLabel`) without changing how
|
|
`speedHistogram` itself bins — binning stays km/h always, matching the invariant that
|
|
storage and computation never see the display unit. Export was the one place explicitly
|
|
*not* touched: `gpx()`/`geoJson()` take no `UnitSystem` parameter at all, which is a
|
|
stronger guarantee than validating one.
|
|
|
|
One real finding: `config_test.dart`'s locale-default test deliberately does not assert
|
|
a specific value, because it cannot know the test runner's own locale — and that caution
|
|
was immediately vindicated. `settings_screen_test.dart` first asserted a fresh `Config`
|
|
defaults to metric and failed, because this dev machine's own locale resolves to a
|
|
region in the imperial set. Fixed by seeding an explicit value before asserting, the
|
|
same technique already used elsewhere for exactly this reason.
|
|
|
|
22 tests: 13 `format_test.dart`, 9 `config_test.dart`.
|