Repository navigation
today: tap a hero ring to open its metric detail - #1995
Merged
Merged
Conversation
ryanbr
force-pushed
the
feat-hero-rings-open-detail
branch
4 times, most recently
from
September 8, 2026 05:54
5a0f351 to
1778ad7
Compare
The three hero rings on the liquid Today screen only splashed. Their labels tapped through to the scoring guide, so the whole card explained how a score is calculated but offered no way to the score's own data, and a user had to scroll to the Key Metrics tile below to reach it. A reporter asked for the rings themselves to open the detail so the tiles were not the only route. The gauge now taps through to `TabRoute.metric(key)`, the same value-based route the Recovery Vitals rows use, resolving to the same MetricDetailView the tile for that score already pushes. Charge goes to "recovery", Effort to "strain", Rest to "sleep_performance": the exact keys those tiles pass, so a ring and the tile under it can never lead to different screens. The label keeps its own job. It still opens the scoring guide, which is this screen's only route to that explainer, so the change adds a destination rather than replacing one. The splash survives. A plain onTapGesture inside a NavigationLink consumes the tap and the link never fires, which is why a tappable gauge needs the splash to run as a SIMULTANEOUS gesture instead. LiquidVessel gains an opt-in `tapPassesThrough` that does exactly that; it defaults false, so every standalone vessel behaves as before. Two tests pin the keys against MetricCatalog. `TabRoute.metric` falls back to the Health screen for an unknown key, so renaming a catalog entry would leave the ring working, animating and landing on the wrong screen with nothing to notice. These turn that into a build failure. The ring's spoken label carries the SCORE, not just the metric name. The ring is the element showing the number, so a VoiceOver user hearing only "Charge" on it while the label below reads the value would have it backwards. One new hint string, appended to the catalogue with all nine non-English locales. Android's Effort and Rest rings get the same treatment, opening their own vital detail through the existing onOpenMetric callback that the metric cards below already use, so a ring and its card cannot lead to different screens there either. The Rest key is NOT the same on the two platforms and that is deliberate. iOS routes on "sleep_performance"; Android's VitalDetailScreen resolves "rest" and reads the sleep_performance series underneath it. Those are two namespaces, series keys and detail keys. Using the iOS key on Android would compile, run and render an empty screen, because that `when` ends in `else -> null`: no crash, no log. Both keys are named constants now, pinned by a test that also asserts "sleep_performance" is not a detail key. Android's Charge ring still opens its breakdown sheet, and the sheet now offers the trend from inside. Routing the ring away was the obvious way to make the three rings behave alike, and it is wrong: the sheet's only other entry is the Synthesis card, which sits in a TodaySection the user can hide, so anyone who hid it would lose the breakdown entirely. A row beside "How Charge is calculated" resolves the inconsistency instead, and loses nothing. So on both platforms every hero ring now leads to that score's own detail. Charge's is simply the richer one on Android, with the trend a tap further in.
ryanbr
force-pushed
the
feat-hero-rings-open-detail
branch
from
September 8, 2026 06:00
1778ad7 to
6348400
Compare
ryanbr
added a commit
that referenced
this pull request
Sep 8, 2026
…#1996) LiquidVessel puts a clickable on its own Canvas to drive the splash, and in Compose a child's clickable consumes the event, so a clickable parent never sees a touch that lands on the vessel. The hero rings were wrapped rather than wired, so Charge's ring tap had been inert on Android for as long as it existed and the Effort and Rest rings from #1995 inherited it. Only the animated render attaches that clickable, so anyone on reduce-motion or power-save kept a working ring, which is much of why this went unnoticed. The action is now passed into the vessel instead. A sweep of every call site on both platforms found five more: CoupledScreen's HeroCard and SleepCard on Android, and on iOS, where the splash gesture is attached unconditionally and animated: false is no protection, the two Today rows plus CoupledView's sleep ring and HealthView's Fitness Age hero. Effort and Rest also stop announcing "See what shaped your Effort" for a ring that opens a trend.
7 tasks done
DX23876
added a commit
to DX23876/noop
that referenced
this pull request
Sep 16, 2026
Thirty real conflicts. The Android tree is dropped again per FORK_GUIDE; every other hunk was resolved by asking which mechanism this fork actually runs, not by preferring a side. Taken from upstream: • Protocol: ryanbr#2017/ryanbr#2018/ryanbr#2020 — the 5/MG historical optical and raw-IMU layouts, and the v26 window's ABSOLUTE base code beside its deltas. Its migration lands as `v57-ppg-waveform-base-code` (upstream's v44), since this fork's sequence is nine ahead of theirs. • BLE diagnostics ryanbr#1998-ryanbr#2003: a bond end that says WHY, named BLE integers, a session-budgeted hex dump, and "we hold the link" only when the OS says so. • Battery: ryanbr#1951 stop asking a 5/MG for a reading it refuses; ryanbr#1959 poll 0x2A19 for an unbonded one. • Scoring: ryanbr#1978/ryanbr#1982/ryanbr#1990 the RHR/HRV baseline gates, ryanbr#1977 the strain denominator's domain, RMSSD withheld on an over-counted night, ryanbr#1943 the resting-HR bin gate, the closed final HR/HRV window, HR-recovery requiring one CONTINUOUS effort, ryanbr#1984 nights bridged under minSleepMin, ryanbr#2015 motion re-folded only when that day's gravity moved, ryanbr#2016 a provisional Rest. • Charts ryanbr#2004/ryanbr#2005/ryanbr#2007/ryanbr#2009/ryanbr#2011 and ryanbr#1995's hero-ring detail route, ryanbr#1164's "Pending sync" caption under Rest. • Widgets: the heart-rate widget with its trace (ryanbr#1957/ryanbr#1964) and the widget cost counters. • Health: ryanbr#1503 — the Apple Health external UUID drops the strap id, so a re-pair no longer strands every prior record, plus the one-off sweep that reclaims the stranded ones; ryanbr#1205 GPS routes read back from Apple Health. • Storage: rolling retention for ppgWaveformSample, the skin-temp absolute backfill outside the 21-night window, per-table byte estimates. • Units split (body vs exercise distance), per-day wake overrides, the wind-down nudge pinned to its own day, and the German Sie→du conversion (29 entries, merged per LOCALE so the fork's own de edits stayed). Kept on the fork's side, with the reason: • ryanbr#1005's in-memory `dayScanCache` stays out, again: the persisted `DayScanFingerprint` survives relaunch and its key folds the per-day weight and max HR upstream's does not. • The steps-motion cache stays PERSISTED for the same reason. Upstream's failed-read-vs-empty-day distinction IS adopted into it — on a persisted cache a stale zero from a read error would outlive the process that made it. • Every stream read in the day loop keeps `limit: Int.max`. All five are uncapped together; taking StreamReadCap for three of them would leave the loop half-capped, which is worse than either choice. • The coach engine. Upstream's K1-K15 layer is not adopted: its single 40-row `coachMessage` table, its tool-less pre-baked context and its own brief are behind what this fork already ships. Its ADDITIVE half is kept — the on-device voice input, the Siri "Ask Coach" intent, the brief scheduler and brief widget, and streaming for the OpenAI-compatible providers this fork's Anthropic-only StreamingToolClient does not cover. They are wired up in the next commit; `CoachBriefWidget` is deliberately not registered until then. • Liquid's `DomainTheme` labels, value-sampled Charge tint, `LiquidScoreGauge` and `TodayCardSurface`; classic Today's ring order and `LiveEffort.today`. • RootTabView's four-tab shell and `MoreCatalog`: Coach hangs on content here, not on a tab. Two duplicate mechanisms collapsed rather than carried: • `ChargeBreakdownWiring` (upstream) and `ChargeBreakdownFormat` (fork) are the same extraction done twice. The fork's survives, Coupled now reads it, and upstream's TEST is kept — it is coverage this path never had. • Upstream's `CoachMessageStore` is dropped; `CoachConversationStore` already autosaves the whole multi-conversation history. One defect found while resolving, fixed here: `LiveEffort.today` read HR with no explicit limit, so `hrSamples`' 8000-row default truncated it — at ~18k rows a day that covered about ten hours and the live Effort silently stopped climbing. It failed safe (`effectiveEffort` takes the max), which is why it went unseen. `Live5RestFrozenTests` is removed by upstream, which replaces it with `Live5NoGravityRestChainTests`. `local_day_windows_oracle.json` moves out of upstream's android path into `Tests/StrandAnalyticsTests/Oracles/` — NOT `Fixtures/`, which this repo's .gitignore excludes everywhere. Verified: StrandAnalytics 2233, WhoopStore 627, WhoopProtocol 700, StrandImport 293, StrandDesign 120, NoopLocalAccess 14, SemanticMemory 19, OuraProtocol 209 and PolarProtocol 25 all green; macOS and iOS both build; doc_comment_lint clean; `i18n_audit --ci` byte-identical to its pre-merge output (it is red on main for reasons that predate this, and the seven new widget literals were given all nine locales so the gate did not get worse). Six StrandTests were ALREADY failing on main before this merge and still are — `CoachMyLogsAndZonesTests` (the tool census pin was not bumped when the Hevy tools landed) and five in `RawPhysiologyUnionTests`. Every input to them — CoachTools.swift, ToolConsent.swift, Repository.swift and both test files — is byte-identical to main, so this merge neither caused nor fixed them. They went unnoticed because app-build.yml last ran on 2026-08-25. Analysis migration required: YES. The scoring changes above make stored derived values stale; the recipe bump and its resumable migration are the next commit.
ryanbr
added a commit
that referenced
this pull request
Sep 18, 2026
#2311 swapped Today's hero gauges from liquid vessels to GlowRing arcs. The rings are the better default and stay the default, but the vessels were a look people chose NOOP for, so keep them available instead of deleting them outright. Adds noop.todayRingGauges, default true, with a toggle beside "Reduce motion in NOOP" in Settings. It is deliberately NOT Apple's noop.liquidTodayEnabled: that key picks between two whole Today SCREENS there, while this picks a gauge inside the single Android one. Sharing one key would bind two unrelated meanings together and make a later divergence on either platform quietly wrong. Three gauge sites branch: the hero, the 26dp vital row and the 30dp card row. Two things the vessel rendering needs that the ring does not, both restored rather than left at their defaults: - onTap. The vessel's animated render owns a clickable that CONSUMES the touch, so a clickable parent never sees a tap landing on it (#1995). That is why #2311 could move the handlers up to HeroRingColumn.onRingTap and drop the forwarding. With the vessel selectable again the forwarding comes back, for that rendering only. The two small sites pass animated = false, which attaches no clickable, so they need none. - The empty-hero cost gate. The vessels ran static until the row had a real score, so a brand-new user's launch churn was not fighting four live canvases (Android's equivalent of the iOS dataLoaded gate on HeroScoreCell). Reduce Motion would have masked its absence, so this would only have shown up on a fresh install with motion on. GlowRing needs no equivalent: it springs once to its target and then sits still. Strings added in all 8 Android locales. Tests pin the default, since an install that has never opened Settings has to get rings, and a pref defaulting the other way would silently undo a shipped change for everyone who upgrades.
ryanbr
added a commit
that referenced
this pull request
Sep 18, 2026
#2311 swapped Today's hero gauges from liquid vessels to GlowRing arcs. The rings are the better default and stay the default, but the vessels were a look people chose NOOP for, so keep them available instead of deleting them outright. Adds noop.todayRingGauges, default true, with a toggle beside "Reduce motion in NOOP" in Settings. It is deliberately NOT Apple's noop.liquidTodayEnabled: that key picks between two whole Today SCREENS there, while this picks a gauge inside the single Android one. Sharing one key would bind two unrelated meanings together and make a later divergence on either platform quietly wrong. Three gauge sites branch: the hero, the 26dp vital row and the 30dp card row. Two things the vessel rendering needs that the ring does not, both restored rather than left at their defaults: - onTap. The vessel's animated render owns a clickable that CONSUMES the touch, so a clickable parent never sees a tap landing on it (#1995). That is why #2311 could move the handlers up to HeroRingColumn.onRingTap and drop the forwarding. With the vessel selectable again the forwarding comes back, for that rendering only. The two small sites pass animated = false, which attaches no clickable, so they need none. - The empty-hero cost gate. The vessels ran static until the row had a real score, so a brand-new user's launch churn was not fighting four live canvases (Android's equivalent of the iOS dataLoaded gate on HeroScoreCell). Reduce Motion would have masked its absence, so this would only have shown up on a fresh install with motion on. GlowRing needs no equivalent: it springs once to its target and then sits still. Strings added in all 8 Android locales. Tests pin the default, since an install that has never opened Settings has to get rings, and a pref defaulting the other way would silently undo a shipped change for everyone who upgrades.
ryanbr
added a commit
that referenced
this pull request
Sep 18, 2026
) feat(android): let Today choose ring gauges or the liquid vessels #2311 swapped Today's hero gauges from liquid vessels to GlowRing arcs. The rings are the better default and stay the default, but the vessels were a look people chose NOOP for, so keep them available instead of deleting them outright. Adds noop.todayRingGauges, default true, with a toggle beside "Reduce motion in NOOP" in Settings. It is deliberately NOT Apple's noop.liquidTodayEnabled: that key picks between two whole Today SCREENS there, while this picks a gauge inside the single Android one. Sharing one key would bind two unrelated meanings together and make a later divergence on either platform quietly wrong. Three gauge sites branch: the hero, the 26dp vital row and the 30dp card row. Two things the vessel rendering needs that the ring does not, both restored rather than left at their defaults: - onTap. The vessel's animated render owns a clickable that CONSUMES the touch, so a clickable parent never sees a tap landing on it (#1995). That is why #2311 could move the handlers up to HeroRingColumn.onRingTap and drop the forwarding. With the vessel selectable again the forwarding comes back, for that rendering only. The two small sites pass animated = false, which attaches no clickable, so they need none. - The empty-hero cost gate. The vessels ran static until the row had a real score, so a brand-new user's launch churn was not fighting four live canvases (Android's equivalent of the iOS dataLoaded gate on HeroScoreCell). Reduce Motion would have masked its absence, so this would only have shown up on a fresh install with motion on. GlowRing needs no equivalent: it springs once to its target and then sits still. Strings added in all 8 Android locales. Tests pin the default, since an install that has never opened Settings has to get rings, and a pref defaulting the other way would silently undo a shipped change for everyone who upgrades.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A user asked for the hero rings on the liquid Today screen to open detail, so the Key Metrics tiles below are not the only way to reach a score's own data.
What they did before
HeroScoreCellgave the ring a splash animation and nothing else. The label plus its chevron openedScoringGuideView. So the card explained how Charge, Effort and Rest are calculated but offered no route to what they actually are, and the obvious target, the ring, did nothing but ripple.Worth noting for anyone reading the earlier triage: this is
LiquidTodayView, notTodayView. The classic screen's Charge ring does open a breakdown sheet; this one's does not, and never did. Same app, different Today.What changed
The gauge taps through to
TabRoute.metric(key), the same value-based route the Recovery Vitals rows already use, landing on the sameMetricDetailViewthe Key-Metrics tile for that score pushes:recoverystrainsleep_performanceThose are the exact keys the tiles beneath already pass, so a ring and the tile under it cannot lead to different screens.
The label keeps its own job. It still opens the scoring guide, which is this screen's only route to that explainer. The change adds a destination rather than replacing one.
The splash survives, and that took a change
A plain
onTapGestureinside aNavigationLinkconsumes the tap, so the link never fires. That is precisely why a gauge that splashes could not also navigate.LiquidVesselgains an opt-intapPassesThroughwhich runs the splash as asimultaneousGestureinstead, so both happen: the liquid still splashes and the link still pushes.It defaults to
false, so every other vessel in the app is byte-identical. Only the three hero rings opt in.Tests
Two, pinning the keys against
MetricCatalog. This is not ceremony:TabRoute.metricfalls back to the Health screen when a key does not resolve, and its own comment calls that fallback theoretical. Rename a catalog key and the ring would keep working, keep animating, and quietly land on the wrong screen with no crash and nothing in a log. These turn that into a build failure, and they also guard the ring/tile pair from drifting apart.Scope
Android
The Effort and Rest rings get the same treatment, opening their own vital detail through the
onOpenMetriccallback the metric cards below already use, so a ring and its card cannot lead to different screens there either.The Rest key is not the same on the two platforms, and that is deliberate. iOS routes on
sleep_performance; Android'sVitalDetailScreenresolvesrest, and reads thesleep_performanceseries underneath it. Those are two namespaces: series keys and detail keys. Using the iOS key on Android would compile, run, and render an empty screen, because thatwhenends inelse -> null, so there is no crash and nothing in a log. Both are named constants now, pinned by a test that also assertssleep_performanceis not a detail key, which is the mistake a future reader is most likely to make.Android's Charge ring still opens its breakdown sheet, and the sheet now offers the trend from inside.
Routing the ring straight to the trend is the obvious way to make three identical-looking rings behave alike, and it is wrong. That sheet's only other entry is the Synthesis card, and
SYNTHESISis aTodaySectionthe user can hide, so anyone who has hidden it would lose the breakdown entirely. A "See the Charge trend" row beside the existing "How Charge is calculated" resolves the inconsistency without losing anything.So on both platforms every hero ring now leads to that score's own detail. Charge's is simply the richer one on Android, with the trend one tap further in.
Android suite: 670 classes, 5,659 tests, 0 failures.
i18n_audit.py --ci --platform androidreports no new gaps.Two new Android strings, added across all seven locales with keys generated by the repository's own algorithm, which I verified rather than assumed:
sha1("How Charge is calculated")[:8]really is142d5e8e. Each locale uses its own established term for Charge (Ladung, Carga, charge, Energia, Carga, Заряд, 充能). Android says "Ladung" in German where iOS says "Energie"; that divergence pre-dates this and is left alone rather than quietly changed.Accessibility and localisation
The ring's spoken label carries the score, not just the metric name. The ring is the element showing the number, so a VoiceOver user hearing only "Charge" on it, while the label beneath reads the value, would have it backwards.
That needs one new hint string. I claimed in an earlier draft of this description that there were no new strings, which was wrong:
"Opens the trend and readings"is new. It is appended to the catalogue with all nine non-English locales, text-spliced rather than round-tripped, so the diff is 11 lines rather than a reformat of a 6 MB file.i18n_audit.py --cireports no new gaps.Swift rests on CI as always.