[1795] Add status view about internet connection#1839
[1795] Add status view about internet connection#1839WentuM wants to merge 2 commits intodedis:masterfrom
Conversation
|
|
WentuM
changed the title
WentuM
changed the title
WentuM
changed the title
|
Hey! I can’t review this any soon, but I noticed you did not sign the CLA, which you should do ! 👍🏻 (the comment from CLA Assistant)
|
|
Hi! Please make sure to sign the CLA, add some testing and use |
|
Please @WentuM sign the CLA if you want your PR to be reviewed and merged |
WentuM
force-pushed
the
work-fe2-android-internet_connection
branch
from
d741d90 to
9b0ac6f
Compare
matteosz
left a comment
matteosz
left a comment
There was a problem hiding this comment.
I'd also like not only to test with unity tests the connectivity flag, it would be nice to also test the correct UI behaviour with an integration test (i.e. when connection is down the banner pops up correctly).
Also, please include in the PR description screenshot of this 'no connection' alert.
| import javax.inject.Singleton | ||
|
|
||
| @Singleton | ||
| class ConnectivityRepository @Inject constructor(application: Application) { |
There was a problem hiding this comment.
I don't think there's the need for a connectivity repository, as repositories have a different purpose than this. I'd suggest putting this in an utility class as in the end it's just a static method to which you can pass the application context
| /** This LiveData boolean is used to indicate whether the HomeFragment is displayed */ | ||
| val isHome = MutableLiveData(java.lang.Boolean.TRUE) | ||
|
|
||
| val isInternetConnected = MutableLiveData(java.lang.Boolean.TRUE) |
There was a problem hiding this comment.
Perhaps I'd refactor this to isConnectedToInternet
| viewModel.observeInternetConnection() | ||
|
|
||
| handleTopAppBar() | ||
|
|
||
| observeInternetConnection() | ||
|
|
There was a problem hiding this comment.
I'd avoid to have 2 function calls semantically so close, so you can call that viewModel.observeInternetConnection in the observeInternetConnection method
5 participants
Added a status view to the android version of the app with a subscription to the current internet connection