[MBL-19634][Student] - Show toast message when click on 'Add Bookmark' button on the Assignment List Page #3450
Conversation
…ent List page in Offline mode. refs: MBL-19634 affects: Student release note: Show toast message in offline mode 'Add Bookmark' button click on Assignment List Page.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Summary
This PR adds an offline check before allowing bookmark creation in the student assignment list, which is a good improvement for user experience. The implementation prevents the dialog from showing when there's no network connection and displays an appropriate message to the user.
Positive aspects:
- Prevents unnecessary dialog display when offline
- Uses existing
APIHelper.hasNetworkConnection()utility - Provides user feedback with an appropriate offline message
- Simple and focused change
Areas for improvement:
-
Architecture concern -
Toastis used directly in the behavior class (line 98), which couples UI presentation logic with business logic. Consider refactoring to return state/results that the Fragment can handle for better testability and separation of concerns. -
Consistency check needed - Verify if other network-dependent operations in the assignments list feature have similar offline checks. This PR only adds it to bookmark creation.
-
Race condition consideration - The network check happens before showing the dialog, but the network could drop between the check and the actual API call. Ensure
BookmarkCreationDialoghandles offline states gracefully during execution.
Testing recommendations:
- Test the bookmark creation flow in offline mode
- Test network dropping between the check and dialog interaction
- Verify the Toast message displays correctly and is accessible
Overall, this is a straightforward improvement that enhances the offline experience. The inline comments provide suggestions for architectural improvements that could be addressed now or in future refactoring.
| val dialog = BookmarkCreationDialog.newInstance(activity, fragment, null) | ||
| dialog?.show(activity.supportFragmentManager, BookmarkCreationDialog::class.java.simpleName) | ||
| } else { | ||
| Toast.makeText(activity, activity.getString(com.instructure.pandautils.R.string.notAvailableOffline), Toast.LENGTH_SHORT).show() |
There was a problem hiding this comment.
Using Toast directly in behavior/business logic classes couples the UI layer with the business logic. Consider:
- Moving this offline check to a higher level (e.g., in the Fragment/ViewModel)
- Or, have the behavior return a result/state that the Fragment can handle (showing Toast or Snackbar)
This would make the code more testable and follow separation of concerns principles.
| AssignmentListMenuOverFlowItem(activity.getString(R.string.addBookmark)) { | ||
| val dialog = BookmarkCreationDialog.newInstance(activity, fragment, null) | ||
| dialog?.show(activity.supportFragmentManager, BookmarkCreationDialog::class.java.simpleName) | ||
| if (APIHelper.hasNetworkConnection()) { |
There was a problem hiding this comment.
The APIHelper.hasNetworkConnection() check is good, but consider:
- This check is only added to the bookmark creation menu item. Are there other actions in this or related classes that should also have offline checks?
- The
BookmarkCreationDialogitself might attempt network operations - does it handle offline states gracefully if the network drops between this check and the actual API call?
For consistency, it may be worth auditing other network-dependent operations in the assignments list feature.
🧪 Unit Test Results✅ 📱 Student App
✅ 🌅 Horizon
✅ 📦 Submodules
📊 Summary
Last updated: Wed, 17 Dec 2025 17:01:55 GMT |
|
Student Install Page |
📊 Code Coverage Report
|
| val dialog = BookmarkCreationDialog.newInstance(activity, fragment, null) | ||
| dialog?.show(activity.supportFragmentManager, BookmarkCreationDialog::class.java.simpleName) | ||
| } else { | ||
| Toast.makeText(activity, activity.getString(com.instructure.pandautils.R.string.notAvailableOffline), Toast.LENGTH_SHORT).show() |
There was a problem hiding this comment.
this could be removed com.instructure.pandautils. and imported
also at other screens we use the No internet connection dialog, and we have an extension for this:
com.instructure.pandautils.utils.ActivityExtensionsKt#withRequireNetwork
What do you think? I think toast is better UX, but it is not consistent with the other screens.
There was a problem hiding this comment.
Removed the package name as we are using the student R (just like in other screens).
However, I would stay with the toast since for 'Add Bookmark' function we are using this approach on other screens as well. You can see it in the corresponding video in the bug ticket.
kdeakinstructure
deleted the
MBL-19634-display-toast-while-click-add-bookmark-on-assignment-list
branch
Show toast message when click on 'Add Bookmark' button on the Assignment List Page in Offline mode (just like we do on other pages).
refs: MBL-19634
affects: Student
release note: Show toast message in offline mode 'Add Bookmark' button click on Assignment List Page.