From 7dded999564f693c0d53a4e80e0302cd02c61c59 Mon Sep 17 00:00:00 2001 From: Fanboynz Date: Sat, 10 Oct 2026 00:54:10 +1300 Subject: [PATCH] fix(today): take the workout Start back off Today The report asked for Start on Today as the first of five parts, the other four being the merge of workout recording and heart-rate coaching into one session: one Start/Pause/End pair, one screen, one saved summary. Only the button was built, so Today carried both of the entry points the report was about, Start session for coaching and Start workout for recording, which is more of the confusion it described rather than less. It also drew as a full-width primary slab above the day's scores, because the Start-beside-Add row it belongs to collapses to a single weighted child when Today passes no Add. Both Today screens go back to the active-workout indicator alone, on Apple and on Android, and so does the liquid Home. Starting a workout stays on Workouts, the centre FAB and Live. The supporting plumbing goes back with it rather than sitting unwired: WorkoutStartSection requires its Add again, WorkoutStartControl is a button again, and ActiveWorkoutIndicatorCard is private again. Refs #2467 --- Strand/Liquid/LiquidTodayView.swift | 4 +-- Strand/Screens/TodayView.swift | 18 ++++++------ Strand/Screens/WorkoutStartControl.swift | 28 ++++++------------- .../src/main/java/com/noop/ui/TodayScreen.kt | 27 ++++++++++-------- .../src/main/java/com/noop/ui/WorkoutStart.kt | 14 +++++----- 5 files changed, 43 insertions(+), 48 deletions(-) diff --git a/Strand/Liquid/LiquidTodayView.swift b/Strand/Liquid/LiquidTodayView.swift index c98e651c5d..7179ee9ba7 100644 --- a/Strand/Liquid/LiquidTodayView.swift +++ b/Strand/Liquid/LiquidTodayView.swift @@ -357,8 +357,8 @@ struct LiquidTodayView: View { // #105: the live "workout in progress" card, dropped in the liquid Home rewrite. Restored // here as the SAME leaf the classic TodayView renders (and Android's WorkoutInProgressCard), // pinned above the reorderable block so an active manual workout is immediately visible - // and opens the existing workout flow. Today also offers Start when no workout is active. - ActiveWorkoutIndicatorSection(showStart: selectedDayOffset == 0) + // and taps straight through to Live. Renders nothing when no workout is active. + ActiveWorkoutIndicatorSection() // #today-layout (parity with Android): every Today section — the Charge/Effort/Rest hero // and Start-session included — renders in the user's saved order. Reorder via the Arrange // sheet (the header's up/down button; native drag rows); the order persists under the diff --git a/Strand/Screens/TodayView.swift b/Strand/Screens/TodayView.swift index c4abf88b2e..4e91a53946 100644 --- a/Strand/Screens/TodayView.swift +++ b/Strand/Screens/TodayView.swift @@ -95,7 +95,7 @@ struct ActiveWorkoutIndicatorModel: Equatable { } } -struct ActiveWorkoutIndicatorCard: View { +private struct ActiveWorkoutIndicatorCard: View { let model: ActiveWorkoutIndicatorModel let onReturn: () -> Void @@ -165,23 +165,19 @@ struct ActiveWorkoutIndicatorCard: View { /// Leaf-isolated so an in-progress workout's ~per-sample `AppModel` churn (the elapsed clock tick + the /// rewritten `activeWorkout`) re-renders ONLY this card, never the whole Today dashboard, the same -/// leaf-isolation pattern the file documents for the live status/sync rows. With `showStart`, today's -/// idle state offers the shared workout picker; past days only show an active-workout indicator. +/// leaf-isolation pattern the file documents for the live status/sync rows. Renders nothing when no workout +/// is active, so the card auto-appears/clears purely off `AppModel.activeWorkout`. /// /// Non-private so the liquid Home (`LiquidTodayView`) renders the SAME leaf — the liquid rewrite dropped this /// indicator (#105), and sharing one implementation keeps the two Today screens (and Android's /// `WorkoutInProgressCard`) from drifting. It carries its own `app`/`router` environment objects, so a caller /// only needs to place `ActiveWorkoutIndicatorSection()` in its body. struct ActiveWorkoutIndicatorSection: View { - var showStart = false @EnvironmentObject var app: AppModel @EnvironmentObject var router: NavRouter var body: some View { - if showStart { - // Keep this host mounted when a workout starts so its picker and live view survive the update. - WorkoutStartControl(showsActiveIndicator: true) - } else if let model = ActiveWorkoutIndicatorModel.make(from: app.activeWorkout) { + if let model = ActiveWorkoutIndicatorModel.make(from: app.activeWorkout) { ActiveWorkoutIndicatorCard(model: model) { StrandHaptic.selection.play() router.openActiveWorkout() @@ -1491,7 +1487,11 @@ struct TodayView: View { // A "workout in progress" indicator whenever a manual workout is active. A tap routes to Live // and opens the in-exercise screen. Its own leaf owns the AppModel observation + per-second // clock, so the live tick never re-renders TodayView.body. - ActiveWorkoutIndicatorSection(showStart: selectedDayOffset == 0) + // + // No Start here, the Android twin carries the reasoning (TodayScreen.kt, the same block): + // #2467 wanted Today's Start to arrive WITH the recording-plus-coaching merge, and only the + // button was built, so Today held both of the entry points the report was about. + ActiveWorkoutIndicatorSection() // The "still building" and "new here?" prompts are about getting today's scores going, // so they stay anchored to today rather than reappearing on every navigated past day. if selectedDayOffset == 0 && repo.today?.recovery == nil { diff --git a/Strand/Screens/WorkoutStartControl.swift b/Strand/Screens/WorkoutStartControl.swift index 8c91c8ef87..603bd976df 100644 --- a/Strand/Screens/WorkoutStartControl.swift +++ b/Strand/Screens/WorkoutStartControl.swift @@ -15,31 +15,21 @@ import StrandDesign /// re-renders only this small leaf. Owns its own sheet-presentation state so nothing about it needs to /// live on the parent either. struct WorkoutStartControl: View { - var showsActiveIndicator = false @EnvironmentObject var model: AppModel @State private var showLiveWorkout = false @State private var showStartSport = false var body: some View { - Group { - if showsActiveIndicator, let active = ActiveWorkoutIndicatorModel.make(from: model.activeWorkout) { - ActiveWorkoutIndicatorCard(model: active) { - StrandHaptic.selection.play() - showLiveWorkout = true - } - } else { - NoopButton(model.activeWorkout == nil ? "Start workout" : "View active workout", - systemImage: model.activeWorkout == nil ? "figure.run" : "timer", - kind: .primary, - fullWidth: true) { - // No active session → pick a named sport first (#519), then the sheet's onStart begins it - // and opens the in-exercise view. Already active → jump straight back into the live view. - if model.activeWorkout == nil { showStartSport = true } - else { showLiveWorkout = true } - } - .accessibilityLabel(model.activeWorkout == nil ? "Start a workout" : "View the active workout") - } + NoopButton(model.activeWorkout == nil ? "Start workout" : "View active workout", + systemImage: model.activeWorkout == nil ? "figure.run" : "timer", + kind: .primary, + fullWidth: true) { + // No active session → pick a named sport first (#519), then the sheet's onStart begins it + // and opens the in-exercise view. Already active → jump straight back into the live view. + if model.activeWorkout == nil { showStartSport = true } + else { showLiveWorkout = true } } + .accessibilityLabel(model.activeWorkout == nil ? "Start a workout" : "View the active workout") // #459: the in-exercise view, presented when Start Workout is tapped here (same screen LiveView // shows). activeWorkout is global on AppModel, so ending it from either surface stays in sync. .sheet(isPresented: $showLiveWorkout) { diff --git a/android/app/src/main/java/com/noop/ui/TodayScreen.kt b/android/app/src/main/java/com/noop/ui/TodayScreen.kt index 857d1a035d..c6ba971de6 100644 --- a/android/app/src/main/java/com/noop/ui/TodayScreen.kt +++ b/android/app/src/main/java/com/noop/ui/TodayScreen.kt @@ -1501,17 +1501,22 @@ fun TodayScreen( } } - // Today's shared workout host offers Start when idle and the recording controls when active. - // Past days retain the active-workout shortcut. Per-second clocks remain inside their leaves. - if (selectedDayOffset == 0) { - // Keep the existing picker/live-view host mounted across idle → recording transitions. - // Manual entry stays on Workouts; Today offers the same live recording controls. - item { WorkoutStartSection(viewModel) } - } else { - activeWorkout?.let { w -> - item { - WorkoutInProgressCard(workout = w, onReturn = onOpenActiveWorkout) - } + // A "workout in progress" indicator whenever a manual workout is active (iOS parity: the Today + // ActiveWorkoutIndicator). A tap routes to Live and re-opens the in-exercise overlay. Gated purely on + // `activeWorkout`, so it auto-appears/clears with no extra lifecycle wiring. Its per-second clock + // ticks inside the card's own LaunchedEffect, never recomposing the Today body. + // + // STARTING a workout is deliberately not offered here, and that is the whole of #2467's first part + // being held back until its other four exist. #2467 asked for Today's Start to come WITH the merge + // of recording and heart-rate coaching into one session: one Start/Pause/End pair, one screen, one + // saved summary. Only the button was built, so Today ended up carrying both of the entry points the + // report opened about, "Start session" for coaching and "Start workout" for recording, which is more + // of the confusion it described rather than less. It also drew as a full-width primary slab above the + // scores, because the Start-beside-Add row it belongs to collapses to a single weighted child when + // Today passes no Add. Starting stays on Workouts, the centre FAB and Live meanwhile. + activeWorkout?.let { w -> + item { + WorkoutInProgressCard(workout = w, onReturn = onOpenActiveWorkout) } } diff --git a/android/app/src/main/java/com/noop/ui/WorkoutStart.kt b/android/app/src/main/java/com/noop/ui/WorkoutStart.kt index 9532d19757..a30b4700ef 100644 --- a/android/app/src/main/java/com/noop/ui/WorkoutStart.kt +++ b/android/app/src/main/java/com/noop/ui/WorkoutStart.kt @@ -181,13 +181,13 @@ private fun StartSportRow(sp: Sport, isSelected: Boolean, onPick: () -> Unit) { } /** - * Shared workout entry for Workouts and Today — mirrors the Live screen's control so a user - * can begin a session from each place. Shows a compact "running" banner while a workout is active + * Start-a-workout entry for the Workouts screen (#115) — mirrors the Live screen's control so a user + * can begin a session from either place. Shows a compact "running" banner while a workout is active * (the rich live card stays on Live); otherwise an action row with Start (when a strap is bonded, since - * a live session needs the strap to stream). Workouts supplies Add; Today omits manual entry. + * a live session needs the strap to stream) beside Add — or just Add when there's no strap. */ @Composable -fun WorkoutStartSection(vm: AppViewModel, onAdd: (() -> Unit)? = null) { +fun WorkoutStartSection(vm: AppViewModel, onAdd: () -> Unit) { val live by vm.live.collectAsStateWithLifecycle() val activeWorkout by vm.activeWorkout.collectAsStateWithLifecycle() var showSportPicker by remember { mutableStateOf(false) } @@ -261,7 +261,7 @@ fun WorkoutStartSection(vm: AppViewModel, onAdd: (() -> Unit)? = null) { } } } - if (onAdd != null) AddWorkoutButton(onAdd, Modifier.fillMaxWidth()) + AddWorkoutButton(onAdd, Modifier.fillMaxWidth()) } } else if (live.bonded) { // Start + Add as an equal-width action row (EXP-018 parity with the iOS workoutActionRow). @@ -278,9 +278,9 @@ fun WorkoutStartSection(vm: AppViewModel, onAdd: (() -> Unit)? = null) { onClick = { showSportPicker = true }, modifier = Modifier.weight(1f), ) - if (onAdd != null) AddWorkoutButton(onAdd, Modifier.weight(1f)) + AddWorkoutButton(onAdd, Modifier.weight(1f)) } - } else if (onAdd != null) { + } else { // No strap to stream from: no live Start, but keep Add so a user with no imports can still log. // PRIMARY here, unlike the pair above. There is no Start to be secondary to, and this is the // only action the screen offers, so the secondary treatment would leave a wearer with no strap