Repository navigation
today: open a session's detail from the Latest Workouts feed - #1701
Merged
Merged
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Requested by @h-nry in #1694. The Today feed listed your recent 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 —
WorkoutDetailSheeton Android,WorkoutDetailViewon iOS. Reusing those instead of building a Today-specific view is what makes this safe: neither can edit or delete, so a tap from Today carries no risk that list does not already carry.@h-nry also offered a fallback of linking to the Workouts page instead. The popup is the better of the two and cost the same, since both detail views were already built and reusable.
Two things that had to be right rather than merely compile
The sheet cannot live inside the section. Today's sections are
LazyColumnitems. Hosting the sheet insideTodayWorkoutsSectionbuilds clean and looks correct, but a disposed item takes an open sheet down with it. The state lives onTodayScreenand the sheet sits with Today's other dialogs — which is whereWorkoutsScreenalready keeps its own. A side benefit: the section stays presentational, reporting a tap and no longer needing the view model at all.Press feedback comes from each platform's own primitive. The first iOS draft used
.buttonStyle(.plain), which gives no feedback — Android would have settled inward on press and iOS would have done nothing.WorkoutsView's rows useLiquidPressStyle, the twin of Android'sliquidPress, so that is what this uses.Parity
WorkoutDetailSheetWorkoutDetailViewliquidPressLiquidPressStyle()Accessibility diverges on purpose: Android gets an
onClickLabelstring in all seven locales, because Compose derives nothing and the Sources row beside it does the same; iOS letsButtonderive its label from the tile content, exactly as the Workouts rows do. Adding an iOS string would mean ten xcstrings locales for no gain.Not addressed here, and pre-existing: Android shows 4 tiles with a "14 days" trailing header, iOS 6 with "N total". Same section, different cap and trailing text. Worth its own issue.
Tests
TodayWorkoutTapTestguards the wiring on both platforms, including the lazy-disposal trap — which compiles cleanly and no other test here would catch.It reads source, so it strips comments before matching. The first version did not, and failed on its own first run: this file's comments name
WorkoutDetailSheetwhile explaining the change, so the assertion matched its own prose rather than the code. Worth knowing for any future source-audit test in this repo, which comments densely.Verified
4,712 tests, 0 failures with
--rerun-tasks --no-build-cache;Tools/i18n_audit.py --ciclean; all sevenstrings.xmlparse with the new key present.The iOS half is app-target Swift, which default CI does not compile — dispatching
app-buildfor it, and I will post the result here before this merges.