a 13-week activity heatmap on the workouts screen - #222
Conversation
The session feed is reverse-chronological, so it can show you what you did but never the shape of a quarter: where the gaps are, and whether the rhythm is holding. This adds a Monday-aligned day grid above the feed, shaded by the calories burned in logged workouts. Notes on the choices that aren't obvious: - The board is deliberately independent of the Today/Week/Month/3M selector. It stays a fixed 13-week reference while the feed below it filters, so on the Today tab you still see the quarter instead of only an empty state. - Shade is scaled to the user's own 90th-percentile day (floored at 250 kcal) rather than fixed thresholds, which would leave a beginner's grid permanently cold and an endurance athlete's permanently maxed. The percentile rather than the max keeps one exceptional session from flattening everything else. - Data comes from getSessions() with an explicit window, not getWorkouts(range: 'quarter'): that tops out at 90 days and a Monday-aligned 13-week grid needs up to 97, which would leave the oldest column silently short. Unconfirmed auto-detections and live sessions are filtered out so the grid agrees with the feed and the training summary. - Days after today render as nothing rather than empty wells, because an empty well reads as a day you skipped. - The ramp's interpolation factors differ per palette. coralSoft sits close to coral on paper and far below it on char, so one shared set of factors produced a lopsided ramp on dark (2.48:1 between levels 0-1 vs 1.29:1 between 3-4). Both are now within a 1.55 spread, asserted by test. Verification: 34 tests covering the calendar layer (DST transitions, local-day bucketing, streak edges), the ramp (monotonic luminance and minimum step in both palettes), and the widget. Run on an iPhone 17 Pro simulator in both themes -- screenshots in the PR. Simulator data is synthetic.
|
Warning Review limit reached
Next review available in: 23 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds a 13-week calorie heatmap with local-day aggregation, intensity levels, streaks, responsive rendering, selection, animation controls, and workout-screen integration. Adds session filtering support, unit and widget tests, and generated-file ignore rules. ChangesWorkout calorie heatmap
Generated preview exclusions
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant WorkoutsScreen
participant LocalRepository
participant loggedForHeatmap
participant buildHeatDays
participant CalorieHeatmapCard
WorkoutsScreen->>LocalRepository: getSessions(105-day window, includeDetected=false)
LocalRepository-->>WorkoutsScreen: saved session records
WorkoutsScreen->>loggedForHeatmap: session records
loggedForHeatmap-->>WorkoutsScreen: logged sessions
WorkoutsScreen->>buildHeatDays: logged sessions and today
buildHeatDays-->>WorkoutsScreen: 13-week HeatDay list
WorkoutsScreen->>CalorieHeatmapCard: days and today
CalorieHeatmapCard-->>WorkoutsScreen: rendered heatmap and selection events
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/ui/workouts/calorie_heatmap.dart`:
- Line 522: Update CalorieHeatmapCard’s grid construction and header to handle
lists whose length is not a multiple of seven: guard each days[col * 7 + row]
access so missing cells are skipped or rendered safely, and derive the displayed
week count from weeks instead of hardcoding “13 weeks.”
- Around line 643-648: Update the AnimatedBuilder around the cell builder so its
animation listenables include pulse only when isPeak is true; non-peak cells
must subscribe solely to reveal while preserving the existing pulse.value glow
behavior for the peak cell.
- Around line 654-657: Update the tappable cell built by GestureDetector and
_cellKey to wrap its subtree in a Semantics node with a descriptive day readout,
button role, and enabled tap action, and ensure the wrapper is closed at the
matching subtree end. Expand the cell’s effective hit target to at least 44
logical pixels while preserving the existing onTap behavior.
In `@lib/ui/workouts/workouts_screen.dart`:
- Around line 174-192: Update _load to accept a refreshHeat flag and skip the
getSessions heatmap fetch when it is false, retaining the existing _heat value.
In the SegmentedControl onChanged handler, call _load(refreshHeat: false); keep
normal loads refreshing heatmap data.
- Around line 308-314: Store the anchor day captured during _load alongside
_heat, then pass that stored value to CalorieHeatmapCard instead of calling
DateTime.now() during build. Ensure the heatmap data, today marker, future-cell
handling, and currentStreak all use the same captured day.
In `@test/calorie_heatmap_test.dart`:
- Around line 504-513: Update the test setup around CalorieHeatmapCard to derive
the reduced-motion MediaQueryData from the ambient MediaQuery rather than
replacing all inherited fields with a bare const value. Preserve the ambient
size, textScaler, padding, platformBrightness, and other fields while overriding
only disableAnimations to true.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1a33f38d-d703-448c-b4d4-fc91471cff44
⛔ Files ignored due to path filters (3)
docs/calorie-heatmap-ios-dark.pngis excluded by!**/*.pngdocs/calorie-heatmap-ios-light.pngis excluded by!**/*.pngdocs/calorie-heatmap-preview.svgis excluded by!**/*.svg
📒 Files selected for processing (4)
.gitignorelib/ui/workouts/calorie_heatmap.dartlib/ui/workouts/workouts_screen.darttest/calorie_heatmap_test.dart
| child: GestureDetector( | ||
| key: ValueKey(_cellKey(d.date)), | ||
| behavior: HitTestBehavior.opaque, | ||
| onTap: () => onTap(d), |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a semantics node for each tappable cell.
The cell exposes no label and no button role, so a screen reader announces nothing and the day readout is unreachable without sight. The tap target is also 9..26 logical pixels, well below the 44/48 minimum.
♿ Proposed fix
- child: GestureDetector(
- key: ValueKey(_cellKey(d.date)),
- behavior: HitTestBehavior.opaque,
- onTap: () => onTap(d),
- child: AnimatedScale(
+ child: Semantics(
+ button: true,
+ selected: isSelected,
+ label: '${_readoutDate(d.date)}, '
+ '${d.kcal > 0 ? '${d.kcal} kcal' : 'no workout'}',
+ child: GestureDetector(
+ key: ValueKey(_cellKey(d.date)),
+ behavior: HitTestBehavior.opaque,
+ onTap: () => onTap(d),
+ child: AnimatedScale(Close the extra Semantics at the matching end of the subtree.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/ui/workouts/calorie_heatmap.dart` around lines 654 - 657, Update the
tappable cell built by GestureDetector and _cellKey to wrap its subtree in a
Semantics node with a descriptive day readout, button role, and enabled tap
action, and ensure the wrapper is closed at the matching subtree end. Expand the
cell’s effective hit target to at least 44 logical pixels while preserving the
existing onTap behavior.
The workouts feed's filter empty-state landed next to where the board inserts, so the two hunks collide. Both are kept: the board sits above the feed, and the feed still explains an empty list caused by the filter rather than by having trained nothing.
The board filtered out anything with source 'auto', but that field records where a workout came from, not whether it is still a proposal — it stays 'auto' after you confirm a suggestion. So a workout sitting in the feed with an auto tag, counted by the training summary, painted as a rest day on the grid: the exact disagreement the filter exists to prevent, pointing the other way. Someone whose training is mostly detected-then-confirmed got a mostly empty board, or none at all, since the card stays hidden until a day has calories on it. Status alone is the test — only an unconfirmed bout is ever 'detected'. The old case set source and status together on one row, so it passed either way and never pinned the thing it was named for. The board's fetch also pulled every recent day's bundle. getSessions builds its detected half by decoding each day's payload — hr_curve, hypnogram, HRV, tens of KB a day — and the grid threw all of it away a line later, on every load and every pull-to-refresh. It asks for saved sessions only now. Cells carry a label too. Colour is the entire content of a heatmap, so without one the board was silent to a screen reader.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/ui/workouts/workouts_screen.dart (1)
105-112: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winGuard the screen context after closing the sheet.
After Line 106 pops the sheet, check
context.mountedbefore Line 107 uses the screen context to push the interval timer. The screen can be disposed during navigation.Proposed fix
onTap: () { Navigator.pop(ctx); + if (!context.mounted) return; Navigator.of(context).push(As per coding guidelines: "After await or navigation, guard context and UI operations with mounted checks; do not use context or Provider lookups after unmount or in dispose."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/ui/workouts/workouts_screen.dart` around lines 105 - 112, After Navigator.pop(ctx) in the onTap callback, check context.mounted before calling Navigator.of(context).push for IntervalTimerScreen. Keep the existing themedRoute configuration and avoid using the screen context when it is no longer mounted.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@lib/ui/workouts/workouts_screen.dart`:
- Around line 105-112: After Navigator.pop(ctx) in the onTap callback, check
context.mounted before calling Navigator.of(context).push for
IntervalTimerScreen. Keep the existing themedRoute configuration and avoid using
the screen context when it is no longer mounted.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 055f8124-baf9-4574-8aa0-5957f5050c52
📒 Files selected for processing (7)
lib/data/local_repository.dartlib/data/local_repository_impl.dartlib/ui/workouts/calorie_heatmap.dartlib/ui/workouts/workouts_screen.darttest/ai_briefing_test.darttest/calorie_heatmap_test.darttest/get_sessions_merge_test.dart
The peak day's pulse repeats for as long as the card is mounted, and every one of the 91 cells had merged it into its AnimatedBuilder — so the entire grid rebuilt every frame, indefinitely, reallocating an Opacity, Transform, AnimatedScale, Container and BoxDecoration each time, to animate the single cell that actually reads the value. Only the peak subscribes now; the reveal stops notifying once it settles, so the rest go quiet after the intro. The grid also took its column count from a rounded-up week count, so a list that wasn't whole weeks indexed past the end, and the header announced 13 weeks whatever it had been handed. Both come off the list it was given now. Changing the range selector no longer refetches the board either. Its window is fixed and independent of that control, so the read could not have changed a cell.
|
Went through the review. Three of the five were real and are fixed in 5ffa07f. The pulse subscription was the one that mattered. Every cell had the repeating The part-week indexing and the hardcoded The semantics finding was already implemented in the commit under review — the Not touching the |
|
@CodeRabbit review |
✅ Action performedReview finished.
|
The grid was built against the clock read inside the load, but the card was handed a fresh one at build time. They agree until the next local midnight, and a screen left open across it got a board describing yesterday under today's clock: the new day still carried its future flag so no cell was drawn for it, the today ring matched nothing, and the streak was counted against a day the board did not contain. The anchor is stored beside the grid now and the two only ever move together. The reduced-motion test built its MediaQueryData from scratch, which resets size to zero and drops text scale, padding and brightness. It reads from the ambient one instead, so it keeps describing a real device.
Skipping the board's read on a range-selector tap traded a correctness property for a query that was already cheap. The window doesn't depend on the selector but the table does, and the tap is one of the few things that re-reads at all: confirming a suggestion from the pushed "did you work out?" screen never reloads the workouts screen, so the next tap refreshed the feed and left the board still calling that day a rest day. The expensive part of the read was the day-bundle decode, and that had already gone. The derived week count also read "1 weeks" for any span under a fortnight, which is the derivation announcing itself in the one place it was added to help.
|
Two more from the earlier review that I'd missed on a first pass, plus one I The board was built against the clock read inside the load but handed a fresh Then the one I introduced. Skipping the board's read on a range tap was wrong, Deriving the week count turned out to print "1 weeks" for any span under a Leaving the ragged day-label gutter alone. It only misaligns below seven days, |
2 participants
What
A 13-week activity heatmap above the workouts feed, shaded by the calories burned in logged workouts.
The session feed is reverse-chronological, so it shows you what you did but never the shape of a quarter — where the gaps are, and whether the rhythm is holding. That's the question this answers.
Before / after
Before, the Workouts screen went straight from the range selector to the feed. Now:
iPhone 17 Pro, iOS 26. The workout data in these shots is synthetic — 54 seeded sessions, since a simulator has no band to pair with.
Tapping a day swaps the header to that day's readout; tapping again releases. An animated preview of the interaction and the four micro-animations is committed at
docs/calorie-heatmap-preview.svg(open it in a browser — it re-themes to your system setting).Why these choices
Independent of the range selector. The board stays a fixed 13-week reference while the feed below it filters. On the Today tab you still see the quarter's rhythm rather than only an empty state.
Shade scales to your own 90th-percentile day (floored at 250 kcal) rather than fixed thresholds, which would leave a beginner's grid permanently cold and an endurance athlete's permanently maxed. Using the percentile rather than the max stops one exceptional session flattening everything else. The
(i)discloses this.getSessions()with an explicit window, notgetWorkouts(range: 'quarter'). That range tops out at 90 days; a Monday-aligned 13-week grid needs up to 97, so the oldest column would have been silently short of data. Unconfirmed auto-detections and live sessions are filtered out, so the grid agrees with the feed and withTrainingSummaryCardinstead of shading a day the feed omits.Future days render as nothing, not as empty wells — an empty well reads as a day you skipped.
The ramp's interpolation factors differ per palette.
coralSoftsits close tocoralon paper and far below it on char, so one shared set of factors produced a visibly lopsided ramp on dark — 2.48:1 between levels 0–1 against 1.29:1 between 3–4. Both palettes are now inside a 1.55 spread, pinned by test.How I verified it
34 tests, written before the code:
DateTime(y, m, d + i), notDuration(days: 1), which loses or repeats a day across a clock change), local-day bucketing, multi-session days, streak edges including the "ends yesterday" allowance.zone_contrast_test.dart.!reducegate makes it fail, so it isn't vacuous.Full suite passes. One unrelated pre-existing flake in
workout_reliability_test.dart(DeriveScheduler) fails only under parallel load and passes in isolation.Known gaps
Summary by CodeRabbit