Repository navigation
Confirm before ending an active workout - #523
Conversation
|
Reviewed — this is ready, and first let me own something. This was filed before the PR that superseded it. #523 went up 16 Jul 18:47; #561 went up 17 Jul 19:52 and merged on the 19th. So you were 25 hours first and this sat while a later PR for the same issue landed. That is on the review side, not yours. It is not a duplicate — it closes the set#561 only covered the full-screen views (
I enumerated every Zero new translation burdenThe part I liked most. Rather than adding a parallel set of strings, this reuses #561's:
So no new keys, no new translation work, and the copy is identical to the full-screen dialog — the compact affordances read the same as the rich one rather than drifting into their own wording. Since this PR predates #561, adapting to reuse its strings was deliberate work on your part. Extracting Optional follow-up, not a blocker
VerificationCI is green, and the i18n audit is the gate that would have caught a missing key, so the string reuse is machine-checked rather than just eyeballed. I have no Xcode or JDK here so I have not built it; this is UI wiring against strings that already exist, so the exposure is a compile error at worst rather than anything behavioural. Happy to merge as-is. |
MARKETING_VERSION/versionName move together per the versioning rule; build numbers increment independently (207->208 Apple, 298->299 Android). The staging workflow reads MARKETING_VERSION for its title and asset names, so main has to carry 9.1.1 for the rolling testing-latest release to be labelled as such — the previous 9.1.1 staging build was cut from a commit that never landed on main. This is also the first compile of the app-target changes merged today (#783/#784/#785, #787, #788, #793, #795, #519/#798, #523, #778), which CI does not cover: app-build.yml and android.yml are disabled and only the Swift packages and the i18n gate run on PRs.
…anbr#517) (ryanbr#523) ryanbr#561 added the end-workout confirmation to the full-screen views only. Three destructive one-tap paths survived: LiveView.swift's .destructive NoopButton called model.endWorkout() immediately, and LiveScreen.kt / WorkoutStart.kt both had a statusCritical Button wired straight to endWorkout(). A stray tap on any of them stopped the session with no way back — the same complaint ryanbr#517 reported. This confirms all three. Counting every endWorkout() call site in the app, there are five: two were already behind ryanbr#561's dialog, these are the other three, so after this no end-workout path in the app is unconfirmed. No new strings on either platform. The iOS literals match ryanbr#561's catalog keys exactly, so SwiftUI's LocalizedStringKey resolves the existing de/es/fr/pt-PT translations, and the Kotlin dialog reuses ryanbr#561's l10n_live_workout_screen_* resources. The copy is therefore identical to the full-screen dialog rather than drifting into its own wording, and there is no new translation burden. Since this PR predates ryanbr#561, reusing its strings was deliberate adaptation by the author. EndWorkoutConfirmationDialog is extracted rather than inlined so the compact cards cannot regress to one-tap termination independently. LiveWorkoutScreen.kt keeps ryanbr#561's inline dialog for now; folding it onto the shared component is a possible follow-up that would leave one implementation instead of two. Note on sequencing: this was filed 16 Jul, 25 hours before ryanbr#561, and sat while the later PR for the same issue merged. The reviewer's miss, not the author's. Not built locally — no Xcode or JDK available — but the i18n audit gate passes, which is what would catch a missing string key. By @kavemang.
MARKETING_VERSION/versionName move together per the versioning rule; build numbers increment independently (207->208 Apple, 298->299 Android). The staging workflow reads MARKETING_VERSION for its title and asset names, so main has to carry 9.1.1 for the rolling testing-latest release to be labelled as such — the previous 9.1.1 staging build was cut from a commit that never landed on main. This is also the first compile of the app-target changes merged today (ryanbr#783/ryanbr#784/ryanbr#785, ryanbr#787, ryanbr#788, ryanbr#793, ryanbr#795, ryanbr#519/ryanbr#798, ryanbr#523, ryanbr#778), which CI does not cover: app-build.yml and android.yml are disabled and only the Swift packages and the i18n gate run on PRs.
What changed
mainUpstream overlap resolved
Current
mainnow contains #561, which added the confirmation to the full-screen Apple and Android workout views. This branch keeps that newer implementation intact and removes its original duplicate full-screen/catalog changes. The PR is now limited to the three compact stop buttons that #561 did not cover.Why
Those compact End workout buttons still called
endWorkout()immediately. An accidental tap could therefore finalize and save an incomplete session even though the full-screen button now asks first.The first tap now opens End this workout? with the same explanation used by the full-screen dialog. Cancel leaves the active workout untouched; only the destructive End workout action finalizes it.
Closes #517.
Validation
ANDROID_HOME=/home/kaveman/Android/Sdk ./gradlew :app:compileFullDebugKotlin— passedpython3 Tools/i18n_audit.py --ci origin/main— passedpython3 -m json.tool Strand/Resources/Localizable.xcstrings— passedgit diff --checkDeviceRegistryDao.setModelimplementations tracked/fixed in Android: unblock testFullDebugUnitTest — implement DeviceRegistryDao.setModel in the three fake DAOs #725