Refactor Sleep UI sections#734
Conversation
ryanbr
left a comment
There was a problem hiding this comment.
The extraction itself looks good — a genuine behavior-preserving move, and the SleepChartCard/SleepChartFooter/SleepMetricsHairline rename + internal promotion is the right way to let the moved code reach the former private helpers. Scope is UI-only (no analytics/storage/wiring touched). Two things block it, and the first means it needs a re-review after you update it.
1. Needs a rebase — and it's not a trivial one
The PR is currently CONFLICTING. SleepScreen.kt changed on main after this branched — #748 (compact metric-summary layouts) and #743 (compact range selectors) both edited Sleep UI. Since this PR deletes ~1,162 lines of that same file, please re-apply the extraction on top of the current SleepScreen.kt rather than resolving the conflict mechanically — a blind merge here can silently drop #748/#743's changes. The resulting diff will be meaningfully different, so I'll give it a fresh pass once it's rebased.
2. pt-PT coverage for the two moved strings
The green i18n check is stale — it ran before #769 made pt-PT a focus locale. The two delete-dialog strings you moved into resources are in de/es/fr but not values-pt-rPT, so after rebasing onto current main the gate fails with 2 pt-PT gaps:
sleep_delete_user_edited_bodysleep_delete_recorded_body
Add both to android/app/src/main/res/values-pt-rPT/strings.xml (Portuguese) as part of the rebase.
Note
Unit tests weren't run (the DeviceRegistryDao.setModel fake-DAO blocker, tracked separately). It's a UI-only move so behavioral risk is low, but the rebase is exactly where a large move-refactor can go wrong — worth a compileFullDebugKotlin + a manual pass over the Sleep screen after re-applying it on current main.
Happy to re-review as soon as it's rebased with the pt-PT strings added.
What this PR does
Moves three remaining Sleep screen UI clusters into sibling files:
SleepNightNavUi.ktfor the sleep window row and night navigation/edit/delete controls.SleepMetricCardsUi.ktfor the metric grid, sleep debt ledger, stage comparison, trend cards, and shared Sleep chart helpers.SleepMetricDetailSheet.ktfor the metric detail bottom sheet.This keeps the existing comments with the code they describe and reduces
SleepScreen.ktto about 2.1k lines without changing sleep calculations, storage, BLE behavior, or screen wiring. Two delete-dialog body strings were moved to Android resources so the i18n gate stays clean after the file move.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