-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Added no internet check #1351
Added no internet check #1351
Conversation
@97balakrishnan since I was clear that I work on same task, why did you send this PR? |
I just created a small function and i wanted to test it. Ignore this if you already implemented it. Thanks. |
I think this can be reused while loading uploads also , i thought this will be useful to you. |
Since you have already started I will review this one, but just remember it next time. Firstly we expect from this PR to display message to the user as you did, So there should be an internet connectivity listener. When connecton is extablished, map should be uploaded again. |
Thanks I'll remember it next time :-) |
Toast.makeText(getApplicationContext(),"No internet connnection ",Toast.LENGTH_LONG).show(); | ||
hideProgressBar(); | ||
return; | ||
} | ||
if (lockNearbyView) { |
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.
We have a showLongToast method in our ViewUtils class, please use that one.
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.
I couldn't find the showLongToast method there's only a showSnackbar method.
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.
Very weird I have merged a PR recently which adds it.
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.
I'll check again
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.
Oh, actually PR removes it, sorry for confusion. You can etiher use snackbar to display network problem, or create a method for Toast in ViewUtils. SnackBar can be better according to em, but your decision
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.
ok thanks for the clarification :-)
I'll use the snackbar.
@@ -495,4 +505,25 @@ public void onLocationChangedSlightly(LatLng latLng) { | |||
public void prepareViewsForSheetPosition(int bottomSheetState) { | |||
// TODO | |||
} | |||
private boolean haveNetworkConnection() { | |||
|
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.
Please move this method to a new class named NetworkUtils as a public static method. So that is can be used on every network check
Yes @97balakrishnan please. |
Ok I'll make the changes 👍 |
if(!haveNetworkConnection()) | ||
{ | ||
Toast.makeText(getApplicationContext(),"No internet connnection ",Toast.LENGTH_LONG).show(); | ||
hideProgressBar(); |
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.
Please add strings to strings.xml file.
@neslihanturan I've added the listener for internet network change. 👍 |
A werid thing happens @97balakrishnan, the rest of the activity isn't visible: |
I noticed it just now. Iam not able to figure out why this is happening. |
I discovered the reason, but it is complicated to explain inline. If you let, I can make the required changes @97balakrishnan |
Yeah sure 👍 |
Is everything else working fine? |
@97balakrishnan I don't know why but couldn't fix the issue on your PR. Besides the bug I shared, there are wrong practices needs to be changed on this PR. So I will close this and create a new one. |
ok , but can you pls tell me the wrong practices so that i can correct them next time? |
Sure @97balakrishnan :) Firstly passing activity to receiver with string seems wrong, you can pass context. Secondly using static reference for an activity may cause memory leaks (you can take a look at weak references). Lastly a better practice to unregister our receiver onPause. Ands thanks for your accessibility and fast responding during our work:) |
Thanks a lot for your tips @neslihanturan :) I'll follow them for sure next time |
Description
Fixes #1329 no internet crash
I added a no internet check (WiFi and mobile data ) on refreshView function.
If there is no internet enabled then a toast is displayed and the progress bar is hidden
Tests performed
Android 5.1.1