Fix/network check on article#2111
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request centralizes UI-related composition locals by moving LocalSnackbarHostState and introducing LocalIsOffline within the core:ui module. It also implements an offline check in NewsFeed and NewsResourceCardList to prevent opening news resources without a connection. Feedback suggests consolidating the duplicated offline notification logic into a reusable helper and switching from Toast to Snackbar to ensure consistency with the app's existing notification patterns.
| if (isOffline) { | ||
| Toast.makeText( | ||
| context, | ||
| context.getString(R.string.core_ui_not_connected), | ||
| Toast.LENGTH_SHORT, | ||
| ).show() | ||
| } else { |
There was a problem hiding this comment.
The logic for checking the offline state and displaying a Toast is duplicated here and in NewsResourceCardList.kt. Consider abstracting this into a reusable helper function (e.g., an extension on Context) within the core:ui module. This would improve maintainability and ensure that any future changes to the offline notification logic only need to be made in one place. Additionally, using a Toast is inconsistent with the Snackbar-based notification pattern used elsewhere in the app; since LocalSnackbarHostState is now available in this module, it should be preferred for a more consistent user experience.
| if (isOffline) { | ||
| Toast.makeText( | ||
| context, | ||
| context.getString(R.string.core_ui_not_connected), | ||
| Toast.LENGTH_SHORT, | ||
| ).show() | ||
| } else { |
There was a problem hiding this comment.
This offline check and Toast notification logic is identical to the implementation in NewsFeed.kt. To adhere to DRY (Don't Repeat Yourself) principles and simplify future updates to the user feedback mechanism, it is recommended to consolidate this logic into a shared utility function within this module.
1 participant
What I have done and why
Added a network connectivity check before launching a Chrome Custom Tab when a user taps on a news article card.
Previously, tapping an article while offline would launch Chrome Custom Tab unconditionally, resulting in Chrome displaying its own network error page and taking the user outside of the app with no app feedback.
The changes add a LocalIsOffline CompositionLocal in core/ui and provides it from NiaApp.kt, where isOffline is already collected. Both NewsFeed.kt and NewsResourceCardList.kt now check this value before launching the Chrome Custom Tab. If the device is offline, a Toast is shown with the message "Can't open, no internet connection".
Additionally, LocalSnackbarHostState was moved from feature/bookmarks/impl to core/ui so that it can be accessed from lower level UI modules without creating an invalid dependency.
This Fixes the issue #2110