Refactor Sleep stage breakdown UI#733
Conversation
ryanbr
left a comment
ryanbr
left a comment
There was a problem hiding this comment.
Verified as a genuine pure move rather than taking the label on trust — extracted every function body from both sides and compared them.
Five functions moved, five removed, sets identical. Bodies are byte-for-byte unchanged except one thing: private → internal on exactly the three that cross the file boundary (ClockLabelRow, HypnogramWithAxis, StageBreakdownRows). That's the minimum widening the move requires.
The discipline is what makes this easy to approve. StageBreakdownRow and stageColorFor stay private because they're only called from inside the new file — no blanket widening while you were in there. I checked the two apparent external references and both are false hits: StageBreakdownRow matches as a substring of StageBreakdownRows, and stageColorFor appears only in a comment in SleepStageTimelineLogic.kt:128.
The +30 line delta is package declaration, imports and file header, not logic.
On the untestable-tests note in your description — that's not on you. DeviceRegistryDao.setModel blocking testFullDebugUnitTest is the same wall @dnesdan hit on #808 today, and #725 is the open fix for it. You're the third person to run into it. Nothing Kotlin-side can be unit-tested until that lands, which is a repo problem, not a PR problem.
Thanks for running compileFullDebugKotlin locally and saying so — android.yml is disabled, so a local compile is the only build this code gets. That matters more here than usual.
Approving. Behaviour-preserving, minimally scoped, and it moves the stage-UI cluster next to the existing Sleep logic helpers where it belongs.
…ty by comment (#829) stageColorFor did its own trim().lowercase() and folded "wake"/"awake" in a when branch, duplicating canonicalStage in SleepStageTimelineLogic — which carried the comment "(stageColorFor parity)", a drift risk documented rather than removed. #733 did not create this; it made it visible by moving the two halves into adjacent files. stageColorFor now keys off canonicalStage. Both files are package com.noop.ui and canonicalStage is internal, so no plumbing was needed. Adding a stage alias is a one-place edit and the tint follows; before, a new alias would silently not colour. Behaviour-identical by inspection, not just by sampling: canonicalStage is trim().lowercase() plus "wake" -> "awake", so the only change to the when subject is that "wake" now arrives as "awake", which the same arm still catches. No input can reach a different branch. Checked across "deep", " REM ", "Light", "awake", "WAKE", "wake", whitespace variants, empty and unknown; all resolve to the same tint, fallback included. Also fixed three documentation defects introduced while writing this change: a KDoc link to a private symbol in another file (would not resolve), the last stale "(stageColorFor parity)" note in StageRowSpansTest, and a stacked pair of /** */ blocks on stageColorFor that silently detached the original case-insensitive note. Android only. No schema, no scores, no strings; i18n clean. Not compiled — android.yml is disabled.
2 participants
What this PR does
Moves the Sleep screen stage breakdown and hypnogram rendering helpers into
SleepStageBreakdownUi.kt. This keeps the stage UI cluster beside the existing Sleep logic helper files and reducesSleepScreen.ktwithout changing behavior, calculations, persistence, or BLE paths.Type of change
How it was tested
git diff --checkpython3 Tools/i18n_audit.py --ci origin/mainJAVA_HOME=/opt/homebrew/Cellar/openjdk@21/21.0.11/libexec/openjdk.jdk/Contents/Home ANDROID_HOME=$HOME/Library/Android/sdk ./gradlew --dependency-verification=off :app:compileFullDebugKotlinfromandroid/Android unit tests were not run for this UI-only extraction; the current test task is already blocked by the existing
DeviceRegistryDao.setModel(id, model)fake DAO mismatch tracked separately.Checklist
swift testinPackages/<name>)android/(./gradlew testFullDebugUnitTest)StrandDesigntokens — no hardcoded colors, fonts, or spacingdocs/CONTRIBUTING.mdStrand.xcodeproj/) or any secrets/keystoresRelated issues
Refs #665