Write FB-06..FB-09 feedback tickets (round two)
Turns the newly-added issues in docs/FEEDBACK.md into four more self-contained tickets: always-interactive map with a recenter button, Route Planner's Null Island centering bug, HUD reflow-on-hide, and title-driven HUD sizing. Sequenced FB-08 before FB-09 since both rework the same HUD layout files.
This commit is contained in:
153
docs/feedback/FB-06-always-interactive-map-recenter.md
Normal file
153
docs/feedback/FB-06-always-interactive-map-recenter.md
Normal file
@@ -0,0 +1,153 @@
|
||||
# FB-06 — Map is always pannable and zoomable, with a recenter control
|
||||
|
||||
**Depends on** — · **Size** M · **Status** Not started
|
||||
|
||||
## Goal
|
||||
The rider must be able to pan and zoom the map at all times. Today the map locks all
|
||||
interaction while idle. Add a recenter button. The button must bring the camera back to
|
||||
the rider's live position.
|
||||
|
||||
## Context
|
||||
Direct user feedback (`docs/FEEDBACK.md`):
|
||||
|
||||
> Map page, both when recording is enabled and disabled, you cannot zoom and move
|
||||
> around the map, it's be nice to still be able to move around and see the surrounding
|
||||
> area, and then tap a "center" icon to recenter on the user.
|
||||
>
|
||||
> User dot on the map should always glow and occilate in size like it does when
|
||||
> recording.
|
||||
|
||||
`lib/src/ui/components/ride_map.dart`, `_RideMapState.build()` (~line 270):
|
||||
|
||||
```dart
|
||||
interactionOptions: hasPoints
|
||||
? const InteractionOptions(
|
||||
flags: InteractiveFlag.pinchZoom | InteractiveFlag.drag,
|
||||
)
|
||||
: const InteractionOptions(flags: InteractiveFlag.none),
|
||||
```
|
||||
|
||||
`hasPoints` is `widget.points.isNotEmpty`. The shared background map is idle (no
|
||||
recorded points) any time no trip is recording. In that state `InteractiveFlag.none`
|
||||
blocks every pan and zoom gesture. This is the exact cause of "when recording is
|
||||
disabled you cannot zoom and move around the map."
|
||||
|
||||
During an active recording, `hasPoints` is true and pan/zoom are already allowed. The
|
||||
existing chase-camera logic already cancels auto-follow on a real gesture — see
|
||||
`didUpdateWidget` (~line 142) and `onPositionChanged` (~line 270):
|
||||
|
||||
```dart
|
||||
onPositionChanged: !widget.follow
|
||||
? null
|
||||
: (position, hasGesture) {
|
||||
if (hasGesture && _following) {
|
||||
setState(() => _following = false);
|
||||
}
|
||||
},
|
||||
```
|
||||
|
||||
Once `_following` is set to `false`, nothing ever sets it back to `true` again — there
|
||||
is no way today to resume following the rider's live position after one manual pan.
|
||||
That is the missing "recenter" affordance the feedback names directly.
|
||||
|
||||
`_following` is a `late bool` field set once, at `State` creation
|
||||
(`late bool _following = widget.follow;`, ~line 128) — it never re-reads `widget.follow`
|
||||
on a later rebuild. A recenter action must set this field back to `true` directly
|
||||
(inside `_RideMapState`, since it owns the field) — there is no way to do this from
|
||||
outside the widget today, and there should not be; this stays internal.
|
||||
|
||||
`lib/src/ui/components/pulsing_location_marker.dart` already animates continuously
|
||||
(`AnimationController.repeat()` in `didChangeDependencies`, unless the platform's
|
||||
reduced-motion setting is on) and is the same widget instance used for both the idle
|
||||
(ambient) marker and the recording marker — see `ride_map.dart` ~line 307:
|
||||
|
||||
```dart
|
||||
if (widget.showLocationMarker && (hasPoints || widget.ambientPosition != null))
|
||||
MarkerLayer(
|
||||
markers: [
|
||||
Marker(
|
||||
key: const Key('location-marker'),
|
||||
point: hasPoints ? ... : widget.ambientPosition!,
|
||||
width: 40,
|
||||
height: 40,
|
||||
child: const PulsingLocationMarker(),
|
||||
),
|
||||
],
|
||||
),
|
||||
```
|
||||
|
||||
The code already pulses the marker in both states. This ticket's job for the marker is
|
||||
to confirm, on a real device, that the pulse is actually visible in both states — not to
|
||||
assume the report is wrong. If the pulse is confirmed working, say so plainly in the
|
||||
Outcome section and make no code change for it.
|
||||
|
||||
## Design
|
||||
- **Always allow pan and zoom.** Remove the `hasPoints` gate on `interactionOptions`.
|
||||
Use `const InteractionOptions(flags: InteractiveFlag.pinchZoom | InteractiveFlag.drag)`
|
||||
unconditionally.
|
||||
- **Add a recenter button inside `RideMap`.** Show it only when `widget.follow` is true
|
||||
and `_following` is false — the exact state where the rider panned away from an
|
||||
actively-followed camera. Place it as a small circular icon button, bottom-right of
|
||||
the map, above the tile layer, using `Icons.my_location` and the app's existing
|
||||
`GlassPanel`/theme conventions (check `lib/src/ui/components/glass_panel.dart` for the
|
||||
existing floating-control pattern this app already uses, e.g. `FloatingPill`, and
|
||||
match it rather than inventing new chrome). Give it `key: const Key('recenter-button')`.
|
||||
On tap:
|
||||
1. Set `_following = true`.
|
||||
2. Move the camera to the latest known position: `widget.points.last` if
|
||||
`widget.points.isNotEmpty`, else `widget.ambientPosition` if it is not null. If
|
||||
neither is available, do nothing (no position to recenter on yet).
|
||||
3. Keep the current zoom level — do not force a specific zoom on recenter, since the
|
||||
rider may have deliberately zoomed in or out and recenter should not undo that.
|
||||
- **Do not show the recenter button when there is no `follow` mode at all** (e.g. a
|
||||
finished-ride static map in Trip Detail, where `follow` is always false) — the gate
|
||||
above (`widget.follow && !_following`) already excludes this case correctly.
|
||||
- **Marker verification.** Run the app on the Android emulator. Watch the location
|
||||
marker in the idle state and in the recording state. Confirm the pulse ring expands
|
||||
and fades in both states. Write the result in the Outcome section. Fix the code only
|
||||
if the pulse is genuinely missing in one of the two states — do not change the
|
||||
animation if it is already working.
|
||||
|
||||
## Implementation
|
||||
1. Remove the `hasPoints` conditional on `interactionOptions` in `ride_map.dart`. Use
|
||||
the pinch/drag flags unconditionally.
|
||||
2. Add a recenter button widget inside `_RideMapState.build()`, gated on
|
||||
`widget.follow && !_following`.
|
||||
3. Wire the button's `onPressed` to set `_following = true` and move the camera to the
|
||||
latest point or ambient position, keeping the current zoom.
|
||||
4. Run the app on the Android emulator. Confirm the marker pulses in both the idle and
|
||||
the recording state. Record the result in the ticket's Outcome section.
|
||||
|
||||
## Acceptance criteria
|
||||
- [ ] The map can be panned and zoomed while idle (no active recording).
|
||||
- [ ] The map can be panned and zoomed while recording.
|
||||
- [ ] Panning the map while `follow` is active shows a recenter button.
|
||||
- [ ] Tapping the recenter button returns the camera to the rider's latest known
|
||||
position and resumes following new position updates.
|
||||
- [ ] The recenter button does not appear on a static, non-following map (e.g. Trip
|
||||
Detail's finished-ride view).
|
||||
- [ ] The location marker's pulse animation is confirmed visible on-device in both the
|
||||
idle and the recording state, or fixed if it is not.
|
||||
- [ ] `flutter analyze` clean, `flutter test` green, test count only goes up.
|
||||
|
||||
## Tests
|
||||
- Widget test: `RideMap` with `hasPoints: false` (no recorded points) allows a pan
|
||||
gesture to change the camera position — assert the map's `InteractionOptions.flags`
|
||||
include `InteractiveFlag.drag`/`pinchZoom` regardless of `points`/`ambientPosition`.
|
||||
- Widget test: after a manual pan cancels following (`_following` set to `false` via
|
||||
the existing `onPositionChanged` gesture path — see the pattern already used in
|
||||
`test/ride_map_test.dart`'s "a manual pan cancels ambient following" test), the
|
||||
recenter button (`find.byKey(const Key('recenter-button'))`) appears.
|
||||
- Widget test: before any manual pan (or when `follow` is false), the recenter button
|
||||
does not appear.
|
||||
- Widget test: tapping the recenter button moves the camera back to the latest point
|
||||
(or ambient position) and the button disappears again (following resumed).
|
||||
|
||||
## Risks
|
||||
- None significant. This is additive (a new optional control) and a removed
|
||||
restriction (interaction gating), not a data-model or persistence change.
|
||||
|
||||
## Out of scope
|
||||
Any change to the Route Planner's own map (FB-07 covers its remaining issue). Any
|
||||
change to `PulsingLocationMarker`'s own animation code, unless the on-device check in
|
||||
this ticket finds it is genuinely not visible in one of the two states.
|
||||
149
docs/feedback/FB-07-route-planner-null-island.md
Normal file
149
docs/feedback/FB-07-route-planner-null-island.md
Normal file
@@ -0,0 +1,149 @@
|
||||
# FB-07 — Route Planner opens on Null Island instead of the rider's real location
|
||||
|
||||
**Depends on** FB-01 (reuses its ambient-location provider) · **Size** S/M · **Status** Not started
|
||||
|
||||
## Goal
|
||||
A new route, or a route with fewer than two pins, must open the map on the rider's real
|
||||
location. Today it opens on the middle of the ocean.
|
||||
|
||||
## Context
|
||||
Direct user feedback (`docs/FEEDBACK.md`):
|
||||
|
||||
> Pin drops still do not work at all, the map when placing the pins doesn't render at a
|
||||
> all, it's just a blank canvas that you can zoom in and out of but no detail appears.
|
||||
> The pins can also be placed but nothing is rendered on the map so you have no clue
|
||||
> where they are.
|
||||
|
||||
`lib/src/ui/routes/route_planner_screen.dart`, `build()` (~line 165):
|
||||
|
||||
```dart
|
||||
child: FlutterMap(
|
||||
mapController: _mapController,
|
||||
options: MapOptions(
|
||||
initialCameraFit: _initialFit(waypoints),
|
||||
initialCenter: waypoints.isEmpty
|
||||
? const ll.LatLng(0, 0)
|
||||
: ll.LatLng(waypoints.first.latitude, waypoints.first.longitude),
|
||||
initialZoom: waypoints.length <= 1 ? ambientZoom : maxTileZoom - 3,
|
||||
maxZoom: maxTileZoom,
|
||||
onTap: (tapPosition, point) {
|
||||
repo.addWaypoint(widget.routeId, point.latitude, point.longitude);
|
||||
...
|
||||
```
|
||||
|
||||
`ll.LatLng(0, 0)` is Null Island — a single point in the Atlantic Ocean with no land
|
||||
and no map detail at any zoom level. A brand-new route (0 pins), or a route with exactly
|
||||
1 pin dropped near that point, opens the map centered there. At street-level zoom, an
|
||||
area of open ocean legitimately renders as a flat, featureless expanse — this matches
|
||||
the report exactly: "a blank canvas... no detail appears," and a pin dropped anywhere
|
||||
near that same spot lands in the same empty area, with nothing else on screen to show
|
||||
where it is relative to.
|
||||
|
||||
`RoutePlannerScreen` manages its own `FlutterMap` directly — it does not use `RideMap`
|
||||
and does not currently read `ambientPositionProvider` at all (confirmed by grep: no
|
||||
reference to `ambientPositionProvider` anywhere in `route_planner_screen.dart`). FB-01
|
||||
already built exactly the provider this ticket needs — `lib/src/app/providers.dart`
|
||||
(~line 66):
|
||||
|
||||
```dart
|
||||
final ambientPositionProvider = StreamProvider.autoDispose<LocationFix?>((ref) async* {
|
||||
if (!ref.watch(mapEnabledProvider)) {
|
||||
yield null;
|
||||
return;
|
||||
}
|
||||
final source = ref.watch(locationSourceProvider);
|
||||
try {
|
||||
await source.start(); // idempotent; safe even if a recording already started it
|
||||
} on LocationException {
|
||||
yield null; // permission denied / service disabled — ambient mode is best-effort
|
||||
return;
|
||||
}
|
||||
yield* source.fixes.map<LocationFix?>((fix) => fix);
|
||||
});
|
||||
```
|
||||
|
||||
`LocationFix` is defined in `lib/src/recording/location_source.dart`.
|
||||
|
||||
**`initialCenter`/`initialZoom` are read exactly once, at `FlutterMap` construction —
|
||||
not on every rebuild.** The existing comment in this file already documents this
|
||||
constraint for the waypoints stream (~line 158): the screen waits for
|
||||
`waypointsAsync.hasValue` before building `FlutterMap` at all, specifically so the
|
||||
first real camera position is correct from the start. The same constraint applies here:
|
||||
reading `ambientPositionProvider` must happen before `FlutterMap` is constructed, not
|
||||
patched in afterward with a controller move — mirror the existing wait-for-first-value
|
||||
pattern, do not invent a new one.
|
||||
|
||||
## Design
|
||||
- Watch `ambientPositionProvider` in `build()`, the same way `ShellScaffold` already
|
||||
does (`lib/src/ui/app_shell.dart` ~line 74):
|
||||
```dart
|
||||
final ambientFix = ref.watch(ambientPositionProvider).valueOrNull;
|
||||
final ambientPosition = ambientFix == null
|
||||
? null
|
||||
: ll.LatLng(ambientFix.latitude, ambientFix.longitude);
|
||||
```
|
||||
- Use `ambientPosition` for `initialCenter` when there are fewer than 2 waypoints,
|
||||
falling back to `(0, 0)` only when no ambient fix is available yet (permission denied,
|
||||
service disabled, or the fix has not arrived yet):
|
||||
```dart
|
||||
initialCenter: waypoints.isEmpty
|
||||
? (ambientPosition ?? const ll.LatLng(0, 0))
|
||||
: ll.LatLng(waypoints.first.latitude, waypoints.first.longitude),
|
||||
```
|
||||
- Do **not** block the map on waiting for the ambient fix the way `waypointsAsync` is
|
||||
blocked on its own first value. A missing ambient fix must fall back to `(0, 0)`
|
||||
immediately, not show a spinner — the rider must always eventually reach a usable map,
|
||||
even with location permission denied. This matches the existing `RideMap` fallback
|
||||
behavior in FB-01 exactly (see `ride_map.dart`'s `bounds == null` branch).
|
||||
- Leave the 1-waypoint and 2-plus-waypoint camera logic unchanged — a route that already
|
||||
has a real pin should still center on that pin, not on the ambient position, since the
|
||||
pin is a stronger signal of where the rider actually wants to look.
|
||||
- **Confirm tiles genuinely render once centered on a real location.** After this fix,
|
||||
drop a pin near a real city on the Android emulator (use `adb emu geo fix` to set a
|
||||
real location, as FB-01's own verification pass did). Take a screenshot. Confirm
|
||||
street-level map detail actually appears, not just a differently-colored blank area.
|
||||
If tiles still do not render even at a real location, that is a second, distinct bug —
|
||||
investigate `TileLayer`'s setup in this file (`urlTemplate`, `tileProvider`,
|
||||
`mapConnectivityProvider`'s `skeletonMode`) before assuming this ticket's fix is
|
||||
sufficient, and document the real cause in the Outcome section.
|
||||
|
||||
## Implementation
|
||||
1. Add `final ambientFix = ref.watch(ambientPositionProvider).valueOrNull;` and the
|
||||
`ambientPosition` conversion to `build()`.
|
||||
2. Change the `initialCenter` fallback for the empty-waypoints case from `const
|
||||
ll.LatLng(0, 0)` to `ambientPosition ?? const ll.LatLng(0, 0)`.
|
||||
3. Run the app on the Android emulator. Set a real mock location. Open a new route.
|
||||
Confirm the map opens on that location with visible street detail, not open ocean.
|
||||
|
||||
## Acceptance criteria
|
||||
- [ ] A brand-new route (0 pins) opens the map centered on the rider's real location,
|
||||
when a location fix is available.
|
||||
- [ ] A brand-new route still opens on `(0, 0)` when no location fix is available
|
||||
(permission denied, service disabled, or no fix yet) — no crash, no infinite
|
||||
spinner.
|
||||
- [ ] A route with 1 or more pins still centers on the pin, unchanged from today.
|
||||
- [ ] Dropping a pin near a real city on the emulator shows visible street-level map
|
||||
detail underneath it, confirmed by a real screenshot.
|
||||
- [ ] `flutter analyze` clean, `flutter test` green, test count only goes up.
|
||||
|
||||
## Tests
|
||||
- Widget test: pump `RoutePlannerScreen` for a route with 0 waypoints, with
|
||||
`ambientPositionProvider` overridden to a fixed `LocationFix` — assert the
|
||||
`FlutterMap`'s `options.initialCenter` matches that fix, not `(0, 0)`.
|
||||
- Widget test: pump `RoutePlannerScreen` for a route with 0 waypoints, with
|
||||
`ambientPositionProvider` overridden to emit `null` (no fix available) — assert
|
||||
`initialCenter` falls back to `(0, 0)`, matching today's existing behavior.
|
||||
- Widget test: pump `RoutePlannerScreen` for a route with 1 real waypoint — assert
|
||||
`initialCenter` still matches that waypoint's own coordinates, not the ambient
|
||||
position, even when `ambientPositionProvider` emits a different fix.
|
||||
|
||||
## Risks
|
||||
- If the on-device check in this ticket's Implementation step 3 finds tiles still do
|
||||
not render at a real location, this ticket's fix alone is not sufficient — document
|
||||
the real root cause found and either fix it in this same ticket or state plainly in
|
||||
the Outcome section that a further ticket is needed. Do not claim this ticket is done
|
||||
without confirming real map detail actually appears on a screenshot.
|
||||
|
||||
## Out of scope
|
||||
Any change to the Map tab's own map (FB-06 covers its remaining issues). Turn-by-turn
|
||||
route following (V3-09, already deferred elsewhere).
|
||||
138
docs/feedback/FB-08-hud-reflow-on-hide.md
Normal file
138
docs/feedback/FB-08-hud-reflow-on-hide.md
Normal file
@@ -0,0 +1,138 @@
|
||||
# FB-08 — HUD widgets reflow to fill the gap when one is hidden
|
||||
|
||||
**Depends on** — · **Size** M · **Status** Not started
|
||||
|
||||
## Goal
|
||||
Hiding a HUD metric must resize the remaining widgets in its row to fill the empty
|
||||
space. Today hiding a metric leaves an empty gap in the grid.
|
||||
|
||||
## Context
|
||||
Direct user feedback (`docs/FEEDBACK.md`):
|
||||
|
||||
> When disabling some statistic during the HUD sessions, they should resize
|
||||
> themselves, instead of leaving a gap by default.
|
||||
|
||||
`lib/src/hud/hud_layout_controller.dart`, `setVisible` (full method):
|
||||
|
||||
```dart
|
||||
void setVisible(HudMetric metric, bool visible) {
|
||||
var current = state[metric] ?? HudWidgetLayout.defaultFor(metric);
|
||||
if (visible && _overlapsAnyOther(metric, current.copyWith(visible: true))) {
|
||||
current = HudWidgetLayout.nextFreeSlot(
|
||||
metric,
|
||||
occupied: state,
|
||||
colSpan: current.colSpan,
|
||||
rowSpan: current.rowSpan,
|
||||
);
|
||||
}
|
||||
state = {...state, metric: current.copyWith(visible: visible)};
|
||||
}
|
||||
```
|
||||
|
||||
This method only ever changes the toggled metric's own entry. No other metric's `col`,
|
||||
`row`, or `colSpan` ever changes as a side effect. When a metric in a row of four is
|
||||
hidden, the other three keep their exact original `col`/`colSpan` — the grid cell the
|
||||
hidden metric used to occupy simply renders nothing, showing as blank space in the HUD.
|
||||
|
||||
`HudWidgetLayout` (`lib/src/hud/hud_widget_layout.dart`) already has everything this
|
||||
ticket needs to build a row-reflow function: `col`, `row`, `colSpan`, `rowSpan` (all
|
||||
`int`), `copyWith`, and `hudRectsOverlap`. `hudGridColumns` is `4`.
|
||||
|
||||
## Design
|
||||
- Add a private helper to `HudLayoutController`:
|
||||
```dart
|
||||
/// Spreads every visible widget in [row] evenly across the full grid width,
|
||||
/// left to right in their existing column order. A widget whose row has no other
|
||||
/// visible member is left alone -- there is nothing to redistribute.
|
||||
Map<HudMetric, HudWidgetLayout> _reflowRow(
|
||||
Map<HudMetric, HudWidgetLayout> layouts,
|
||||
int row,
|
||||
) {
|
||||
final members = layouts.values.where((l) => l.visible && l.row == row).toList()
|
||||
..sort((a, b) => a.col.compareTo(b.col));
|
||||
if (members.length < 2) return layouts;
|
||||
final baseSpan = hudGridColumns ~/ members.length;
|
||||
final extra = hudGridColumns % members.length;
|
||||
final result = Map<HudMetric, HudWidgetLayout>.from(layouts);
|
||||
var col = 0;
|
||||
for (var i = 0; i < members.length; i++) {
|
||||
final span = baseSpan + (i < extra ? 1 : 0);
|
||||
result[members[i].metric] = members[i].copyWith(col: col, colSpan: span);
|
||||
col += span;
|
||||
}
|
||||
return result;
|
||||
}
|
||||
```
|
||||
A row of 4 becomes 4 equal columns (unchanged from today). A row of 3 becomes
|
||||
columns of width `[2, 1, 1]` (`4 ~/ 3 = 1`, remainder `1` goes to the first member).
|
||||
A row of 2 becomes `[2, 2]`. A row of 1 becomes `[4]` — a single remaining widget
|
||||
fills the whole row.
|
||||
- Call `_reflowRow` from `setVisible`, after computing the toggled metric's own new
|
||||
entry, using that metric's **row before the toggle** when hiding, and its **row
|
||||
after placement** when showing (the row a newly-shown metric lands in via
|
||||
`defaultFor`/`nextFreeSlot`):
|
||||
```dart
|
||||
void setVisible(HudMetric metric, bool visible) {
|
||||
var current = state[metric] ?? HudWidgetLayout.defaultFor(metric);
|
||||
if (visible && _overlapsAnyOther(metric, current.copyWith(visible: true))) {
|
||||
current = HudWidgetLayout.nextFreeSlot(
|
||||
metric,
|
||||
occupied: state,
|
||||
colSpan: current.colSpan,
|
||||
rowSpan: current.rowSpan,
|
||||
);
|
||||
}
|
||||
final updated = {...state, metric: current.copyWith(visible: visible)};
|
||||
state = _reflowRow(updated, current.row);
|
||||
}
|
||||
```
|
||||
- Reflow only touches the toggled metric's own row. A different row's widgets never
|
||||
move when a metric in another row is hidden or shown.
|
||||
- Reflow applies to every visible widget currently in that row, whether it is at its
|
||||
default position or one the rider dragged there manually. This ticket does not track
|
||||
"was this widget moved by the rider" — the row is always kept evenly filled, by
|
||||
design, since the feedback asks for the row to "resize themselves" whenever a metric
|
||||
in it is hidden, not only in the untouched-default case.
|
||||
- A drag or a resize (as opposed to a visibility toggle) does **not** trigger reflow —
|
||||
`updatePosition`/`updateSize` are unchanged. Reflow is scoped to `setVisible` only.
|
||||
|
||||
## Implementation
|
||||
1. Add `_reflowRow` to `HudLayoutController`.
|
||||
2. Call `_reflowRow` at the end of `setVisible`, passing the toggled metric's row.
|
||||
3. Run `flutter analyze` and `flutter test`.
|
||||
|
||||
## Acceptance criteria
|
||||
- [ ] Hiding one metric out of four in the same row resizes the remaining three to
|
||||
fill the row evenly — no empty gap.
|
||||
- [ ] Hiding a metric that is alone in its row leaves every other row untouched.
|
||||
- [ ] Showing a hidden metric back reflows its landing row to make room for it,
|
||||
shrinking the row's other members evenly.
|
||||
- [ ] A metric in a different row from the one just toggled never moves.
|
||||
- [ ] `flutter analyze` clean, `flutter test` green, test count only goes up.
|
||||
|
||||
## Tests
|
||||
In `test/hud_layout_controller_test.dart` (alongside the existing `setVisible` tests):
|
||||
- Hiding one of four same-row, equally-sized metrics leaves the remaining three each
|
||||
spanning `hudGridColumns ~/ 3` or one more (matching the `baseSpan`/`extra` split),
|
||||
covering the full row width with no gap (assert the sum of `colSpan` across the
|
||||
remaining visible row members equals `hudGridColumns`).
|
||||
- Hiding a metric whose row has no other visible member leaves every other metric's
|
||||
`col`/`row`/`colSpan` exactly unchanged (assert full equality against the pre-toggle
|
||||
state for every other metric).
|
||||
- Showing a previously-hidden metric back into a row with existing members reflows
|
||||
that row to fit it — assert the sum of `colSpan` across the row (including the
|
||||
newly-shown metric) still equals `hudGridColumns`.
|
||||
- Toggling a metric in one row leaves a different row's own members' `col`/`colSpan`
|
||||
unchanged.
|
||||
|
||||
## Risks
|
||||
None significant — this only changes `col`/`colSpan` values within a single row at the
|
||||
moment of a visibility toggle; it does not touch persistence format, the drag/resize
|
||||
collision logic, or any other file.
|
||||
|
||||
## Out of scope
|
||||
FB-09's title-driven default sizing — this ticket does not change what colSpan a
|
||||
metric starts at, only how a row's members redistribute space among themselves when
|
||||
one of them is hidden or shown. Sequence FB-09 after this ticket merges, since both
|
||||
touch `HudLayoutController`/`HudWidgetLayout` and a large simultaneous diff in both is
|
||||
harder to review and merge than one after the other.
|
||||
260
docs/feedback/FB-09-hud-title-driven-sizing.md
Normal file
260
docs/feedback/FB-09-hud-title-driven-sizing.md
Normal file
@@ -0,0 +1,260 @@
|
||||
# FB-09 — HUD default sizing follows the title, not a fixed uniform cell
|
||||
|
||||
**Depends on** FB-08 (same files, sequence after it merges) · **Size** L · **Status** Not started
|
||||
|
||||
## Goal
|
||||
A HUD widget's title must always fit at a fixed, readable size. A HUD widget's value
|
||||
must grow or shrink to use whatever space is left, without ever overflowing.
|
||||
|
||||
## Context
|
||||
Direct user feedback (`docs/FEEDBACK.md`):
|
||||
|
||||
> Also the default sizing is fucked. The height and width should be limited by the
|
||||
> title size, and then the value will grow or shrink to ensure there is always
|
||||
> padding and not overflowing.
|
||||
|
||||
### Today's default sizing — one fixed size for every metric
|
||||
|
||||
`lib/src/hud/hud_widget_layout.dart`, `defaultFor` (full factory):
|
||||
|
||||
```dart
|
||||
factory HudWidgetLayout.defaultFor(HudMetric metric) {
|
||||
const columns = 4;
|
||||
const rowSpan = 2;
|
||||
final index = HudMetric.values.indexOf(metric);
|
||||
final row = (index ~/ columns) * rowSpan;
|
||||
final col = index % columns;
|
||||
return HudWidgetLayout(
|
||||
metric: metric,
|
||||
col: col,
|
||||
row: row,
|
||||
colSpan: 1,
|
||||
rowSpan: rowSpan,
|
||||
visible: index < columns,
|
||||
);
|
||||
}
|
||||
```
|
||||
|
||||
Every metric gets `colSpan: 1`, no matter how long its label is. `HudMetric.label`
|
||||
(`lib/src/hud/hud_metric.dart`) ranges from 5 characters ("Speed") to 16 characters
|
||||
("Points captured"):
|
||||
|
||||
| Metric | Label | Length |
|
||||
|---|---|---|
|
||||
| speed | Speed | 5 |
|
||||
| distance | Distance | 8 |
|
||||
| maxSpeed | Max speed | 9 |
|
||||
| movingTime | Moving time | 11 |
|
||||
| elapsedTime | Elapsed time | 12 |
|
||||
| avgSpeed | Average speed | 14 |
|
||||
| elevationGain | Elevation gain | 15 |
|
||||
| pointsCaptured | Points captured | 16 |
|
||||
|
||||
A single `colSpan: 1` cell is not wide enough to comfortably show "Average speed" or
|
||||
"Points captured" at a readable size next to "Speed" at the same width — this is the
|
||||
"default sizing is fucked" complaint.
|
||||
|
||||
### Today's text sizing — one `FittedBox` shrinks both lines together
|
||||
|
||||
`lib/src/ui/record/record_screen.dart`, `_HudMetricValue.build()` (~lines 328-360):
|
||||
|
||||
```dart
|
||||
return FittedBox(
|
||||
fit: BoxFit.scaleDown,
|
||||
child: Column(
|
||||
mainAxisSize: MainAxisSize.min,
|
||||
children: [
|
||||
Text(
|
||||
metric.label.toUpperCase(),
|
||||
style: TextStyle(fontSize: 10 * scale, letterSpacing: 1, color: colors.onSurfaceVariant),
|
||||
),
|
||||
const SizedBox(height: 4),
|
||||
Text(
|
||||
_value,
|
||||
style: monoDigits.copyWith(fontSize: 18 * scale, fontWeight: FontWeight.bold, color: _valueColor(colors)),
|
||||
),
|
||||
],
|
||||
),
|
||||
);
|
||||
```
|
||||
|
||||
One `FittedBox` wraps both the title and the value together. `BoxFit.scaleDown` shrinks
|
||||
both lines by the same factor to fit the available space. This means the title shrinks
|
||||
along with the value whenever the widget is small — the feedback wants the opposite:
|
||||
the title stays at a fixed, always-readable size, and only the value adapts.
|
||||
|
||||
### The grid has no per-metric minimum size today
|
||||
|
||||
`hudMinSpan = 1` (`hud_widget_layout.dart`) is the same floor for every metric,
|
||||
regardless of label length. Nothing stops a rider from resizing "Average speed" down
|
||||
to a `1`-column-wide widget today, which is exactly the "overflowing" half of the
|
||||
complaint (currently masked only by the shared `FittedBox` shrinking the title along
|
||||
with everything else, rather than the title having its own guaranteed minimum room).
|
||||
|
||||
## Design
|
||||
`defaultFor` is a static factory with no access to real screen pixel dimensions (no
|
||||
`BuildContext`, no `areaSize`) — an exact per-pixel text measurement is not available
|
||||
at the point this factory runs. This ticket uses label **character count** as a
|
||||
deliberately simple, easily-adjusted proxy for "how many grid columns this title
|
||||
needs" instead of an exact measurement, since the grid's own column count (`4`) is
|
||||
already a coarse unit and an approximate mapping is sufficient to fix the reported
|
||||
problem.
|
||||
|
||||
- Add a helper to `hud_widget_layout.dart`:
|
||||
```dart
|
||||
/// A label under 10 characters fits one column. A label under 15 characters needs
|
||||
/// two. Anything longer needs three. This is a character-count proxy for "how much
|
||||
/// horizontal room this title needs," not an exact pixel measurement -- `defaultFor`
|
||||
/// runs with no `BuildContext` and cannot measure real text width. Adjust these
|
||||
/// thresholds directly if a future label reads too cramped or too loose in practice.
|
||||
int minColSpanForLabel(String label) {
|
||||
if (label.length < 10) return 1;
|
||||
if (label.length < 15) return 2;
|
||||
return 3;
|
||||
}
|
||||
```
|
||||
Applying this to the table above: Speed (1), Distance (1), Max speed (1), Moving
|
||||
time (2), Elapsed time (2), Average speed (2), Elevation gain (3), Points captured
|
||||
(3).
|
||||
- **Rework `defaultFor` into a left-to-right row-packing layout.** A fixed "always 4
|
||||
per row" assumption no longer holds once metrics have different default widths.
|
||||
Replace the index-based `row`/`col` math with a running cursor that wraps to a new
|
||||
row when the current one would overflow:
|
||||
```dart
|
||||
factory HudWidgetLayout.defaultFor(HudMetric metric) {
|
||||
const rowSpan = 2;
|
||||
var col = 0;
|
||||
var row = 0;
|
||||
for (final m in HudMetric.values) {
|
||||
final span = minColSpanForLabel(m.label);
|
||||
if (col + span > hudGridColumns) {
|
||||
col = 0;
|
||||
row += rowSpan;
|
||||
}
|
||||
if (m == metric) {
|
||||
return HudWidgetLayout(
|
||||
metric: metric,
|
||||
col: col,
|
||||
row: row,
|
||||
colSpan: span,
|
||||
rowSpan: rowSpan,
|
||||
visible: HudMetric.values.indexOf(metric) < 4,
|
||||
);
|
||||
}
|
||||
col += span;
|
||||
}
|
||||
// Unreachable: metric is always one of HudMetric.values.
|
||||
throw StateError('Unknown metric: $metric');
|
||||
}
|
||||
```
|
||||
The `visible: index < 4` rule is unchanged — the first four enum values (Speed, Avg
|
||||
Speed, Distance, Elapsed Time) still start visible, matching the Map HUD mockup's own
|
||||
fixed row. Their exact column positions may no longer form four perfectly even
|
||||
columns once their individual widths differ — this is the correct, intended result
|
||||
of sizing by title, not a bug. Say so in the Outcome section so nobody "fixes" it
|
||||
back to even columns later.
|
||||
- **Enforce the same minimum as a resize floor.** In
|
||||
`lib/src/ui/components/draggable_resizable_hud_widget.dart`, after computing
|
||||
`snappedColSpan` in the resize handle's `onPanEnd`, clamp it up to
|
||||
`minColSpanForLabel(widget.layout.metric.label)` before calling `widget.onResized`:
|
||||
```dart
|
||||
final snappedColSpan = (((layout.colSpan * _cellWidth + _resizeDelta.width) / _cellWidth)
|
||||
.round())
|
||||
.clamp(minColSpanForLabel(layout.metric.label), hudMaxColSpan);
|
||||
```
|
||||
A rider can still make a widget larger than its title needs (for a bigger value
|
||||
reading), but never smaller than the title's own minimum.
|
||||
- **Split the `FittedBox`: title fixed, value flexible.** In `_HudMetricValue.build()`,
|
||||
keep the title as a plain, unwrapped `Text` at its fixed reference size (with
|
||||
`maxLines: 1`/`TextOverflow.ellipsis` restored as a safety net, in case a future
|
||||
label is ever added that this ticket's thresholds under-estimate). Wrap only the
|
||||
value in its own `FittedBox`:
|
||||
```dart
|
||||
return Column(
|
||||
mainAxisSize: MainAxisSize.min,
|
||||
children: [
|
||||
Text(
|
||||
metric.label.toUpperCase(),
|
||||
style: TextStyle(fontSize: 10 * scale, letterSpacing: 1, color: colors.onSurfaceVariant),
|
||||
maxLines: 1,
|
||||
overflow: TextOverflow.ellipsis,
|
||||
),
|
||||
const SizedBox(height: 4),
|
||||
Flexible(
|
||||
child: FittedBox(
|
||||
fit: BoxFit.scaleDown,
|
||||
child: Text(
|
||||
_value,
|
||||
style: monoDigits.copyWith(fontSize: 18 * scale, fontWeight: FontWeight.bold, color: _valueColor(colors)),
|
||||
),
|
||||
),
|
||||
),
|
||||
],
|
||||
);
|
||||
```
|
||||
`Flexible` around the value's `FittedBox` lets it claim whatever vertical space is
|
||||
left after the fixed-size title, rather than both competing for space inside one
|
||||
shared `Column`/`FittedBox` as today.
|
||||
|
||||
## Implementation
|
||||
1. Add `minColSpanForLabel` to `hud_widget_layout.dart`.
|
||||
2. Rework `HudWidgetLayout.defaultFor` into the row-packing algorithm above.
|
||||
3. Add the `minColSpanForLabel` floor to the resize clamp in
|
||||
`draggable_resizable_hud_widget.dart`.
|
||||
4. Change `_HudMetricValue.build()` to a fixed-size title `Text` plus a
|
||||
`Flexible(child: FittedBox(...))`-wrapped value `Text`.
|
||||
5. Run `flutter analyze` and `flutter test`.
|
||||
|
||||
## Acceptance criteria
|
||||
- [ ] Every metric's default widget is wide enough to show its full title without
|
||||
truncation at the fixed title font size.
|
||||
- [ ] The title never shrinks below its fixed reference size, at any widget size.
|
||||
- [ ] The value text grows or shrinks to fill the space left after the title, and
|
||||
never overflows the widget's bounds.
|
||||
- [ ] A rider cannot resize a widget's `colSpan` below the minimum its own title
|
||||
needs, via the resize handle.
|
||||
- [ ] `flutter analyze` clean, `flutter test` green, test count only goes up.
|
||||
|
||||
## Tests
|
||||
In `test/hud_widget_layout_test.dart`:
|
||||
- `minColSpanForLabel` returns `1` for a label under 10 characters, `2` for one under
|
||||
15, `3` for 15 or more — test with `HudMetric.speed.label`,
|
||||
`HudMetric.elapsedTime.label`, and `HudMetric.pointsCaptured.label` directly, not
|
||||
synthetic strings, so the test breaks if a real label's length crosses a threshold.
|
||||
- `defaultFor` gives every metric a `colSpan` at least equal to
|
||||
`minColSpanForLabel(metric.label)`.
|
||||
- `defaultFor` never produces a row whose members' `colSpan` sum exceeds
|
||||
`hudGridColumns` (the packing algorithm's own invariant).
|
||||
- `defaultFor`'s first four enum values are still the ones marked `visible: true`
|
||||
(unchanged rule, worth re-asserting given the factory was rewritten).
|
||||
|
||||
Widget tests (extend `test/hud_edit_overlay_test.dart` or add to `test/widget_test.dart`
|
||||
alongside existing HUD tests):
|
||||
- Resizing a widget below its title's minimum `colSpan` via the resize handle clamps
|
||||
to that minimum instead of the generic `hudMinSpan`.
|
||||
- Pump `_HudMetricValue` for `HudMetric.pointsCaptured` (the longest label) inside a
|
||||
widget sized to exactly `minColSpanForLabel`'s column count — assert
|
||||
`tester.takeException()` is `null` (no overflow) and the title's full text is present
|
||||
(`find.text('POINTS CAPTURED')`, not an ellipsized fragment).
|
||||
- Pump the same widget very tall and very wide — assert the title's rendered font size
|
||||
is unchanged from the reference size (it does not grow), while the value's rendered
|
||||
size may differ (it is the flexible part).
|
||||
|
||||
## Risks
|
||||
- The default HUD row of four no longer looks like four perfectly even columns once
|
||||
title-driven widths differ (e.g. "Average speed" is now wider than "Speed"). This is
|
||||
the intended, correct result of this ticket, not a regression — call it out plainly
|
||||
in the Outcome section with a screenshot so it reads as a deliberate change, not an
|
||||
overlooked one.
|
||||
- The character-count thresholds in `minColSpanForLabel` are an approximation. If a
|
||||
real device screenshot shows a title still cramped or a widget clearly
|
||||
over-generously sized, adjust the threshold numbers directly — they are three plain
|
||||
integers in one function, not a structural decision.
|
||||
|
||||
## Out of scope
|
||||
FB-08's row-reflow-on-hide behavior — implement this ticket assuming FB-08 has already
|
||||
merged, and re-run FB-08's own tests to confirm the two features compose correctly
|
||||
(a reflowed row must still respect each remaining member's own title-driven minimum
|
||||
`colSpan` — if `_reflowRow`'s even split would push a widget below its own minimum,
|
||||
that is a real interaction between the two tickets worth checking explicitly and
|
||||
documenting in this ticket's Outcome section).
|
||||
@@ -5,8 +5,9 @@ Implementation · Acceptance criteria · Tests · Risks · Out of scope, written
|
||||
implementing, with an Outcome section appended after.
|
||||
|
||||
Source: `docs/FEEDBACK.md` — hands-on feedback after using the redesigned app. Turned
|
||||
into 5 tickets, each independently completable (self-contained enough for a fresh
|
||||
subagent with no prior context to implement correctly).
|
||||
into 9 tickets so far (5 from the first pass, 4 from a second round of feedback after
|
||||
FB-01..FB-05 shipped), each independently completable (self-contained enough for a
|
||||
fresh subagent with no prior context to implement correctly).
|
||||
|
||||
## The tickets
|
||||
|
||||
@@ -17,6 +18,10 @@ subagent with no prior context to implement correctly).
|
||||
| [FB-03](FB-03-grid-snapped-hud-widgets.md) | Grid-snapped HUD widgets with auto-fit text | L | — | Done |
|
||||
| [FB-04](FB-04-route-planner-map-render-fix.md) | Fix Route Planner's map failing to render + zoom | M | FB-01 (shared zoom constant) | Done |
|
||||
| [FB-05](FB-05-closed-loop-routes.md) | Closed-loop routes in Route Planner | M | FB-04 (same file) | Done |
|
||||
| [FB-06](FB-06-always-interactive-map-recenter.md) | Map always pannable/zoomable, with a recenter button | M | — | Not started |
|
||||
| [FB-07](FB-07-route-planner-null-island.md) | Route Planner opens on Null Island instead of your real location | S/M | FB-01 (ambient location provider) | Not started |
|
||||
| [FB-08](FB-08-hud-reflow-on-hide.md) | HUD widgets reflow to fill the gap when one is hidden | M | — | Not started |
|
||||
| [FB-09](FB-09-hud-title-driven-sizing.md) | HUD default sizing follows the title, value adapts | L | FB-08 (same files) | Not started |
|
||||
|
||||
## Dependencies / dispatch order
|
||||
|
||||
@@ -24,6 +29,9 @@ subagent with no prior context to implement correctly).
|
||||
Wave 1 (parallel): FB-01, FB-02, FB-03
|
||||
Wave 2 (after Wave 1 lands): FB-04 — reuses FB-01's ambientZoom constant
|
||||
Wave 3 (after Wave 2 lands): FB-05 — heavily edits the same file FB-04 just touched
|
||||
|
||||
Wave 4 (parallel): FB-06, FB-07, FB-08 — no file overlap between these three
|
||||
Wave 5 (after Wave 4 lands): FB-09 — heavily edits the same files FB-08 just touched
|
||||
```
|
||||
|
||||
FB-02 and FB-03 both touch `lib/src/ui/record/record_screen.dart`, but disjoint
|
||||
|
||||
Reference in New Issue
Block a user