From 2c49ce96f740eaaa877878c91de224746ec2f839 Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Sat, 25 Jul 2026 20:20:24 +0530 Subject: [PATCH] fix a handful of ui things: empty strain rings, calories/steps on body 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). --- lib/ui/activity/strain_detail_screen.dart | 48 ++++------------------- lib/ui/ai/ai_breakdown_screen.dart | 6 ++- lib/ui/kit/os_icons.dart | 27 +++++++++++-- lib/ui/profile/profile_screen.dart | 11 ++++-- lib/ui/recap/recap_screen.dart | 14 ++++++- lib/ui/workouts/workouts_screen.dart | 16 +++++--- 6 files changed, 66 insertions(+), 56 deletions(-) diff --git a/lib/ui/activity/strain_detail_screen.dart b/lib/ui/activity/strain_detail_screen.dart index 4dd0ae59..e62be7e3 100644 --- a/lib/ui/activity/strain_detail_screen.dart +++ b/lib/ui/activity/strain_detail_screen.dart @@ -6,7 +6,6 @@ import 'dart:math' as math; import 'package:flutter/material.dart'; import 'package:provider/provider.dart'; -import '../../data/day_label.dart'; import '../../data/local_repository.dart'; import '../../state/app_state.dart'; import '../../theme/theme.dart'; @@ -334,26 +333,17 @@ class _StrainDetailScreenState extends State { List _content() { final load = _map(_data['load']); final fitness = _data['fitness_trend']?.toString(); - final cals = _num(_data['calories']); - // Same steps figure as Today + the Steps screen: finalized day estimate - // + today's in-flight live fold-in, so all three never disagree. - final rawSteps = _num(_data['steps']); - final isToday = widget.date == todayLabel(); - final liveSteps = isToday - ? context.select((a) => a.liveSteps) - : 0; - final steps = (rawSteps == null && liveSteps == 0) - ? null - : (rawSteps?.toDouble() ?? 0) + liveSteps; final effort = _num(_data['effort']); // "Training load" is an intensity/strain concept (ACWR, fitness trend, // effort) — calories/steps are energy expenditure, a different concept - // that used to be nested under this section (contributor feedback: "why - // is calories under training load? that makes no sense"). Split into its - // own "Calories & steps" section below instead. + // that doesn't belong on the strain/body screen at all (already-fixed + // contributor feedback: "why is calories under training load? that makes + // no sense" led to splitting it into its own "Calories & steps" section + // here; the follow-up ask was to drop that section from THIS screen + // entirely — calories/steps live on Today and the dedicated Steps + // screen, which is where a reader actually expects energy-expenditure + // numbers, not the strain/training-load detail view). final hasLoad = load.isNotEmpty || fitness != null || effort != null; - final hasEnergy = - cals != null || _num(_data['calories_total']) != null || steps != null; final drivers = [ for (final dr in _list(_map(_data['drivers'])['strain'])) _map(dr), ].where((dr) => (dr['label']?.toString() ?? '').isNotEmpty).toList(); @@ -365,11 +355,6 @@ class _StrainDetailScreenState extends State { _loadCard(load, fitness, effort), const SizedBox(height: Sp.x4), ], - if (hasEnergy) ...[ - const SectionHeader('Calories & steps'), - _energyCard(cals, steps), - const SizedBox(height: Sp.x4), - ], ..._fitnessSection(), _curveCard(), const SizedBox(height: Sp.x4), @@ -558,25 +543,6 @@ class _StrainDetailScreenState extends State { ); } - /// Energy expenditure — deliberately separate from "Training load" above - /// (calories/steps are energy, not strain/intensity). - Widget _energyCard(num? cals, num? steps) { - final total = _num(_data['calories_total']); - return ProCard( - child: Column( - crossAxisAlignment: CrossAxisAlignment.start, - children: [ - if (cals != null) - DetailRow(label: 'Active calories', value: '${cals.round()} kcal'), - if (total != null) - DetailRow(label: 'Total calories', value: '${total.round()} kcal'), - if (steps != null) - DetailRow(label: 'Steps (est.)', value: '${steps.round()}'), - ], - ), - ); - } - // ── 2. HERO ─────────────────────────────────────────────────────────────────── Widget _hero() { diff --git a/lib/ui/ai/ai_breakdown_screen.dart b/lib/ui/ai/ai_breakdown_screen.dart index 3eae94c2..8e5b6b7e 100644 --- a/lib/ui/ai/ai_breakdown_screen.dart +++ b/lib/ui/ai/ai_breakdown_screen.dart @@ -104,8 +104,12 @@ class _AiBreakdownScreenState extends State { return AppScaffold( title: widget.period.title, actions: [ + // Regenerate/rerun action — was OsIcon.activity (a generic pulse + // glyph with no "refresh/rerun" meaning); OsIcon.sync (arrows in a + // circle) is the icon this app already uses everywhere else for + // "run this again"/refresh, so this button now actually reads as one. if (_phase == _Phase.ready) - RoundIconButton(OsIcon.activity, onTap: _generate), + RoundIconButton(OsIcon.sync, onTap: _generate), ], children: switch (_phase) { _Phase.noKey => _noKey(), diff --git a/lib/ui/kit/os_icons.dart b/lib/ui/kit/os_icons.dart index 5304e0af..13e3b716 100644 --- a/lib/ui/kit/os_icons.dart +++ b/lib/ui/kit/os_icons.dart @@ -40,7 +40,11 @@ import 'package:fluentui_system_icons/fluentui_system_icons.dart' show FluentIco import 'package:hugeicons/hugeicons.dart' show HugeIcons; import 'package:iconsax_flutter/iconsax_flutter.dart' show Iconsax; import 'package:phosphor_flutter/phosphor_flutter.dart' - show PhosphorIconsDuotone, PhosphorIcon, PhosphorDuotoneIconData; + show + PhosphorIconsDuotone, + PhosphorIconsRegular, + PhosphorIcon, + PhosphorDuotoneIconData; import 'package:solar_icons/solar_icons.dart' show SolarIconsBold; import '../../theme/tokens.dart'; @@ -125,9 +129,17 @@ enum OsIcon { logout, server, shield, - // NOTE: `github`/`discord`/`reddit`/`twitter` existed in the old package - // enum but had zero live call sites (they were only referenced from the - // now-deleted dead `Ic` class in kit.dart) — dropped rather than mapped. + // Community/social brand marks (Profile → Community links). These DID + // exist as real call sites all along (`profile_screen.dart`'s `_socials` + // list) — just passing `OsIcon.activity` as a stand-in for every one of + // them, not an actual per-brand glyph. An earlier pass at this migration + // checked for direct `OsIcon.github`-style references, found none, and + // wrongly concluded the concept itself was unused — missed that the call + // sites were already degraded to a generic icon rather than removed. + github, + discord, + reddit, + xTwitter, } /// The pack-specific glyph for each [OsIcon]. `IconData` is the common @@ -231,6 +243,13 @@ const Map _glyphs = { OsIcon.logout: PhosphorIconsDuotone.signOut, OsIcon.server: PhosphorIconsDuotone.database, OsIcon.shield: PhosphorIconsDuotone.shield, + // Real brand marks, not duotone (a two-tone render would misrepresent a + // monochrome brand logo) — Phosphor's flat Regular weight, same "plain + // chrome" register Fluent already occupies elsewhere in this map. + OsIcon.github: PhosphorIconsRegular.githubLogo, + OsIcon.discord: PhosphorIconsRegular.discordLogo, + OsIcon.reddit: PhosphorIconsRegular.redditLogo, + OsIcon.xTwitter: PhosphorIconsRegular.xLogo, }; /// Sensible per-domain default tint, used whenever a call site doesn't pass diff --git a/lib/ui/profile/profile_screen.dart b/lib/ui/profile/profile_screen.dart index 1b69d5e9..b2cbb8d4 100644 --- a/lib/ui/profile/profile_screen.dart +++ b/lib/ui/profile/profile_screen.dart @@ -40,11 +40,14 @@ class ProfileScreen extends StatelessWidget { const ProfileScreen({super.key}); // Community links. Editable here; swap any URL and rebuild — no backend needed. + // Real per-brand marks now (were all OsIcon.activity — a generic pulse + // glyph standing in for every single one, so GitHub/Discord/Reddit/X all + // looked identical in this list). static const List<({String label, OsIcon icon, String url})> _socials = [ - (icon: OsIcon.activity, label: 'GitHub', url: '/OpenStrap'), - (icon: OsIcon.activity, label: 'Discord', url: 'https://discord.gg/dUXds5MWkd'), - (icon: OsIcon.activity, label: 'Reddit', url: 'https://reddit.com/r/openstrap'), - (icon: OsIcon.activity, label: 'X', url: 'https://x.com/OpenStrap'), + (icon: OsIcon.github, label: 'GitHub', url: '/OpenStrap'), + (icon: OsIcon.discord, label: 'Discord', url: 'https://discord.gg/dUXds5MWkd'), + (icon: OsIcon.reddit, label: 'Reddit', url: 'https://reddit.com/r/openstrap'), + (icon: OsIcon.xTwitter, label: 'X', url: 'https://x.com/OpenStrap'), ]; static Future _openUrl(String url) async { diff --git a/lib/ui/recap/recap_screen.dart b/lib/ui/recap/recap_screen.dart index df75d2ad..41c7acf3 100644 --- a/lib/ui/recap/recap_screen.dart +++ b/lib/ui/recap/recap_screen.dart @@ -387,7 +387,19 @@ class RecapShareCard extends StatelessWidget { color: AppColors.accent, borderRadius: BorderRadius.circular(R.chip), ), - child: const AppIcon(OsIcon.bodyStrain, size: 14, color: Colors.white), + // The share-card wordmark badge — this is a brand lockup + // (logo + "OpenStrap" text), so it needs the actual app icon, + // not a random domain glyph (`OsIcon.bodyStrain`, a barbell, + // was left here with no real connection to the app's own + // identity). Same asset + errorBuilder-degrades-safely + // pattern boot_splash.dart already uses for this exact icon. + child: Image.asset( + 'assets/images/icon.png', + width: 14, + height: 14, + errorBuilder: (_, _, _) => + const AppIcon(OsIcon.recap, size: 14, color: Colors.white), + ), ), const SizedBox(width: Sp.x2), Text( diff --git a/lib/ui/workouts/workouts_screen.dart b/lib/ui/workouts/workouts_screen.dart index aab7181f..f3d264f7 100644 --- a/lib/ui/workouts/workouts_screen.dart +++ b/lib/ui/workouts/workouts_screen.dart @@ -855,21 +855,27 @@ class WorkoutFeedCard extends StatelessWidget { ), const SizedBox(width: Sp.x3), if (!live && !detected) - (noData + // `noData` alone missed the case where avg_hr is real but + // strain itself is null (e.g. an old pruned workout with HR + // but no strain recompute) — that fell through to ArcGauge + // with `value: double.nan`, which the gauge's own contract + // renders as a "muted empty ring" (see arc_gauge.dart) — + // exactly the hollow "training load" circle users were + // seeing for workouts with no real strain figure. Gate the + // text-vs-gauge choice on strain==null directly instead. + ((noData || strain == null) ? Text('No data', style: AppText.captionMuted .copyWith(color: tone.fgMuted)) : ArcGauge( - value: strain == null - ? double.nan - : (strain / 21).clamp(0.0, 1.0).toDouble(), + value: (strain / 21).clamp(0.0, 1.0).toDouble(), color: tone.accent, size: 54, stroke: 6, sweepFraction: 0.75, animate: false, center: Text( - strain == null ? '—' : strain.toStringAsFixed(1), + strain.toStringAsFixed(1), style: AppText.metricSm .copyWith(fontSize: 13, color: tone.fg), ),