[MBL-19925][Student] NGC Module Extraction and Dashboard Navigation Refactor#3631
[MBL-19925][Student] NGC Module Extraction and Dashboard Navigation Refactor#3631tamaskozmer merged 15 commits intomasterfrom
Conversation
More descriptive name for the navigation handler implementation that uses RouteMatcher for navigation in the classic Student app experience. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
# Conflicts: # apps/student/src/main/java/com/instructure/student/features/dashboard/compose/DashboardScreen.kt
🧪 Unit Test Results✅ 📱 Student App
✅ 🌅 Horizon
✅ 📦 Submodules
📊 Summary
Last updated: Mon, 13 Apr 2026 14:08:47 GMT |
There was a problem hiding this comment.
PR Review Update — Synchronize Push
This push introduces the libs/ngc module (NGC activity, theme, navigation, dashboard, and design system) alongside completing the dashboard refactoring that replaces DashboardRouter with DashboardNavigationHandler.
Summary of Changes
- New
libs/ngcmodule —NGCActivity,NGCTheme,NGCNavigation,NGCDashboardScreen, design system components (NGCIconButton,NGCTypography,NGCColors,DSIconButton,DSTypography,DesignSystem), splash screen + VM - Dashboard refactoring completed —
DashboardBody,DashboardViewModel,DashboardUiStatemoved topandautils; all widgets now emitSharedFlow<DashboardNavigationEvent>instead of calling routers withFragmentActivity - Login flow extended with
Experience.NextGenCanvasbacked byNEXT_GEN_CANVASremote config - New use cases —
LoadSplashDataUseCase,SetupPendoTrackingUseCase;ThemeRepositoryandFeaturesRepositoryadded
Issues Requiring Changes
- Incorrect accessibility content descriptions — cloud-download button announces "Notebook", edit button announces "Inbox" (
strings.xmllines 23–24,NGCDashboardScreen.ktlines 170 & 180) -
forceRefresh = truedefault inLoadSplashDataUseCase— will force-refresh all startup data on every cold launch; should befalse(line 35) - Duplicate
ngc_dashboardTitlestring — defined in bothlibs/ngcandlibs/pandares; will cause aapt2 duplicate resource error (pandares line 2479) - Unreferenced
proguard-rules.txt— not referenced bybuild.gradle.kts, contains unrelated Amazon/AWS rules; should be deleted - Unused
@SerializableonNGCNavigationRoute— string-based routing is used throughout; annotation creates false impression of type-safe navigation (NGCNavigation.ktline 32)
Positive Notes
- The
DashboardNavigationHandler+DashboardNavigationEventpattern is well-structured and testable. The accompanying test updates (switching fromverifytoSharedFlowcollection assertions) are thorough and correct. - Removing
FragmentActivityfrom all widget callback signatures is a clean improvement — widgets are now properly decoupled from the Android framework. RouteMatcherNavigationHandlercorrectly centralises all student-app routing in one place, making future navigation changes easier to track.
There was a problem hiding this comment.
PR Review Update — Synchronize Event
Summary of Changes
This is a large, well-structured PR with two primary goals:
-
Navigation architecture refactor: Replaces the
DashboardRouter+ per-widget*Router/*Behaviorpattern with a unifiedDashboardNavigationHandler+DashboardNavigationEventsealed interface. AllFragmentActivityreferences are removed from widget composables and ViewModels — a clean move toward proper Compose-first design. -
New NGC (Next Gen Canvas) module: Introduces
libs/ngcwith its ownNGCActivity, design system, navigation scaffold, and splash screen, gated behind a newExperience.NextGenCanvasremote-config flag.
The architecture is significantly cleaner after this refactor. Test coverage was maintained and updated throughout.
Issues Requiring Changes
| Severity | Issue |
|---|---|
| 🔴 | Wrong accessibility content descriptions on NGC top bar buttons ("Notebook" for offline content, "Inbox" for customize dashboard) |
| 🔴 | LoadSplashDataUseCase defaults forceRefresh = true with a TODO — will force a network call on every cold start |
| 🟡 | ngc_dashboardTitle string defined in both libs/ngc and libs/pandares — duplicate resource |
| 🟡 | NGCActivity.loginWithToken is public with no @VisibleForTesting guard or debug-build check |
| 🟡 | libs/ngc/proguard-rules.txt is unreferenced and should be deleted |
| 🟡 | NGCComposeNavigationHandler is entirely stub — worth documenting this is intentional in the PR description |
Inline comments mark the specific locations. The accessibility issues are the blocking ones; the rest are improvements.
Positive Changes
FragmentActivitycleanly removed from all widget callbacksDashboardViewModel,DashboardUiState, andDashboardBodymoved topandautilsfor proper reuse by both Student and NGC- Navigation events use typed sealed interfaces — exhaustive
whenprevents missed cases - All unit tests updated to assert on emitted navigation events rather than mock verify calls
- Old per-widget router/behavior classes fully deleted with no dead code left behind
📊 Code Coverage Report
|
There was a problem hiding this comment.
PR Review Summary
This PR implements a significant architectural improvement: replacing callback-based widget routers with an event-driven navigation system (DashboardNavigationEvent sealed interface + DashboardNavigationHandler), and introduces a new ngc library module with NGCActivity as the entry point for Next Generation Canvas.
The direction is sound and the sealed interface hierarchy is well-structured. A few concerns need attention before merging:
Issues Requiring Changes
-
MutableSharedFlowmissing backpressure config — All five widget ViewModels declareMutableSharedFlow<...>()with noreplayorextraBufferCapacity. Events emitted before a collector subscribes are silently dropped. See inline comment onTodoWidgetViewModel.kt. -
SplashViewModelhas 6 critical TODOs — Unread counts, user settings, launch definitions, and to-do counts are all stubbed out. These affect visible UI (nav badges, course appearance) and feature gating. These should be tracked as explicit tickets or addressed before shipping. See inline comment onSplashViewModel.kt. -
DashboardNavigationHandlerhas no safety net for missing implementations — The interface has 6 required methods; a missing handler silently swallows navigation events. A no-op default with debug logging would make gaps detectable. See inline comment onDashboardNavigationEvent.kt. -
Deleted app-layer behaviour tests have no replacement —
StudentCoursesWidgetBehaviorTest(144 lines) andStudentTodoWidgetBehaviorTest(175 lines) are deleted. The Student-app-specific handler that performs actual routing should have equivalent coverage. See inline comment.
Positive Notes
- The sealed interface hierarchy for
DashboardNavigationEventis clean and exhaustive — all widget navigation cases are covered. NGCActivityis correctly markedandroid:exported="false"in the manifest.- Moving
DashboardViewModeltopandautilsimproves reusability across apps. - The switch from
FragmentActivityparameters in callbacks to stateless event objects is the right call for testability.
There was a problem hiding this comment.
We should have test for this
There was a problem hiding this comment.
We should have test for this
Summary
This PR extracts the Next Generation Canvas (NGC) code into a dedicated module (
libs/ngc) and introduces a unified navigation event pattern for dashboard widgets. This architectural change prepares the codebase for the NGC experience while maintaining the classic Student app functionality.Architecture Overview
Module Structure
Key Architectural Decisions
1. NGC Module (
libs/ngc)A standalone library module containing all NGC-specific code:
NGCTypography,NGCColors,NGCIconButton) without Horizon dependenciesNGCComposeNavigationHandlerThe NGC module depends only on
pandautils(NOT on Horizon), allowing independent evolution of the NGC design system.2. Dashboard Navigation Events Pattern
Replaced scattered router interfaces with a unified event-based navigation system:
Implementations:
RouteMatcherNavigationHandler- For classic Student app (uses RouteMatcher/Fragments)NGCComposeNavigationHandler- For NGC experience (uses Compose Navigation)3. Shared Dashboard Components
Moved to
pandautilsfor sharing between Student and NGC:DashboardBody- Main content area with pull-to-refresh and widgetsDashboardViewModel- Widget loading and state managementDashboardUiState- UI state model4. Design System Abstraction
The NGC module includes an abstract design system layer for future NGC vs Legacy Canvas switching:
DSIconButton,DSTypography- Abstract interfacesNGCIconButton,NGCTypography- NGC implementationsDesignSystemenum withNextGenCanvasandLegacyCanvasvaluesBenefits
Test plan:
refs: MBL-19925
affects: Student
release note: Internal architecture improvements for the upcoming NGC experience
🤖 Generated with Claude Code
Co-Authored-By: Claude [email protected]