From 9e2d2f7c81a61be76e66c4c71fc08249d8e04482 Mon Sep 17 00:00:00 2001 From: Danny McClelland Date: Fri, 24 Jul 2026 08:10:24 +0100 Subject: [PATCH 1/2] fix(timeline): align scrub crosshair to the plotted line + reserve readout slot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The merged-vitals timeline chart draws each line from its 15-minute bucket averages (_Vital.avg), but the scrub crosshair read the raw nearest sample (_Vital.valueAt over pts). At a touched x the dot therefore sat off the line and jittered at a different granularity from the smooth static render. Resolve the crosshair against the SAME bucketed series and x-scale the line is stroked from: plottedLineValueAt() linearly interpolates avg between adjacent bucket centres — the exact y the painter's lineTo segments pass through — so the marker lands on the line at matching granularity. The value readout now reports the same on-line value. x→time mapping is extracted as scrubTimeAt(); both are pure and unit-tested. Also reserve the crosshair readout's vertical space at all times (Visibility with maintainSize) so it appearing on scrub no longer grows the tile and reflows the peak/low + events below it. Idle it lays out the same rows as dashes and is hidden. Static rendering is unchanged. Closes #141 Reported in #102. --- lib/ui/timeline/timeline_screen.dart | 93 ++++++++++++++------ test/timeline_scrub_test.dart | 124 +++++++++++++++++++++++++++ 2 files changed, 193 insertions(+), 24 deletions(-) create mode 100644 test/timeline_scrub_test.dart diff --git a/lib/ui/timeline/timeline_screen.dart b/lib/ui/timeline/timeline_screen.dart index 50b225f..792a997 100644 --- a/lib/ui/timeline/timeline_screen.dart +++ b/lib/ui/timeline/timeline_screen.dart @@ -32,11 +32,55 @@ const Color _kHrv = Color(0xFF2E7D32); // deep green — recovery const Color _kResp = Color(0xFF1565C0); // deep blue — breath const Color _kTemp = Color(0xFFE65100); // deep orange — heat +/// Local drag x (pixels) → timestamp on the chart's time axis. Uses the SAME +/// left-pad and `[t0, t1]` domain the painter maps with (its `x(t)` closure), +/// so a touched x resolves to exactly the time the crosshair is drawn at. Pure +/// + unit-tested. +double scrubTimeAt( + double dx, + double width, { + required double leftPad, + required double t0, + required double t1, +}) { + final usable = width - leftPad; + if (usable <= 0) return t0; + final x = (dx - leftPad).clamp(0.0, usable); + return t0 + (x / usable) * (t1 - t0); +} + +/// Value of the plotted line at time [t] — a linear interpolation between the +/// two adjacent bucket centres, i.e. the exact y the painter's `lineTo` +/// segments pass through (see [_ChartPainter._drawLine], which strokes this +/// same [avg] series). The scrub crosshair reads THIS rather than the raw +/// nearest sample, so the marker lands on the drawn line at the touched x and +/// at the line's own (bucketed) granularity — not on an unaligned raw point. +/// Endpoints are clamped; null only when there are no buckets. Pure + +/// unit-tested. +double? plottedLineValueAt( + List<({double t, double v, double lo, double hi})> avg, + double t, +) { + if (avg.isEmpty) return null; + if (t <= avg.first.t) return avg.first.v; + if (t >= avg.last.t) return avg.last.v; + for (var i = 1; i < avg.length; i++) { + final b = avg[i]; + if (t <= b.t) { + final a = avg[i - 1]; + final span = b.t - a.t; + if (span <= 0) return b.v; + return a.v + (b.v - a.v) * ((t - a.t) / span); + } + } + return avg.last.v; +} + class _Vital { final String label; final String unit; final Color color; - final List<({double t, double v})> pts; // raw (peaks + crosshair) + final List<({double t, double v})> pts; // raw (peaks + envelope extremes) // Per-bucket mean (v) + min/max (lo/hi) → the line + its range envelope. final List<({double t, double v, double lo, double hi})> avg; final int decimals; @@ -48,18 +92,10 @@ class _Vital { this.decimals, this.avg, ); - double? valueAt(double t) { - double? best; - var bestDt = double.infinity; - for (final p in pts) { - final dt = (p.t - t).abs(); - if (dt < bestDt) { - bestDt = dt; - best = p.v; - } - } - return best; - } + + /// The value ON the drawn line at [t] (interpolated over [avg]) — what the + /// scrub crosshair and readout report, so both track the plotted curve. + double? lineValueAt(double t) => plottedLineValueAt(avg, t); } class _Band { @@ -461,17 +497,27 @@ class _TimelineContentState extends State ); }, ), - if (_scrubT != null) ...[ - const SizedBox(height: Sp.x3), - _crosshairReadout(), - ], + // The per-moment vital readout only carries values while scrubbing, + // but its slot is reserved at all times so appearing on scrub does + // NOT grow the tile and shove the peak/low + events below it. When + // idle it lays out the same rows (dashes) and is simply hidden. + const SizedBox(height: Sp.x3), + Visibility( + visible: _scrubT != null, + maintainSize: true, + maintainAnimation: true, + maintainState: true, + child: _crosshairReadout(), + ), ], ), ); } Widget _crosshairReadout() { - final t = _scrubT!; + // Nullable: when idle this still lays out (dashes) to hold the reserved + // slot's height; [Visibility] hides it until a scrub sets [_scrubT]. + final t = _scrubT; return Container( padding: const EdgeInsets.all(Sp.x3), decoration: BoxDecoration( @@ -481,12 +527,12 @@ class _TimelineContentState extends State child: Column( crossAxisAlignment: CrossAxisAlignment.start, children: [ - Text(_clock(t), style: AppText.label), + Text(t == null ? '--:--' : _clock(t), style: AppText.label), const SizedBox(height: Sp.x2), for (final v in _vitals) Builder( builder: (_) { - final val = v.valueAt(t); + final val = t == null ? null : v.lineValueAt(t); return Padding( padding: const EdgeInsets.symmetric(vertical: 2), child: Row( @@ -520,9 +566,8 @@ class _TimelineContentState extends State } void _scrub(double dx, double width) { - final x = (dx - _leftPad).clamp(0.0, width - _leftPad); - final frac = (width - _leftPad) <= 0 ? 0.0 : x / (width - _leftPad); - setState(() => _scrubT = _t0 + frac * (_t1 - _t0)); + final t = scrubTimeAt(dx, width, leftPad: _leftPad, t0: _t0, t1: _t1); + setState(() => _scrubT = t); } // ── peak / low of the ACTIVE vital, numbers-first ─────────────────────── @@ -700,7 +745,7 @@ class _ChartPainter extends CustomPainter { ..strokeWidth = 1); for (final v in vitals) { final (lo, hi) = _range(v); - final val = v.valueAt(scrubT!); + final val = v.lineValueAt(scrubT!); if (val == null) continue; canvas.drawCircle( Offset(cx, yNorm(val, lo, hi)), 3.5, Paint()..color = v.color); diff --git a/test/timeline_scrub_test.dart b/test/timeline_scrub_test.dart new file mode 100644 index 0000000..c289309 --- /dev/null +++ b/test/timeline_scrub_test.dart @@ -0,0 +1,124 @@ +// Tests for the timeline chart's scrub mapping — the pure x→time and +// time→value-on-the-drawn-line functions the crosshair reads (issue #141). +// +// The bug: the plotted lines are the 15-min bucket averages ([_Vital.avg]) but +// the scrub crosshair used to read the RAW nearest sample, so the marker sat +// off the line at a different granularity. These cover that the value the scrub +// reports is the one that lies ON the drawn (bucketed) polyline at the touched +// x, so the marker aligns with the line. + +import 'package:flutter_test/flutter_test.dart'; + +import 'package:openstrap_edge/ui/timeline/timeline_screen.dart'; + +typedef Bucket = ({double t, double v, double lo, double hi}); + +Bucket _b(double t, double v) => (t: t, v: v, lo: v, hi: v); + +void main() { + group('scrubTimeAt (x-pixel → time)', () { + const t0 = 1000.0, t1 = 2000.0, leftPad = 36.0, width = 336.0; + // usable plot width = 336 - 36 = 300px maps linearly onto [1000, 2000]. + + test('left edge of the plot maps to t0', () { + expect(scrubTimeAt(leftPad, width, leftPad: leftPad, t0: t0, t1: t1), t0); + }); + + test('right edge of the plot maps to t1', () { + expect(scrubTimeAt(width, width, leftPad: leftPad, t0: t0, t1: t1), t1); + }); + + test('mid-plot maps to the midpoint time', () { + // 150px into the 300px plot → halfway → 1500. + expect( + scrubTimeAt(leftPad + 150, width, leftPad: leftPad, t0: t0, t1: t1), + 1500.0, + ); + }); + + test('touches inside the left pad clamp to t0 (never negative time)', () { + expect(scrubTimeAt(0, width, leftPad: leftPad, t0: t0, t1: t1), t0); + expect(scrubTimeAt(10, width, leftPad: leftPad, t0: t0, t1: t1), t0); + }); + + test('touches past the right edge clamp to t1', () { + expect( + scrubTimeAt(width + 80, width, leftPad: leftPad, t0: t0, t1: t1), + t1, + ); + }); + + test('degenerate (width <= leftPad) returns t0 without dividing by zero', () { + expect(scrubTimeAt(20, leftPad, leftPad: leftPad, t0: t0, t1: t1), t0); + }); + }); + + group('plottedLineValueAt (time → value ON the drawn line)', () { + // Three bucket centres — the vertices the painter's line passes through. + final avg = [_b(100, 50), _b(200, 70), _b(300, 60)]; + + test('a time exactly on a bucket centre returns that vertex value', () { + expect(plottedLineValueAt(avg, 100), 50); + expect(plottedLineValueAt(avg, 200), 70); + expect(plottedLineValueAt(avg, 300), 60); + }); + + test('a time between centres lies on the straight segment (interpolated)', () { + // Halfway 100→200: (50+70)/2 = 60 — exactly where lineTo draws. + expect(plottedLineValueAt(avg, 150), 60); + // Quarter 200→300: 70 + (60-70)*0.25 = 67.5. + expect(plottedLineValueAt(avg, 225), 67.5); + }); + + test('the interpolated point sits on the line, not on a raw sample', () { + // A raw nearest-sample reader would have snapped 150 to the 100 or 200 + // vertex (50 or 70); the on-line value is the segment midpoint, 60. + final v = plottedLineValueAt(avg, 150)!; + expect(v, isNot(50)); + expect(v, isNot(70)); + expect(v, 60); + }); + + test('times before the first / after the last centre clamp to the ends', () { + expect(plottedLineValueAt(avg, 0), 50); + expect(plottedLineValueAt(avg, 500), 60); + }); + + test('empty series returns null', () { + expect(plottedLineValueAt(const [], 150), isNull); + }); + + test('single bucket returns its value for any time', () { + final one = [_b(100, 42)]; + expect(plottedLineValueAt(one, 0), 42); + expect(plottedLineValueAt(one, 100), 42); + expect(plottedLineValueAt(one, 999), 42); + }); + + test('coincident bucket timestamps do not divide by zero', () { + final dup = [_b(100, 50), _b(100, 80), _b(200, 60)]; + // Lands on the duplicate boundary — returns a finite vertex value. + final v = plottedLineValueAt(dup, 100)!; + expect(v.isFinite, isTrue); + }); + }); + + group('composed scrub → value stays on the line at matching granularity', () { + const t0 = 100.0, t1 = 300.0, leftPad = 36.0, width = 236.0; // 200px plot + final avg = [_b(100, 50), _b(200, 70), _b(300, 60)]; + + test('touch at plot-mid resolves to the on-line midpoint value', () { + final t = scrubTimeAt(leftPad + 100, width, + leftPad: leftPad, t0: t0, t1: t1); // → 200 + expect(t, 200); + expect(plottedLineValueAt(avg, t), 70); // the vertex at t=200 + }); + + test('touch at plot-quarter resolves onto the first segment', () { + final t = scrubTimeAt(leftPad + 50, width, + leftPad: leftPad, t0: t0, t1: t1); // → 150 + expect(t, 150); + expect(plottedLineValueAt(avg, t), 60); // midpoint of 50→70 + }); + }); +} From 9334c90fe28b9075061dbb5b3e5ed31f2c554b75 Mon Sep 17 00:00:00 2001 From: Danny McClelland Date: Fri, 24 Jul 2026 08:17:18 +0100 Subject: [PATCH 2/2] fix(timeline): omit a vital from the scrub crosshair outside its drawn range MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit plottedLineValueAt clamped the scrub value to the nearest bucket centre when the scrub time fell outside a vital's buckets. But the scrub band spans the whole timeline while each vital's line is only drawn from its first to its last bucket centre, so scrubbing before a vital starts (or after it ends) reported a flat/extrapolated value at a time where that vital's line isn't drawn. Return null when t is strictly outside [first centre, last centre]; the boundary centres still return their value and interpolation within the range is unchanged. The crosshair dot already skips a null value (no dot) and the readout already renders it as "—", so an out-of-coverage vital is now omitted exactly where its line is absent, matching the plot. Addresses CodeRabbit review on #144. --- lib/ui/timeline/timeline_screen.dart | 12 ++++--- test/timeline_scrub_test.dart | 50 ++++++++++++++++++++++++---- 2 files changed, 51 insertions(+), 11 deletions(-) diff --git a/lib/ui/timeline/timeline_screen.dart b/lib/ui/timeline/timeline_screen.dart index 792a997..0a0e84b 100644 --- a/lib/ui/timeline/timeline_screen.dart +++ b/lib/ui/timeline/timeline_screen.dart @@ -55,15 +55,19 @@ double scrubTimeAt( /// same [avg] series). The scrub crosshair reads THIS rather than the raw /// nearest sample, so the marker lands on the drawn line at the touched x and /// at the line's own (bucketed) granularity — not on an unaligned raw point. -/// Endpoints are clamped; null only when there are no buckets. Pure + -/// unit-tested. +/// +/// Returns null when [t] falls STRICTLY outside the drawn range — before the +/// first or after the last bucket centre — because the line isn't drawn there +/// (the scrub band spans the whole day, but a vital's line only covers its own +/// buckets). Callers omit the vital there (no dot, "—" readout) rather than +/// extrapolating a value onto empty space. The boundary centres themselves +/// still return their value. Pure + unit-tested. double? plottedLineValueAt( List<({double t, double v, double lo, double hi})> avg, double t, ) { if (avg.isEmpty) return null; - if (t <= avg.first.t) return avg.first.v; - if (t >= avg.last.t) return avg.last.v; + if (t < avg.first.t || t > avg.last.t) return null; for (var i = 1; i < avg.length; i++) { final b = avg[i]; if (t <= b.t) { diff --git a/test/timeline_scrub_test.dart b/test/timeline_scrub_test.dart index c289309..ee3cbd6 100644 --- a/test/timeline_scrub_test.dart +++ b/test/timeline_scrub_test.dart @@ -79,20 +79,29 @@ void main() { expect(v, 60); }); - test('times before the first / after the last centre clamp to the ends', () { - expect(plottedLineValueAt(avg, 0), 50); - expect(plottedLineValueAt(avg, 500), 60); + test('times strictly outside the drawn range return null (not clamped)', () { + // The line is only drawn from the first to the last bucket centre; a + // vital must NOT report an extrapolated value where its line is absent. + expect(plottedLineValueAt(avg, 0), isNull); // before first centre (100) + expect(plottedLineValueAt(avg, 99.9), isNull); + expect(plottedLineValueAt(avg, 300.1), isNull); // after last centre (300) + expect(plottedLineValueAt(avg, 500), isNull); + }); + + test('the boundary centres themselves still return their value', () { + expect(plottedLineValueAt(avg, 100), 50); // first centre — inclusive + expect(plottedLineValueAt(avg, 300), 60); // last centre — inclusive }); test('empty series returns null', () { expect(plottedLineValueAt(const [], 150), isNull); }); - test('single bucket returns its value for any time', () { + test('single bucket: its exact centre only, null elsewhere', () { final one = [_b(100, 42)]; - expect(plottedLineValueAt(one, 0), 42); - expect(plottedLineValueAt(one, 100), 42); - expect(plottedLineValueAt(one, 999), 42); + expect(plottedLineValueAt(one, 100), 42); // the sole centre + expect(plottedLineValueAt(one, 0), isNull); // no line drawn away from it + expect(plottedLineValueAt(one, 999), isNull); }); test('coincident bucket timestamps do not divide by zero', () { @@ -121,4 +130,31 @@ void main() { expect(plottedLineValueAt(avg, t), 60); // midpoint of 50→70 }); }); + + group('a vital with narrower coverage than the scrub span is omitted', () { + // The scrub band spans the whole timeline [0, 400] (e.g. it's driven by a + // sleep band), but this vital only has buckets over [100, 300]. Scrubbing + // outside 100–300 must omit it — so the crosshair draws no dot and the + // readout shows "—" — while a full-coverage vital still reports a value. + final wide = [_b(0, 10), _b(200, 30), _b(400, 20)]; // covers the whole span + final narrow = [_b(100, 50), _b(200, 70), _b(300, 60)]; // covers 100–300 + + test('scrub before the narrow vital starts: narrow null, wide has a value', () { + const t = 50.0; // inside wide's range, before narrow's first centre + expect(plottedLineValueAt(narrow, t), isNull); + expect(plottedLineValueAt(wide, t), isNotNull); + }); + + test('scrub after the narrow vital ends: narrow null, wide has a value', () { + const t = 350.0; // inside wide's range, after narrow's last centre + expect(plottedLineValueAt(narrow, t), isNull); + expect(plottedLineValueAt(wide, t), isNotNull); + }); + + test('scrub within both ranges: both report their on-line value', () { + const t = 200.0; + expect(plottedLineValueAt(narrow, t), 70); + expect(plottedLineValueAt(wide, t), 30); + }); + }); }