fix a handful of ui things: empty strain rings, calories/steps on body screen, wrong icons#151
Merged
Conversation
…y screen, wrong icons - the "training load" ring on a workout row could show up as a hollow/ empty circle even for a workout with real data, because of a gap in the noData check: if avg_hr was real but strain itself happened to be null, it fell through to ArcGauge with value: double.nan, and ArcGauge's own contract literally says NaN renders as "muted empty ring". just gate the text-vs-gauge choice on strain==null directly instead of the compound flag that had this blind spot. - removed the "Calories & steps" section from the strain/body detail screen entirely. it used to be nested under training load until an earlier fix split it into its own section (contributor feedback: "why is calories under training load, makes no sense") - but it still didn't really belong on the strain screen at all, calories/steps are energy expenditure not strain/intensity and they're already shown properly on Today + the Steps screen. deleted the now-unused _energyCard along with it. - community links in profile (github/discord/reddit/x) were ALL using OsIcon.activity - a generic pulse icon - as a stand-in for every single one, so they all looked identical. added real per-brand marks via phosphor's logo set (regular weight, not duotone, since a two-tone render doesn't make sense for a plain monochrome brand logo) and wired them in. worth noting: i dropped these enum members earlier during the icon pack migration because a grep for `OsIcon.github` etc turned up zero hits - what i missed was the call sites were already degraded to a generic icon instead of being removed, so the grep looked clean when it wasn't. added them back properly this time. - the recap share card's little logo badge next to the "OpenStrap" wordmark was OsIcon.bodyStrain (a barbell) which has nothing to do with the app's own identity. swapped it for the actual app icon (assets/images/icon.png, same asset + errorBuilder pattern boot_splash.dart already uses for it). - the regenerate button on the ai breakdown/recap screen was also OsIcon.activity, same generic-icon problem. swapped to OsIcon.sync (arrows in a circle) which is what this app already uses everywhere else for "run this again". 611/611 green, analyze clean (same 8 pre-existing baseline issues, nothing new).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThe PR removes calories and steps from strain details, handles missing workout strain values, restores social brand icons, updates icon usage in AI and profile screens, and changes the recap badge to use the bundled app icon. ChangesStrain display updates
Iconography updates
Recap branding
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
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
1 participant
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.
follow-up on #150, which was already merged before this landed on the same branch — my bad, should've checked. this is just the one remaining commit, fresh PR for it.
empty "training load" ring on a workout row that has real data:
noData's check missed the case where avg_hr is real butstrainitself is null, which fell through toArcGauge(value: double.nan)— and ArcGauge's own doc says NaN renders as a "muted empty ring". gated the text-vs-gauge choice onstrain == nulldirectly instead.removed "Calories & steps" from the strain/body detail screen entirely. it used to be nested inside "Training load" until an earlier fix split it out (contributor feedback: "why is calories under training load, makes no sense"), but it still didn't belong on the strain screen at all — that's energy expenditure, not strain/intensity, and it's already shown properly on Today + the Steps screen. deleted the now-dead
_energyCardwith it.community links (GitHub/Discord/Reddit/X) in Profile were all using the same generic
OsIcon.activitypulse icon — added real per-brand marks via phosphor's logo set (flat Regular weight, not duotone — doesn't make sense for a plain monochrome brand mark). note: i'd actually deleted these enum members during the earlier icon-pack migration because grepping forOsIcon.githubturned up nothing — missed that the call site was already degraded to a generic icon instead of removed, so the grep looked clean when it wasn't. added them back properly.recap share card's logo badge was
OsIcon.bodyStrain(a barbell, no relation to the app itself) — swapped for the real app icon asset (assets/images/icon.png, same oneboot_splash.dartalready uses).AI breakdown/recap screen's regenerate button was also
OsIcon.activity— swapped toOsIcon.sync, which is what this app already uses everywhere else for "run this again".611/611 tests green,
flutter analyzeclean (same pre-existing baseline, nothing new).Summary by CodeRabbit
New Features
Bug Fixes