-
Notifications
You must be signed in to change notification settings - Fork 108
[MBL-19454][Student] Implement dashboard customization feature #3441
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Add the Customize Dashboard screen with the following features: - View list of available widgets with visibility toggles - Move widgets up/down to reorder their position - Brand-colored controls (switch and enabled buttons) - Material 3 card design with proper elevation - Support for widget-specific settings (infrastructure) - Loading, error, and empty states - Dark mode support with previews 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Add item animation when widgets change position - Move display name resolution to ViewModel with Resources injection - Add displayName field to WidgetItem UI state - Add "Dashboard widgets" title above the list - Use CanvasSwitch for brand-colored visibility toggle - Update preview with proper display name - Remove unused imports 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Moved callbacks from screen parameters to UI state following CourseInvitationsWidget pattern. Added feature flag toggle for "New Mobile Dashboard" at the top of the customize dashboard screen. The toggle reads and updates the DASHBOARD_REDESIGN remote config parameter. - Add onMoveUp, onMoveDown, onToggleVisibility, onToggleDashboardRedesign callbacks to CustomizeDashboardUiState - Initialize callbacks in ViewModel with method references - Update Screen to use callbacks from uiState instead of parameters - Inject RemoteConfigUtils and RemoteConfigPrefs in ViewModel and ApplicationModule - Add loadDashboardRedesignFlag to load initial feature flag state - Create FeatureFlagToggle composable with CanvasSwitch component - Add "new_mobile_dashboard" string resource 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
…shboard redesign feature flag Add confirmation dialog when toggling the dashboard redesign feature flag OFF, requiring app restart. After confirmation, display survey dialog to collect user feedback on why they switched back. Both dialogs trigger app restart using ProcessPhoenix after completion. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
- Use flatMapLatest to cancel old flows when metadata changes - Extract combineWidgetsWithConfigs helper for better readability - Combine metadata and config flows to prevent UI from updating twice - Fixes issue where visibility/position changes weren't reflected 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…dget settings - Create ColorPicker composable in pandautils with expand/collapse animation - Add dark mode support with ThemedColor display while persisting light colors - Fix color bleeding artifacts using nested Box approach for borders - Add Int and Number type support to UpdateWidgetSettingUseCase - Integrate color picker into CustomizeDashboardScreen with type conversion - Add backgroundColor setting to WelcomeConfig for testing - Use spring animation for smooth expand/collapse transitions 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Convert ColorCircle to use Material Card for proper elevation support - Add 2dp default elevation and 6dp pressed elevation to color circles - Add 0.5dp border to color circles for better visibility - Fix black color alpha channel in CustomizeDashboardScreen - Simplify preview circle in collapsed state 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Replace AnimatedVisibility with animateContentSize for better LazyColumn sync - Use spring animation with StiffnessMedium and DampingRatioNoBouncy - Simplify expand/collapse logic with if statement instead of AnimatedVisibility 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…mization and fix empty metadata bug This commit adds comprehensive unit test coverage for the dashboard customization feature: - CustomizeDashboardViewModelTest (16 tests): Tests initial state, widget loading, filtering, sorting, move operations, visibility toggling, settings updates, dashboard redesign flag, and error handling - WidgetConfigDataRepositoryTest (6 tests): Tests observing, getting, and saving widget configurations - WidgetMetadataRepositoryTest (7 tests): Tests metadata operations including CRUD and position swapping - ObserveWidgetConfigUseCaseTest (6 tests): Tests config observation with various scenarios - SwapWidgetPositionsUseCaseTest (2 tests): Tests widget position swapping - UpdateWidgetSettingUseCaseTest (7 tests): Tests updating widget settings for all data types - UpdateWidgetVisibilityUseCaseTest (2 tests): Tests toggling widget visibility Bug fix in CustomizeDashboardViewModel: - Fixed issue where empty metadata list would not emit in flow, causing loading state to remain true - Added flowOf(emptyList()) handling when metadata list is empty Total: 46 unit tests covering all use cases, repositories, and ViewModels introduced in this feature. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Added comprehensive instrumentation tests for CustomizeDashboardScreen with 16 test cases covering: - Loading, error, and empty states - Widget list display and settings - Move up/down functionality - Visibility toggle - Dashboard redesign toggle - Confirmation and survey dialogs Added test tags to CustomizeDashboardScreen for all interactive elements to enable UI testing. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dashboard Customization Feature Review
This PR adds a comprehensive dashboard customization feature that allows users to reorder widgets, toggle visibility, and manage the new dashboard redesign. Overall, the implementation is well-structured with good use of Jetpack Compose and modern Android architecture patterns.
Critical Issues Found
-
GlobalScope usage (CustomizeDashboardScreen.kt:84) - Using
GlobalScope.launchcreates unmanaged coroutines that can cause memory leaks and crashes. UseviewModelScopeorlifecycleScopeinstead. -
Remote Config override (CustomizeDashboardViewModel.kt:176) - Writing to
RemoteConfigPrefsdefeats the purpose of remote configuration. Remote configs should be read-only on the client. Create separate user preferences storage instead. -
Empty catch block (CustomizeDashboardScreen.kt:551) - Silent exception swallowing makes debugging impossible. At minimum, log the exception or show a user message.
-
ProcessPhoenix restart (StudentDashboardRouter.kt:76) - Using
triggerRebirth()is a heavy operation that terminates the entire app process. Consider lighter alternatives like navigation-based refreshes, or at minimum add clear user confirmation.
Code Quality Issues
-
State hoisting (DashboardScreen.kt:124) - The
showMenustate should be in the ViewModel to survive configuration changes like screen rotation. -
Inconsistent toggle logic (DashboardScreen.kt:137) - Using
showMenu = !showMenufor dismissal is error-prone. Always set tofalseexplicitly. -
Syntax issue (TeacherDashboardRouter.kt:52) - Extra trailing comma:
makeRoute(), )should bemakeRoute() -
Test timeout (CustomizeDashboardScreenTest.kt:639) - 5000ms timeout in tests can cause flakiness. Use shorter timeouts or Compose's
waitForIdle().
Security Considerations
✅ No SQL injection risks - using Room DAOs with proper parameterization
✅ No XSS vulnerabilities - not rendering user HTML
Performance Considerations
The implementation looks efficient overall:
- ✅ Proper use of Flow for reactive updates
- ✅ LazyColumn for widget lists
- ✅ Appropriate use of
animateItem()for reordering ⚠️ Consider adding loading states for widget reordering operations
Test Coverage
✅ Excellent test coverage with 696 lines of UI tests covering:
- Loading, error, and empty states
- Widget list display and interactions
- Settings display and updates
- Dialog flows
- Move up/down functionality
Suggestion: Add ViewModel unit tests to cover business logic independently of UI.
Positive Highlights
- Clean separation of concerns with Repository → UseCase → ViewModel → UI
- Well-structured Compose UI with reusable components
- Comprehensive test suite
- Good use of Kotlin coroutines and Flow
- Proper dependency injection with Hilt
- Dark theme support
Recommendations
- Fix critical coroutine scope issues immediately - Replace GlobalScope with proper lifecycle-aware scopes
- Rethink remote config modification - Use separate user preference storage
- Add error handling - Don't silently swallow exceptions
- Consider user experience - App restart is disruptive; explore alternatives
- Add ViewModel tests - Test business logic independently
Summary
This is a solid feature implementation with good architecture and test coverage. The main concerns are around coroutine scope management and remote config handling, which should be addressed before merging to prevent potential crashes and unexpected behavior.
📊 Code Coverage Report
|
🧪 Unit Test Results✅ 📱 Student App
✅ 🌅 Horizon
✅ 📦 Submodules
📊 Summary
Last updated: Wed, 17 Dec 2025 15:28:28 GMT |
…board # Conflicts: # apps/student/src/main/java/com/instructure/student/features/dashboard/compose/DashboardScreen.kt # libs/pandares/src/main/res/values/strings.xml # libs/pandautils/src/main/java/com/instructure/pandautils/features/dashboard/widget/WidgetMetadata.kt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
QA Findings:
- In landscape mode, the welcome widget and course invitation widget are not full cover the screen (seems like they have "fixed" width).
Discussed via Slack that this is the expected behaviour according to design. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Textfield cursor color is not correct in the "Switched back?" dialog.
- The scrollable list of widget settings has an unnecessary padding at the top and bottom. It should be content padding so the content could scroll out smoothly.
- Courses widget name is not correct.
- The [] around the student name is not needed.
- There is no "Done" button to apply the changes. They will be applied automatically.
Summary
Implements the Customize Dashboard screen allowing users to:
Note: The welcome widget includes dummy configuration options for testing purposes. These settings can be used to verify the configuration UI and persistence layer.
Test Plan
Testing Notes
./gradle/gradlew -p apps :pandautils:testDebugUnitTest --tests "*CustomizeDashboard*"./gradle/gradlew -p apps :pandautils:connectedDebugAndroidTest --tests "*CustomizeDashboardScreenTest*"refs: MBL-19454
affects: Student
release note: Students can now customize their dashboard by reordering widgets and adjusting visibility settings