zulip-android-legacy icon indicating copy to clipboard operation
zulip-android-legacy copied to clipboard

Connection Status: Show SnackBar on changing network state

Open nitish1211 opened this issue 8 years ago • 42 comments
trafficstars

Fixes #312

I've added the the snackbar which shows up whenever there is change in network state. The snackbar showing offline status stays there till connection is re-established. Once the connection is established the snackbar showing connection established dismisses after a short interval.

@kunall17 @Sam1301 Can you please review this. And I am working on #82 , so will update the PR soon.

nitish1211 avatar Jan 21 '17 02:01 nitish1211

Automated message from Dropbox CLA bot

@nitish1211, it looks like you've already signed the Dropbox CLA. Thanks!

smarx avatar Jan 21 '17 02:01 smarx

How does the snackbar looks in the night mode?

kunall17 avatar Jan 21 '17 03:01 kunall17

Looks fine

screenshot_20170121-093359

nitish1211 avatar Jan 21 '17 04:01 nitish1211

Also unregister the receiver like https://github.com/zulip/zulip-android/blob/master/app/src/main/java/com/zulip/android/activities/ZulipActivity.java#L1998

kunall17 avatar Jan 21 '17 04:01 kunall17

The BroadcastReceiver and IntentFilter filter = new IntentFilter(ConnectivityManager.CONNECTIVITY_ACTION); are working fine, I've tested it on a API 24 device and on API 23 and 19 on the emulator.

nitish1211 avatar Jan 21 '17 05:01 nitish1211

Even when the device is offline the snackbar displays 'connection esatblished' .I ran it on an API level 22 device. 2017_01_21_23_01_55

ShailyTyagi avatar Jan 21 '17 17:01 ShailyTyagi

Also you have only checked if we are connected to wifi, It would be better for checking if the internet is working while connected to wifi!

kunall17 avatar Jan 24 '17 14:01 kunall17

Here I've added a new layout_behaviour file for fab. The fab now animates in and out of view like this

g_20170124_2135569

Also added a line to hide the chat box when network is lost. The BroadcastReceiver has also been unregistered during onPause() and registered onResume(). The BroadcastReceiver is called anytime a change in network connectivity has occurred so there I've placed a check for network connectivity isNetworkAvailable() . @kunall17 any other changes you would suggest?

nitish1211 avatar Jan 24 '17 16:01 nitish1211

I've also disabled the refresh button in the overflow menu when the device goes offline. screenshot_20170127-010301

And I'm working on the offline display of messages #82 . I'll send a separate PR for that. @kunall17 @Sam1301 Can you please review the changes done till now, would really like any suggestions from your side.

nitish1211 avatar Jan 26 '17 19:01 nitish1211

@nitish1211 Its better to include #82 in this PR only as they both are connected and will be easier to debug! For this #82 you just need to play with fetch() method here https://github.com/zulip/zulip-android/blob/master/app/src/main/java/com/zulip/android/activities/MessageListFragment.java#L244 this is how we fetch messages!

kunall17 avatar Jan 28 '17 03:01 kunall17

I've added Offline compatibility now the messages stored offline load into the view. Here's how it looks like now:

g_20170129_1206563

I have used the fetch method by giving a call to onReadyToDisplay() @kunall17 can you please review this.

nitish1211 avatar Jan 29 '17 06:01 nitish1211

@nitish1211 awesome! split the commit which deals with offline messages display!

kunall17 avatar Jan 29 '17 09:01 kunall17

@kunall17 Done.

nitish1211 avatar Jan 29 '17 09:01 nitish1211

The android docs say about the isConnected()

Indicates whether network connectivity exists and it is possible to establish connections and pass data.

I'm still not clear by the docs in a scenario where the wifi is connected by the internet isn't working, what happens in this case?

kunall17 avatar Jan 30 '17 11:01 kunall17

I tried this branch I kept my wifi on but did not start the zulip dev server! And it showed me connection established!

Same goes in the case where my wifi is connected it will show the same messages!

device-2017-01-31-234441 Still overlaps the FAB

Just an suggestion: try to test with the error thrown

java.net.ConnectException: Failed to connect to /10.0.3.2:9991

kunall17 avatar Jan 31 '17 18:01 kunall17

@kunall17 You are right, the isConnected() only checks for an active network but does not check if there is access to the internet. I think the solution would be to send an http request to google.com(or any url just to test the connection). It would raise an Exception if there is no internet connectivity.

For the Fab that was happening because I had tried to combine both the default behaviour(Going out of view when snackbar appears) of the fab and hiding it on scroll into one class which was causing the error. It can be fixed by changing the behaviour of the fab in the ZulipActivity.java whenever there is change in network state.

nitish1211 avatar Jan 31 '17 20:01 nitish1211

@nitish1211 Can you resolve merge conflicts?

yadav-rahul avatar Feb 04 '17 17:02 yadav-rahul

@yadav-rahul Resolved the merge conflicts. Sorry it took so long as I was travelling.

nitish1211 avatar Feb 05 '17 07:02 nitish1211

@kunall17 Ive added the http request that checks whether the internet connection is active and during that duration displays Connecting .... snackbar and changes to Connected or No Connection depending on whether the http request went through.

@kunall17 @Sam1301 @yadav-rahul Please review this do let me know if you can suggest any changes to make it better.

nitish1211 avatar Feb 05 '17 08:02 nitish1211

@vishwesh3 Done.

nitish1211 avatar Feb 05 '17 09:02 nitish1211

@kunall17 @vishwesh3 Resolved the merge conflicts. Please review this.

nitish1211 avatar Feb 06 '17 15:02 nitish1211

