From d65472580658f505104e05cfae54c317d4d95ee4 Mon Sep 17 00:00:00 2001 From: Fanboynz Date: Fri, 28 Aug 2026 19:04:17 +1200 Subject: [PATCH] today: open a session's detail from the Latest Workouts feed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Requested in #1694 by @h-nry. The Today feed listed sessions but every tile was inert, so the only route to a session's detail was More > Workouts. Each tile now opens the SAME read-only detail its Workouts list already opens — WorkoutDetailSheet on Android, WorkoutDetailView on iOS. Reusing those rather than building a Today-specific view is what makes this safe: neither can edit or delete, so a tap from Today carries no risk the list does not already carry. Two things the implementation had to get right rather than merely compile. The Today sections are LazyColumn items. Hosting the sheet inside TodayWorkoutsSection looks correct and builds clean, but a disposed item takes an open sheet down with it, so the state lives on TodayScreen and the sheet sits with Today's other dialogs — where the Workouts screen already keeps its own. That also leaves the section presentational: it reports a tap and no longer needs the view model at all. Press feedback had to come from each platform's existing primitive, not a plain button. iOS uses LiquidPressStyle, the twin of Android's liquidPress, so the tile settles inward on both. Accessibility diverges on purpose. Android gets an onClickLabel string in all seven locales, because Compose derives nothing and the Sources row beside it does the same; iOS lets Button derive its label from the tile content, exactly as the Workouts rows do. TodayWorkoutTapTest guards the wiring, including the lazy-disposal trap, which compiles cleanly and no other test here would catch. It reads source, so it strips comments before matching — this file's comments name the very symbols asserted on, and the first version matched its own prose. Verified: 4,712 tests, 0 failures, --rerun-tasks --no-build-cache; i18n audit clean. The iOS half is app-target Swift, so it is compiled by app-build, not the default CI. --- Strand/Screens/TodayView.swift | 45 +++++++++-- .../src/main/java/com/noop/ui/TodayScreen.kt | 26 +++++- .../main/java/com/noop/ui/WorkoutsScreen.kt | 2 +- .../app/src/main/res/values-de/strings.xml | 1 + .../app/src/main/res/values-es/strings.xml | 1 + .../app/src/main/res/values-fr/strings.xml | 1 + .../app/src/main/res/values-pl/strings.xml | 1 + .../src/main/res/values-pt-rPT/strings.xml | 1 + .../app/src/main/res/values-zh/strings.xml | 1 + android/app/src/main/res/values/strings.xml | 1 + .../java/com/noop/ui/TodayWorkoutTapTest.kt | 81 +++++++++++++++++++ 11 files changed, 149 insertions(+), 12 deletions(-) create mode 100644 android/app/src/test/java/com/noop/ui/TodayWorkoutTapTest.kt diff --git a/Strand/Screens/TodayView.swift b/Strand/Screens/TodayView.swift index 955c296eab..d351af4cb4 100644 --- a/Strand/Screens/TodayView.swift +++ b/Strand/Screens/TodayView.swift @@ -300,6 +300,14 @@ struct TodayView: View { // 14-day sparkline series, keyed by metric key. Loaded once in .task. @State private var sparks: [String: [Double]] = [:] @State private var workouts: [WorkoutRow] = [] + /// #1694: a tapped Latest-Workouts tile. Wrapped so `.sheet(item:)` drives presentation, mirroring + /// WorkoutsView's own detail target — the feed was read-only, so the only route to a session's + /// detail was More > Workouts. + private struct WorkoutDetailTarget: Identifiable { + let row: WorkoutRow + let id = UUID() + } + @State private var workoutDetail: WorkoutDetailTarget? @State private var appleDays: [AppleDaily] = [] // Design Reset / #582, the pinned "Your cards" values (Stress / Fitness age / Vitality), surfaced // on Today so the buried Explore features sit on the home screen. Loaded in loadAll; nil hides the row. @@ -1511,6 +1519,20 @@ struct TodayView: View { hostedCardsRaw: $hostedCardsRaw ) } + // #1694: the same read-only WorkoutDetailView the Workouts list opens. Nothing here can edit or + // delete, so a tap from Today carries no risk that list does not already carry. Rides its own + // NavigationStack because these shared screens are not in a per-screen one — mirrors WorkoutsView. + .sheet(item: $workoutDetail) { target in + NavigationStack { + WorkoutDetailView(row: target.row) + .environmentObject(repo) + } + #if os(iOS) + .noopSheetPresentation(largeFirst: true) + #else + .frame(width: 620, height: 720) + #endif + } #if os(iOS) .fullScreenCover(isPresented: $showLiveSession) { LiveSessionView(onClose: { showLiveSession = false }) @@ -3853,14 +3875,21 @@ struct TodayView: View { trailing: String(localized: "\(workouts.count) total")) LazyVGrid(columns: grid, alignment: .leading, spacing: NoopMetrics.gap) { ForEach(Array(workouts.prefix(6).enumerated()), id: \.offset) { _, w in - StatTile( - label: "\(WorkoutSource.displaySport(w.sport))", - value: workoutDuration(w), - caption: workoutCaption(w), - accent: StrandPalette.effortTint(fraction: (w.strain ?? 0) / StrainScorer.maxStrain), - delta: w.energyKcal.map { "\(Int($0.rounded())) kcal" }, - deltaColor: StrandPalette.metricAmber - ) + Button { + workoutDetail = WorkoutDetailTarget(row: w) + } label: { + StatTile( + label: "\(WorkoutSource.displaySport(w.sport))", + value: workoutDuration(w), + caption: workoutCaption(w), + accent: StrandPalette.effortTint(fraction: (w.strain ?? 0) / StrainScorer.maxStrain), + delta: w.energyKcal.map { "\(Int($0.rounded())) kcal" }, + deltaColor: StrandPalette.metricAmber + ) + } + // The Workouts list's own rows use this, not .plain: it is the iOS twin of + // Android's liquidPress, so the tile settles inward on press on both platforms. + .buttonStyle(LiquidPressStyle()) } } } 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 17cc5485b3..feccff58f1 100644 --- a/android/app/src/main/java/com/noop/ui/TodayScreen.kt +++ b/android/app/src/main/java/com/noop/ui/TodayScreen.kt @@ -663,6 +663,9 @@ fun TodayScreen( // existing RecoveryDriversSection (gated to the calibration countdown when the night can't score) plus // the folded Readiness card (S4). Not persisted, so it reopens closed. Mirrors iOS showChargeBreakdown. var showChargeBreakdown by remember { mutableStateOf(false) } + // #1694: the Latest-Workouts tile that was tapped. Held HERE, not inside the section: the Today + // sections are LazyColumn items, and a disposed item would take an open sheet down with it. + var selectedWorkoutRow by remember { mutableStateOf(null) } // LIVE SESSIONS (beta, default ON): the "Start session" entry under the hero + its full-screen Dialog // (the same presentation the live-workout overlay / Charge breakdown use — deliberately NOT a nav // destination, so dismissing it leaves the session's runner coaching and this entry is the way back @@ -1582,7 +1585,7 @@ fun TodayScreen( modifier = Modifier.fillMaxWidth().staggeredAppear(stagger), verticalArrangement = Arrangement.spacedBy(8.dp), ) { - TodayWorkoutsSection(footer.recentWorkouts) + TodayWorkoutsSection(footer.recentWorkouts, onSelect = { selectedWorkoutRow = it }) } // HEART RATE, the live HR thread / trend card. #991: header + card in a Column. TodaySection.HEART_RATE -> Column( @@ -1741,6 +1744,11 @@ fun TodayScreen( } } + // #1694: the tapped Latest-Workouts session, in the same read-only sheet the Workouts list opens. + selectedWorkoutRow?.let { row -> + WorkoutDetailSheet(vm = viewModel, row = row, onDismiss = { selectedWorkoutRow = null }) + } + // LIVE SESSIONS (beta): the full-screen session dialog — the same presentation the live-workout // overlay uses on Live (Dialog, usePlatformDefaultWidth = false). Dismissing it only HIDES the // screen: the runner (held in LiveSessionRunner.active, ticking on the app-wide viewModelScope) @@ -6213,7 +6221,7 @@ private fun WorkoutGlyph(icon: ImageVector, modifier: Modifier = Modifier) { // MARK: - Today footer sections @Composable -private fun TodayWorkoutsSection(workouts: List) { +private fun TodayWorkoutsSection(workouts: List, onSelect: (WorkoutRow) -> Unit) { // Single column, newest first: the 2x2 grid truncated durations on narrow phones and read as // unrelated stat tiles rather than a chronological feed. Full-width tiles have room for the // kcal chip, so the #332 compactDelta workaround is no longer needed here. @@ -6223,10 +6231,22 @@ private fun TodayWorkoutsSection(workouts: List) { // "Latest Workouts", not "Last": "Last" read as "final". Mirrored on iOS (TodayView). Lives in // strings.xml (values + values-de) so the header is localizable like the nav labels. SectionHeader(stringResource(R.string.today_latest_workouts), overline = uiString(R.string.today_workouts_activity), trailing = uiString(R.string.today_workouts_14_days)) + // #1694: the feed was read-only, so the only way to a session's detail was More > Workouts. The + // tap opens the SAME read-only WorkoutDetailSheet the Workouts list uses — nothing there can edit + // or delete, so a tap from Today carries no risk that list does not already carry. Column(verticalArrangement = Arrangement.spacedBy(Metrics.gap)) { feed.forEach { workout -> + val interaction = remember(workout.startTs, workout.source) { MutableInteractionSource() } StatTile( - modifier = Modifier.fillMaxWidth(), + modifier = Modifier + .fillMaxWidth() + .liquidPress(interaction) + .clickable( + interactionSource = interaction, + indication = null, + onClickLabel = uiString(R.string.today_action_show_workout), + onClick = { onSelect(workout) }, + ), label = WorkoutEditing.displaySport(workout.sport), value = localizedMetricValue(workoutDuration(workout)), caption = workoutCaption(workout), diff --git a/android/app/src/main/java/com/noop/ui/WorkoutsScreen.kt b/android/app/src/main/java/com/noop/ui/WorkoutsScreen.kt index 00edac54ea..a8fd9f0125 100644 --- a/android/app/src/main/java/com/noop/ui/WorkoutsScreen.kt +++ b/android/app/src/main/java/com/noop/ui/WorkoutsScreen.kt @@ -1483,7 +1483,7 @@ private fun SessionRow( @OptIn(ExperimentalMaterial3Api::class) @Composable -private fun WorkoutDetailSheet(vm: AppViewModel, row: WorkoutRow, onDismiss: () -> Unit) { +internal fun WorkoutDetailSheet(vm: AppViewModel, row: WorkoutRow, onDismiss: () -> Unit) { val sheetState = rememberModalBottomSheetState(skipPartiallyExpanded = true) // Per-window reads (#410): the HR curve (downsampled bucket means) and the HR-zone split. Zones diff --git a/android/app/src/main/res/values-de/strings.xml b/android/app/src/main/res/values-de/strings.xml index fcb0780a5f..e1857952ce 100644 --- a/android/app/src/main/res/values-de/strings.xml +++ b/android/app/src/main/res/values-de/strings.xml @@ -2271,6 +2271,7 @@ Herzfrequenz-Zoom zurücksetzen Einflüsse auf %1$s anzeigen Synchronisierte NOOP-Quellen anzeigen + Workout-Details anzeigen Einflussfaktoren Datenquellen Schläge pro Minute diff --git a/android/app/src/main/res/values-es/strings.xml b/android/app/src/main/res/values-es/strings.xml index 674bbb3297..e3a0732790 100644 --- a/android/app/src/main/res/values-es/strings.xml +++ b/android/app/src/main/res/values-es/strings.xml @@ -2257,6 +2257,7 @@ Restablecer zoom de frecuencia cardíaca Ver qué influyó en %1$s Mostrar las fuentes sincronizadas con NOOP + Mostrar los detalles del entrenamiento Factores Fuentes de datos Latidos por minuto diff --git a/android/app/src/main/res/values-fr/strings.xml b/android/app/src/main/res/values-fr/strings.xml index 18ea9857b8..a70dea87fc 100644 --- a/android/app/src/main/res/values-fr/strings.xml +++ b/android/app/src/main/res/values-fr/strings.xml @@ -2256,6 +2256,7 @@ Réinitialiser le zoom de fréquence cardiaque Voir ce qui a influencé %1$s Afficher les sources synchronisées avec NOOP + Afficher les détails de la séance Facteurs Sources de données Battements par minute diff --git a/android/app/src/main/res/values-pl/strings.xml b/android/app/src/main/res/values-pl/strings.xml index 32004d3c39..93827ec201 100644 --- a/android/app/src/main/res/values-pl/strings.xml +++ b/android/app/src/main/res/values-pl/strings.xml @@ -2250,6 +2250,7 @@ Zresetuj powiększenie tętna Zobacz, co wpłynęło na %1$s Pokaż źródła synchronizowane z NOOP + Pokaż szczegóły treningu Czynniki Źródła danych Uderzenia na minutę diff --git a/android/app/src/main/res/values-pt-rPT/strings.xml b/android/app/src/main/res/values-pt-rPT/strings.xml index 5ea8cdcde7..7d7936250e 100644 --- a/android/app/src/main/res/values-pt-rPT/strings.xml +++ b/android/app/src/main/res/values-pt-rPT/strings.xml @@ -2250,6 +2250,7 @@ Repor zoom da frequência cardíaca Ver o que influenciou %1$s Mostrar as fontes sincronizadas com o NOOP + Mostrar os detalhes do treino Contributos Fontes de dados Batimentos por minuto diff --git a/android/app/src/main/res/values-zh/strings.xml b/android/app/src/main/res/values-zh/strings.xml index ff2aae59af..54847fb5aa 100644 --- a/android/app/src/main/res/values-zh/strings.xml +++ b/android/app/src/main/res/values-zh/strings.xml @@ -2185,6 +2185,7 @@ 重置心率缩放 查看影响 %1$s 的因素 查看 NOOP 同步的数据源 + 查看锻炼详情 影响因素 数据源 每分钟心跳 diff --git a/android/app/src/main/res/values/strings.xml b/android/app/src/main/res/values/strings.xml index 55e2a8fc76..cdfd989943 100644 --- a/android/app/src/main/res/values/strings.xml +++ b/android/app/src/main/res/values/strings.xml @@ -2370,6 +2370,7 @@ Reset the heart rate zoom See what shaped your %1$s Show what NOOP is synced from + Show workout detail Contributors Data sources Beats per minute diff --git a/android/app/src/test/java/com/noop/ui/TodayWorkoutTapTest.kt b/android/app/src/test/java/com/noop/ui/TodayWorkoutTapTest.kt new file mode 100644 index 0000000000..deba608149 --- /dev/null +++ b/android/app/src/test/java/com/noop/ui/TodayWorkoutTapTest.kt @@ -0,0 +1,81 @@ +package com.noop.ui + +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test +import java.io.File + +/** + * #1694: the Today "Latest Workouts" tiles open the SAME read-only detail the Workouts list opens. + * + * Compose wiring has no logic function to assert against, so this reads the sources — the idiom + * [ResolvedSeriesCallSiteAuditTest] already uses. What it pins is not cosmetic: + * + * The Today sections are LazyColumn items. Hosting the sheet inside [TodayWorkoutsSection] compiles and + * looks correct, but a disposed item takes an open sheet down with it, so the sheet MUST stay at screen + * level — which is also where every other Today dialog lives. That mistake is invisible to a compiler + * and to every other test in this suite. + */ +class TodayWorkoutTapTest { + + private fun repoRoot(): File { + val userDir = File(System.getProperty("user.dir") ?: ".") + val candidates = listOf(userDir, File(userDir, ".."), File(userDir, "../..")) + return candidates.firstOrNull { File(it, "Strand/Screens/TodayView.swift").isFile } + ?: error("could not locate the repo root from ${userDir.absolutePath}") + } + + private fun todayScreenKt() = File(repoRoot(), "android/app/src/main/java/com/noop/ui/TodayScreen.kt").readText() + + /** + * The section body, from its declaration to the start of the next top-level composable, with `//` + * comments removed. Stripping matters: this file's comments NAME the very symbols asserted on, so a + * raw substring search matches the prose describing the code rather than the code. + */ + private fun workoutsSectionBody(source: String): String { + val start = source.indexOf("private fun TodayWorkoutsSection(") + assertTrue("TodayWorkoutsSection not found", start > 0) + val next = source.indexOf("\n@Composable", start) + val body = source.substring(start, if (next > start) next else source.length) + return body.lines().joinToString("\n") { it.substringBefore("//") } + } + + @Test + fun tilesAreClickableAndReportTheTappedRow() { + val body = workoutsSectionBody(todayScreenKt()) + assertTrue("the feed tiles must be clickable", body.contains(".clickable(")) + assertTrue("a tap must report the row it was on", body.contains("onSelect(workout)")) + assertTrue( + "the tap needs an accessibility action label, like the Sources row beside it", + body.contains("R.string.today_action_show_workout"), + ) + } + + @Test + fun theSheetIsHostedAtScreenLevelNotInsideTheLazySection() { + val source = todayScreenKt() + assertTrue( + "Today must open the Workouts list's own read-only sheet", + source.contains("WorkoutDetailSheet(vm = viewModel, row = row"), + ) + assertFalse( + "hosting the sheet inside the section loses it when the LazyColumn disposes that item", + workoutsSectionBody(source).contains("WorkoutDetailSheet("), + ) + } + + /** Parity: iOS reaches the same read-only detail from the same feed, and presents it at screen level. */ + @Test + fun iosTodayOpensTheSameDetailComponent() { + val swift = File(repoRoot(), "Strand/Screens/TodayView.swift").readText() + assertTrue("iOS tiles must open a detail", swift.contains("workoutDetail = WorkoutDetailTarget(row: w)")) + assertTrue( + "iOS must present the SAME read-only view the Workouts list uses", + swift.contains("WorkoutDetailView(row: target.row)"), + ) + assertTrue( + "the press treatment must match Android's liquidPress, not fall back to .plain", + swift.contains("LiquidPressStyle()"), + ) + } +}