[MBL-19703][Student] Add Progress Notifications Widget to Dashboard#3508
Merged
hermannakos merged 4 commits intomasterfrom Feb 13, 2026
Merged
[MBL-19703][Student] Add Progress Notifications Widget to Dashboard#3508hermannakos merged 4 commits intomasterfrom
hermannakos merged 4 commits intomasterfrom
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Progress Widget Implementation Review
Great work on implementing the Progress Widget! This is a well-architected feature with excellent test coverage and proper use of modern Android development patterns. Here's my comprehensive review:
✅ Strengths
- Clean Architecture: Excellent separation of concerns with dedicated use cases, proper MVVM pattern, and router abstraction
- Outstanding Test Coverage: 38 test cases across 5 test files covering use cases and ViewModel thoroughly
- Modern Tech Stack: Proper use of Jetpack Compose, Kotlin Flow, Room with Flow-based queries, and Dagger Hilt
- Reactive Design: Real-time updates using Flow for both upload progress and sync status
- Good UX: Visual feedback with colored backgrounds, pagination for multiple items, and smart navigation
📋 Issues Found
I've added inline comments for the following items that should be addressed:
- ObserveUploadsUseCase.kt:47 - Add error logging for invalid UUID parsing (currently fails silently)
- DismissUploadUseCase.kt:35 - Add logging when entity is not found during dismiss
- ProgressViewModel.kt:138 - Remove unused
refresh()method or add TODO explaining future intention - ProgressWidget.kt:132 - Memoize pager state to prevent scroll position resets on recomposition
- ObserveUploadsUseCase.kt:56 - Consider performance testing with 10+ simultaneous uploads (Flow combination overhead)
- ProgressWidget.kt:269 - Consider adding dismiss button to upload error cards for UX consistency with sync errors
🧪 Test Coverage Gap
While use case and ViewModel tests are excellent, consider adding Compose UI tests in androidTest to verify:
- Widget rendering with different states
- User interactions (clicks, dismissals)
- Pagination behavior
- Accessibility
💡 Additional Observations
- Widget Positioning: The progress widget is now positioned first in the default widget list. Ensure this aligns with UX expectations.
- Performance: The current Flow combination approach works well for typical usage, but should be monitored with many concurrent uploads.
Overall Assessment
This is a solid, production-ready implementation with only minor improvements needed. The architecture is clean, tests are comprehensive, and the code follows best practices. Once the logging improvements are added and the pager state is memoized, this will be excellent to merge.
Nice work! 🎉
|
|
||
| private fun clearSnackbar() { | ||
| _uiState.update { it.copy(snackbarMessage = null) } | ||
| } |
There was a problem hiding this comment.
This refresh() method is a no-op. If reactive observation is the intended design (which makes sense for this widget), consider removing this callback entirely to avoid confusion. If you anticipate needing manual refresh in the future, add a TODO comment explaining the intention.
| .padding(horizontal = 16.dp) | ||
| .padding(bottom = 12.dp), | ||
| text = stringResource(R.string.progressWidgetTitle), | ||
| fontSize = 14.sp, |
There was a problem hiding this comment.
The pager state should be memoized to prevent potential scroll position resets on recomposition. Consider using a key parameter:
val pagerState = rememberPagerState(
pageCount = { itemPages.size },
key = itemPages.size // Prevents reset when size doesn't change
)Or better yet, track the current page index in the ViewModel if the pages can change dynamically.
| if (item.state != UploadState.UPLOADING) { | ||
| IconButton( | ||
| onClick = onDismiss, | ||
| modifier = Modifier |
There was a problem hiding this comment.
For consistency with the sync error card (which has a dismiss button), consider adding a dismiss option for failed uploads. This would give users a consistent way to clear error states from the widget.
Contributor
🧪 Unit Test Results✅ 📱 Student App
✅ 🌅 Horizon
✅ 📦 Submodules
📊 Summary
Last updated: Fri, 13 Feb 2026 10:40:54 GMT |
Contributor
|
Student Install Page |
Migrate progress notification functionality from DashboardNotificationsFragment to the new widget system. The widget displays file upload progress and offline sync progress with reactive updates. New components: - ProgressWidget: Compose UI with HorizontalPager for progress cards - ProgressViewModel: Observes uploads and sync progress reactively - Use cases: ObserveUploads, ObserveSync, DismissUpload, DismissSync - StudentProgressWidgetRouter: Navigation for uploads and sync screens Features: - Upload progress cards with colored icon strip and progress bar - Sync progress cards with dark background (licorice/danger) - Click to navigate to submission details or file folder - Dismiss completed/failed uploads and sync errors - Database transactions for safe cleanup operations Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
hermannakos
force-pushed
the
MBL-19703-progress-notification-widget
branch
from
73f1f69 to
28cf697
Compare
Contributor
📊 Code Coverage Report
|
kdeakinstructure
requested changes
Feb 8, 2026
Contributor
There was a problem hiding this comment.
- There is no Dashboard notification when uploading a file assignment submission
- Also, in landscape mode, the notifications (and also the course invitations and welcome widget) is not "responsible", they should fill the width of the landscape screen just like the other widgets (e.g. Courses, Groups, Forecast).
Screen_recording_20260208_154509.mp4
- Updated Progress Widget design based on Figma: - Upload notifications: dark card (in-progress), green card (success), red card (error) - Sync notifications: dark card (in-progress/queued), red card (error) - Changed from HorizontalPager to grid layout - Removed "Progress" title from widget - Resolved merge conflicts with master: - Added PROGRESS and CONFERENCES/TODO widget constants - Updated widget positions (PROGRESS at 0, CONFERENCES at 1) - Added all new widget router bindings Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
tamaskozmer
requested changes
Feb 12, 2026
tamaskozmer
approved these changes
Feb 13, 2026
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
3 participants
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.
Test plan:
refs: MBL-19703
affects: Student
release note: Students can now see file upload and offline sync progress directly on the Dashboard