Rather than connecting to google.co.in we can use the request's sent to fetch updates (heartbeat) of the app in AsyncGetEvents! And according to there exceptions, timeouts we can display messages and use broadcast when the status has changed, and again fetch a heartbeat and compute the results!

This way we don't have to request google.com as well!

kunall17 avatar Feb 10 '17 04:02 kunall17

@kunall17 Here I've used the zulip's continuous polling to check for network connectivity. It shows offline when the number of failures to connect is equal to 2(which is equal to a timeout of approx 3 seconds).

Connectivity is reestablished when there is a certain data response(when body.getEvents().size() > 0).

Please suggest any changes required.

nitish1211 avatar Feb 13 '17 22:02 nitish1211

  • You should not remove the old Behaviour which was being used, edit the current behaviour i found some good article which might help you achieving this https://www.bignerdranch.com/blog/customizing-coordinatorlayouts-behavior/
  • Format the code

kunall17 avatar Feb 14 '17 18:02 kunall17

@kunall17 I've added the changes requested by you. Please review them and let me know of any changes you would like to propose.

nitish1211 avatar Feb 15 '17 20:02 nitish1211

@nitish1211 I tested it on Android 7 and got crash. Swipe the chatBox to dismiss either in online or offline mode.

java.lang.ClassCastException: com.zulip.android.util.RemoveFabOnScroll cannot be cast to com.zulip.android.util.RemoveViewsOnScroll
                        at com.zulip.android.activities.ZulipActivity.removeChatBox(ZulipActivity.java:268)
                        at com.zulip.android.util.SwipeRemoveLinearLayout.onInterceptTouchEvent(SwipeRemoveLinearLayout.java:46)
                        at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2212)
                        at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:2669)
                        at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2358)
                        at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:2669)
                        at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2358)
                        at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:2669)
                        at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2358)
                        at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:2669)
                        at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2358)
                        at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:2669)
                        at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2358)
                        at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:2669)
                        at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2358)
                        at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:2669)
                        at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2358)
                        at com.android.internal.policy.DecorView.superDispatchTouchEvent(DecorView.java:411)
                        at com.android.internal.policy.PhoneWindow.superDispatchTouchEvent(PhoneWindow.java:1813)
                        at android.app.Activity.dispatchTouchEvent(Activity.java:3127)
                        at android.support.v7.view.WindowCallbackWrapper.dispatchTouchEvent(WindowCallbackWrapper.java:63)
                        at android.support.v7.view.WindowCallbackWrapper.dispatchTouchEvent(WindowCallbackWrapper.java:63)
                        at com.android.internal.policy.DecorView.dispatchTouchEvent(DecorView.java:373)
                        at android.view.View.dispatchPointerEvent(View.java:10200)
                        at android.view.ViewRootImpl$ViewPostImeInputStage.processPointerEvent(ViewRootImpl.java:4518)
                        at android.view.ViewRootImpl$ViewPostImeInputStage.onProcess(ViewRootImpl.java:4349)
                        at android.view.ViewRootImpl$InputStage.deliver(ViewRootImpl.java:3889)
                        at android.view.ViewRootImpl$InputStage.onDeliverToNext(ViewRootImpl.java:3942)
                        at android.view.ViewRootImpl$InputStage.forward(ViewRootImpl.java:3908)
                        at android.view.ViewRootImpl$AsyncInputStage.forward(ViewRootImpl.java:4035)
                        at android.view.ViewRootImpl$InputStage.apply(ViewRootImpl.java:3916)
                        at android.view.ViewRootImpl$AsyncInputStage.apply(ViewRootImpl.java:4092)
                        at android.view.ViewRootImpl$InputStage.deliver(ViewRootImpl.java:3889)
                        at android.view.ViewRootImpl$InputStage.onDeliverToNext(ViewRootImpl.java:3942)
                        at android.view.ViewRootImpl$InputStage.forward(ViewRootImpl.java:3908)
                        at android.view.ViewRootImpl$InputStage.apply(ViewRootImpl.java:3916)
                        at android.view.ViewRootImpl$InputStage.deliver(ViewRootImpl.java:3889)
                        at android.view.ViewRootImpl.deliverInputEvent(ViewRootImpl.java:6305)
                        at android.view.ViewRootImpl.doProcessInputEvents(ViewRootImpl.java:6279)
                        at android.view.ViewRootImpl.enqueueInputEvent(ViewRootImpl.java:6229)
                        at android.view.ViewRootImpl$WindowInputEventReceiver.onInputEvent(ViewRootImpl.java:6408)
                        at android.view.InputEventReceiver.dispatchInputEvent(InputEventReceiver.java:185)
                        at android.os.MessageQueue.nativePollOnce(Native Method)
                        at android.os.MessageQueue.next(MessageQueue.java:323)
                        at android.os.Looper.loop(Looper.java:136)
                        at android.app.ActivityThread.main(ActivityThread.java:6209)
                        at java.lang.reflect.Method.invoke(Native Method)
                        at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:865)
                        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:755)```

saketkumar avatar Feb 17 '17 01:02 saketkumar

@saketkumar95 Resolved the Error.

nitish1211 avatar Feb 20 '17 18:02 nitish1211

@vishwesh3 I've solved the issues stated above :)

nitish1211 avatar Feb 28 '17 11:02 nitish1211

@saketkumar95 @vishwesh3 @kunall17 @Sam1301 Any other changes you guys can suggest ? :smile:

nitish1211 avatar Mar 01 '17 08:03 nitish1211

I can't find https://github.com/zulip/zulip-android/pull/316#issuecomment-274850058

Snackbar overlaps the FAB

148837387481825

jainkuniya avatar Mar 01 '17 13:03 jainkuniya