feat(ui): modernize shell nav and unify health hub - #99
Conversation
Replace the floating bottom tab bar with a left-edge glass navigation rail for a coach-first layout that keeps the chat input area clear. Merge dashboard today surfaces into the Health tab as a unified hub with tiered sections, sticky jump chips, and collapsible advanced blocks. Polish the coach empty state with a compact readiness chip and streamline Journey with a daily quest teaser. Co-authored-by: Youri Bontekoe <YKDBontekoe@users.noreply.github.com>
|
Warning Review limit reached
Next review available in: 45 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThe app shell now uses a left navigation rail, while TrainingPage becomes a unified health hub with pinned navigation and refreshed health sections. New shared widgets support headers, section jumps, segmented controls, and collapsible content. Character, dashboard, coach chat, and related tests are updated accordingly. ChangesHealth hub redesign
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ShellPage
participant KynosNavRail
participant TrainingPage
participant HealthProviders
User->>KynosNavRail: Select Health
KynosNavRail->>ShellPage: Return selected index
ShellPage->>TrainingPage: Display health hub
TrainingPage->>HealthProviders: Refresh health and dashboard data
TrainingPage-->>User: Render pinned header and health sections
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Co-authored-by: Youri Bontekoe <YKDBontekoe@users.noreply.github.com>
Co-authored-by: Youri Bontekoe <YKDBontekoe@users.noreply.github.com>
Co-authored-by: Youri Bontekoe <YKDBontekoe@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
lib/features/coach_chat/presentation/widgets/message_list.dart (1)
17-18: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueImport
KynosChipfrom the shared widget barrelUse
package:kynos/shared/widgets/widgets.darthere instead of importingkynos_chip.dartdirectly so shared-widget imports stay consistent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/features/coach_chat/presentation/widgets/message_list.dart` around lines 17 - 18, Update the KynosChip import in message_list.dart to use the shared widget barrel package:kynos/shared/widgets/widgets.dart instead of the direct kynos_chip.dart import, while leaving the widget usage unchanged.Source: Coding guidelines
lib/features/training/presentation/pages/training_page.dart (1)
49-49: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftSplit this page into extracted section widgets.
At ~579 lines with a five-section inline
build, this file exceeds the project's file-size limit. Extract the "Today", "This week", "Body signals/Trends", "Recent runs", and "More" sections intopresentation/widgets/to bring the file under the target and simplifybuild.As per coding guidelines: "Keep hand-written files under about 250 lines; split larger UI code into
presentation/widgets/."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/features/training/presentation/pages/training_page.dart` at line 49, Refactor _TrainingPageState.build by extracting the inline Today, This week, Body signals/Trends, Recent runs, and More sections into focused widgets under presentation/widgets/. Pass each widget only the state and callbacks it needs, preserve existing behavior and layout, and leave TrainingPage under the project’s approximately 250-line file-size limit.Source: Coding guidelines
lib/shared/widgets/kynos_section_jump_bar.dart (1)
28-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
Gapfor inter-chip spacing.- if (i > 0) const SizedBox(width: Spacing.xs), + if (i > 0) const Gap(Spacing.xs),Requires adding
import 'package:gap/gap.dart';. As per coding guidelines: "use Gap instead of SizedBox for semantic spacing".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/shared/widgets/kynos_section_jump_bar.dart` at line 28, Replace the inter-chip SizedBox spacing in the section jump bar’s chip-building logic with Gap, add the package:gap/gap.dart import, and preserve the existing Spacing.xs width and conditional placement.Source: Coding guidelines
lib/core/theme/layout.dart (1)
9-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
@Deprecatedhere instead of doc-comment text
///@deprecated`` does not trigger analyzer deprecation warnings, soshellNavBarHeightand `shellBottomPaddingLegacy` remain usable without any migration signal. Replace both with `@Deprecated('...')`.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/core/theme/layout.dart` around lines 9 - 22, Replace the doc-comment deprecation markers on shellNavBarHeight and shellBottomPaddingLegacy with Dart `@Deprecated` annotations containing migration guidance. Keep the existing comments and values unchanged, and ensure both declarations emit analyzer deprecation warnings.test/features/training/training_page_test.dart (1)
7-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winOverride providers with fakes per testing guidelines.
TrainingPagereads from multiple providers (dashboardSummaryProvider,healthSummaryProvider,todayInsightsStateProvider, etc.) but the test uses a bareProviderScopewith no overrides. As per coding guidelines, "widget tests must use ProviderScope with overridden fakes instead of real repositories." The test passes only because it checks static strings, but it is fragile and non-compliant.🧪 Proposed fix: add provider overrides
testWidgets('TrainingPage renders health hub header', (tester) async { await tester.pumpWidget( - const ProviderScope(child: MaterialApp(home: TrainingPage())), + ProviderScope( + overrides: [ + dashboardSummaryProvider.overrideWith(() => FakeDashboardSummaryProvider()), + healthSummaryProvider.overrideWith(() => FakeHealthSummaryProvider()), + todayInsightsStateProvider.overrideWith(() => FakeTodayInsightsProvider()), + // Add other provider overrides as needed + ], + child: MaterialApp(home: TrainingPage()), + ), );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/features/training/training_page_test.dart` around lines 7 - 16, Update the TrainingPage widget test to override its consumed providers, including dashboardSummaryProvider, healthSummaryProvider, and todayInsightsStateProvider, with deterministic fake values or notifier implementations in ProviderScope. Keep the existing header assertions unchanged and avoid allowing real repositories or provider side effects during the test.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/features/character/presentation/pages/character_page.dart`:
- Line 20: Move the DailyQuestTeaser widget from the dashboard presentation
widgets to lib/shared/widgets, export it through the shared widgets barrel, and
update both dashboard and character consumers to import it from the shared
barrel instead of the dashboard feature.
In `@lib/features/training/presentation/pages/training_page.dart`:
- Around line 70-90: Update _refreshHealthHub to await the futures for every
invalidated run/week provider, including healthHistoryProvider(days: 28),
healthHistoryProvider(days: days), recentRunsProvider(days: 365, limit: 60), and
recentRunsProvider(days: 30, limit: 3), so refresh completes only after all
displayed cards reload.
In `@lib/shared/widgets/kynos_segmented_control.dart`:
- Around line 70-91: Wrap the `_Segment` interactive content in `Semantics`,
matching `_RailItem`’s accessibility pattern by exposing `button: true`,
`selected: selected`, and `label: label`. Also ensure the `GestureDetector` uses
an opaque hit-test behavior so the full segment area remains interactive.
---
Nitpick comments:
In `@lib/core/theme/layout.dart`:
- Around line 9-22: Replace the doc-comment deprecation markers on
shellNavBarHeight and shellBottomPaddingLegacy with Dart `@Deprecated` annotations
containing migration guidance. Keep the existing comments and values unchanged,
and ensure both declarations emit analyzer deprecation warnings.
In `@lib/features/coach_chat/presentation/widgets/message_list.dart`:
- Around line 17-18: Update the KynosChip import in message_list.dart to use the
shared widget barrel package:kynos/shared/widgets/widgets.dart instead of the
direct kynos_chip.dart import, while leaving the widget usage unchanged.
In `@lib/features/training/presentation/pages/training_page.dart`:
- Line 49: Refactor _TrainingPageState.build by extracting the inline Today,
This week, Body signals/Trends, Recent runs, and More sections into focused
widgets under presentation/widgets/. Pass each widget only the state and
callbacks it needs, preserve existing behavior and layout, and leave
TrainingPage under the project’s approximately 250-line file-size limit.
In `@lib/shared/widgets/kynos_section_jump_bar.dart`:
- Line 28: Replace the inter-chip SizedBox spacing in the section jump bar’s
chip-building logic with Gap, add the package:gap/gap.dart import, and preserve
the existing Spacing.xs width and conditional placement.
In `@test/features/training/training_page_test.dart`:
- Around line 7-16: Update the TrainingPage widget test to override its consumed
providers, including dashboardSummaryProvider, healthSummaryProvider, and
todayInsightsStateProvider, with deterministic fake values or notifier
implementations in ProviderScope. Keep the existing header assertions unchanged
and avoid allowing real repositories or provider side effects during the test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0ec76666-7ee5-45ac-bdd1-b7e7e9d2d809
📒 Files selected for processing (16)
lib/app/shell_page.dartlib/core/theme/layout.dartlib/features/character/presentation/pages/character_page.dartlib/features/coach_chat/presentation/widgets/message_list.dartlib/features/dashboard/presentation/pages/dashboard_page.dartlib/features/training/presentation/pages/training_page.dartlib/shared/providers/dashboard_summary_provider.dartlib/shared/widgets/kynos_collapsible_section.dartlib/shared/widgets/kynos_nav_rail.dartlib/shared/widgets/kynos_page_header.dartlib/shared/widgets/kynos_section_jump_bar.dartlib/shared/widgets/kynos_segmented_control.dartlib/shared/widgets/widgets.darttest/features/training/training_page_test.darttest/shared/widgets/kynos_nav_rail_test.darttest/widget_test.dart
Move DailyQuestTeaser to shared/widgets to fix cross-feature import. Await all health hub provider futures on pull-to-refresh. Add Semantics to KynosSegmentedControl segments. Co-authored-by: Youri Bontekoe <YKDBontekoe@users.noreply.github.com>
2 participants
Summary
Modernizes KYNOS shell navigation and consolidates health surfaces per the UI modernization plan.
Navigation
KynosNavRail(icon-forward, tooltips, haptics)Health hub
TrainingPageas a unified Health hubKynosSegmentedControlfor chart range/metric pickersCoach & Journey
Design system
KynosPageHeader,KynosCollapsibleSection,KynosSectionJumpBar,KynosSegmentedControl,KynosNavRailLayoutTokens.shellBottomPaddingis now context-aware (safe-area)Validation
flutter analyze— 0 errors (2 info-level directive ordering hints)flutter test— 250 passedflutter build web— succeededFollow-ups (not in this PR)
DashboardPagerouting references in CODEMAPGlassCardon coach messages)Summary by CodeRabbit
New Features
Improvements