today: stop the Key Metrics header naming a window it is not drawing - #2377
Conversation
|
Device check on this branch, since the commit message says the fix had only been compiled. Built Release from this branch and installed on an iPhone 17 Pro (iOS 27.0 beta), 11.8.0 (400), default Liquid Today. With Show a trend graph beneath each metric off, the header now renders Stated precisely: what was confirmed on hardware is the graphs-off case, which is the default and the one the issue is about. The graphs-on case (label returns, naming the selected window) and the three other |
Apply the Apple patch from ryanbr/noop PR ryanbr#2377 by @andremiliano (3aed288). Android is intentionally unchanged in this iPhone-only fork.
ryanbr
left a comment
There was a problem hiding this comment.
Thanks @andremiliano, and sorry this sat. Four workflow runs had been parked awaiting approval since the 21st, which the checks API reports as no checks at all rather than as anything failing, so the PR looked unreviewable when it was only unapproved. I have approved them now; that is on me, not on you.
The change is right. The label names the window the DETAILED tiles graph, the tiles are opt-in and default off, and the header sat outside that gate on both platforms, so a default install announced a trend that nothing in the section drew.
I checked the parts that could have bitten:
sectionHeadreally does have exactly three other callers, all passing a non-nil string.DevicesViewhas its OWN privatesectionHead, so widening this one toString? = nilcannot reach it.- Both platforms gate on the same stored flag,
today.keyMetricsDetailed, which is also what gates the sparkline and the tile's minimum height on Apple anddetailedon Android. One flag, so the header cannot disagree with the tiles. - Android's
SectionHeaderkeeps rendering the row becauseoverlineis still passed; only the trailing caption goes.
Saying plainly that the hardware observation was of the BUG and not of the fix is the right way to write that section, and worth more than a screenshot would have been.
One correction, and it is ours
Built locally, since app-target Swift has no default CI
That was true of our docs, not of the repo. AGENTS.md listed app-build.yml as disabled and on-demand; it is active and auto-runs on PRs touching Strand/**, which yours does. I fixed that line and three comments that had repeated it in 5783c4996. Your local builds were the right instinct regardless.
Before merge
The branch is 94 commits behind main, and TodayScreen.kt and the Liquid Today view have both moved in that window. It merges cleanly, but the CI now starting will be judging a tree nobody will ship. Please rebase onto current main; maintainer_can_modify is on, so say the word if you would rather I did it.
Refs ryanbr#2376. The Key Metrics header carries a trailing "7-day trend" / "14-day trend" / "30-day trend" label naming the window the DETAILED tiles graph over. The detailed tiles are opt-in and default OFF, and the toggle behind them (`today.keyMetricsDetailed`) gated only the sparkline itself and the tile's minimum height. The header was outside that gate on both platforms, so on a default install nothing in the section drew a trend and the header announced one anyway. The tiles in that state render a value and a caption, so the label was the only thing on the screen framing the section as a trend, and there was no setting that removed it: the window picker's shortest choice is a week, and hiding the section takes the values with it. Both platforms gate the label on the same flag now. Android's SectionHeader already accepted `trailing: String? = null` and omits the row when both optional slots are null, so only the call site changed there. The Apple `sectionHead` helper took a required String and always rendered the Text, so it becomes `String? = nil` — the shape its Kotlin twin already had — and renders the caption only when one is passed. Its three other callers pass a non-nil string and are unaffected. Nothing about the window picker, the sparkline data, the tile layout or the label's wording changes; only whether the caption is rendered at all. VERIFICATION Built locally, since app-target Swift has no default CI: xcodebuild -scheme NOOPiOS -configuration Debug -destination 'generic/platform=iOS' build xcodebuild -scheme Strand -destination 'platform=macOS' CODE_SIGNING_ALLOWED=NO build cd android && ./gradlew compileFullDebugKotlin All three succeed; the Kotlin warnings in that run are pre-existing and none are on the changed line. What was observed on hardware is the BUG, not the fix: on an iPhone running 11.8.0 (400), turning the trend graphs off leaves "7-day trend" in the header. This build has not been run on a device, and no screenshot of the fixed header is offered. The change is a render gate on an existing flag, so the compile is the substantive check, but that distinction is worth stating rather than implying a device pass that did not happen. The single-day option the issue also raises is left alone: that is a design decision about the picker, not part of this fix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
3aed288 to
4786355
Compare
Refs #2376.
The Key Metrics header carries a trailing "7-day trend" / "14-day trend" /
"30-day trend" label naming the window the DETAILED tiles graph over. The
detailed tiles are opt-in and default OFF, and the toggle behind them
(
today.keyMetricsDetailed) gated only the sparkline itself and the tile'sminimum height. The header was outside that gate on both platforms, so on a
default install nothing in the section drew a trend and the header announced
one anyway.
The tiles in that state render a value and a caption, so the label was the
only thing on the screen framing the section as a trend, and there was no
setting that removed it: the window picker's shortest choice is a week, and
hiding the section takes the values with it.
Both platforms gate the label on the same flag now. Android's SectionHeader
already accepted
trailing: String? = nulland omits the row when bothoptional slots are null, so only the call site changed there. The Apple
sectionHeadhelper took a required String and always rendered the Text, soit becomes
String? = nil— the shape its Kotlin twin already had — andrenders the caption only when one is passed. Its three other callers pass a
non-nil string and are unaffected.
Nothing about the window picker, the sparkline data, the tile layout or the
label's wording changes; only whether the caption is rendered at all.
VERIFICATION
Built locally, since app-target Swift has no default CI:
xcodebuild -scheme NOOPiOS -configuration Debug -destination 'generic/platform=iOS' build
xcodebuild -scheme Strand -destination 'platform=macOS' CODE_SIGNING_ALLOWED=NO build
cd android && ./gradlew compileFullDebugKotlin
All three succeed; the Kotlin warnings in that run are pre-existing and none
are on the changed line.
What was observed on hardware is the BUG, not the fix: on an iPhone running
11.8.0 (400), turning the trend graphs off leaves "7-day trend" in the header.
This build has not been run on a device, and no screenshot of the fixed header
is offered. The change is a render gate on an existing flag, so the compile is
the substantive check, but that distinction is worth stating rather than
implying a device pass that did not happen.
The single-day option the issue also raises is left alone: that is a design
decision about the picker, not part of this fix.
🤖 Generated with Claude Code