Connection Status: Show SnackBar on changing network state #316

nitish1211 · 2017-01-21T02:31:45Z

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.

smarx · 2017-01-21T02:31:46Z

Automated message from Dropbox CLA bot

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

kunall17 · 2017-01-21T03:56:33Z

app/src/main/java/com/zulip/android/activities/ZulipActivity.java

+                boolean isConnected = intent.getBooleanExtra(ConnectivityManager.EXTRA_NO_CONNECTIVITY, false);
+                if(isConnected){
+                    //Disable fab when there is no connectivity
+                    fab.setEnabled(false);


What if the chatBox is displayed currently, there should be a check if that's visible close that one as well!

Here I was thinking that the chat box would change to fab on scrolling and once it changes to fab we cant go back to chat box as fab is disabled.

But there can be case where the user is being displayed the chatbox and a network change occurs,
Maybe just disable the asyncSend button also save the state, like if the user network disconnects and they were writing a message so we can show the chatbox again when the network connects, for a better UX

This is how its currently is working @kunall17 Please suggest any changes that need to be added.

The displaying of the snackbar at the bottim of the screen takes the chatbox out of the view and as soon as the network connection is reestablished the text that the user had entered is still present.

Hmmm, something is going wrong mostly the snackbar moves the FAB above to make space for itself and not appear on top of it, can you check why?

Its because we have a set a custom app:layout_behaviour in main.xml for our fab which does not include moving up on snackbar's appearance. The moving up on snackbar's appearance is included in the default behaviour of a fab though this can be fixed by creating a different custom layout behaviour class.

kunall17 · 2017-01-21T03:58:01Z

How does the snackbar looks in the night mode?

kunall17

I haven't tried this out, but does this works on a API 21 (lollipop) device?
AFAIR we have to use a NetworkRequest right?

EDIT: Found this bug on SO http://stackoverflow.com/questions/27144026/how-can-i-receive-a-notification-when-the-device-loses-network-connectivity-in-a

kunall17 · 2017-01-21T03:56:54Z

app/src/main/java/com/zulip/android/activities/ZulipActivity.java

+                    //Disable fab when there is no connectivity
+                    fab.setEnabled(false);
+                    fab.setVisibility(View.INVISIBLE);
+                    Snackbar.make(coordinatorLayout,"No Connection!",Snackbar.LENGTH_INDEFINITE).show();


Use displayed text in strings.xml

kunall17 · 2017-01-21T03:57:03Z

app/src/main/java/com/zulip/android/activities/ZulipActivity.java

+                    //Enabling fab when connection is reestablished
+                    fab.setEnabled(true);
+                    fab.setVisibility(View.VISIBLE);
+                    Snackbar.make(coordinatorLayout,"Connection Established",Snackbar.LENGTH_SHORT).show();


Use displayed text in strings.xml

nitish1211 · 2017-01-21T04:13:42Z

Looks fine

kunall17 · 2017-01-21T04:17:06Z

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

nitish1211 · 2017-01-21T05:44:35Z

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.

ShailyTyagi · 2017-01-21T17:44:43Z

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

kunall17 · 2017-01-24T14:27:17Z

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!

nitish1211 · 2017-01-24T16:11:59Z

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

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 · 2017-01-26T19:42:25Z

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

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.

kunall17 · 2017-01-28T03:57:21Z

@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!

nitish1211 · 2017-01-29T06:40:47Z

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

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

kunall17 · 2017-01-29T09:13:59Z

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

nitish1211 · 2017-01-29T09:37:20Z

@kunall17 Done.

kunall17 · 2017-01-30T11:00:35Z

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 · 2017-01-31T18:16:41Z

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!

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

saketkumar · 2017-02-28T14:05:41Z

app/src/main/java/com/zulip/android/activities/ZulipActivity.java

@@ -1972,6 +1965,18 @@ public boolean onCreateOptionsMenu(Menu menu) {
        return false;
    }

+    @Override
+    public boolean onPrepareOptionsMenu(Menu menu) {
+        Log.d("nitish","status"+networkStatus);


Remove this. @vishwesh3 already told you about this. #316 (comment)

I was using it when debugging. Force of habit 😉 will remove it sorry.

saketkumar · 2017-02-28T14:06:24Z

app/src/main/java/com/zulip/android/activities/ZulipActivity.java

@@ -834,9 +831,6 @@ protected void onActivityResult(int requestCode, int resultCode, Intent data) {
            Intent photoSendIntent = new Intent(this, PhotoSendActivity.class);
            photoSendIntent.putExtra(Intent.EXTRA_TEXT, mCurrentPhotoPath);
            startActivity(photoSendIntent);
-
-            // activity transition animation
-            ActivityTransitionAnim.transition(ZulipActivity.this);


why are you removing this?

saketkumar · 2017-02-28T14:07:41Z

app/src/main/java/com/zulip/android/activities/ZulipActivity.java

@@ -1109,7 +1100,7 @@ public void onClick(View view) {
     */
    private void showSoftKeyboard() {
        InputMethodManager imm = (InputMethodManager) getSystemService(Context.INPUT_METHOD_SERVICE);
-        imm.toggleSoftInput(InputMethodManager.SHOW_IMPLICIT,InputMethodManager.HIDE_IMPLICIT_ONLY);
+        imm.toggleSoftInput(InputMethodManager.SHOW_IMPLICIT, InputMethodManager.HIDE_IMPLICIT_ONLY);


Remove this change.

I think its probably fine to retain code refactor changes. These occur clearly because someone forgot to refactor code and it was merged. I think it'd be reasonable to include such changes in a different commit :).

This is because of formatting of the code.Should I include it in a different commit??

saketkumar · 2017-02-28T14:08:24Z

app/src/main/java/com/zulip/android/activities/ZulipActivity.java

@@ -1818,7 +1810,7 @@ private void narrowPMWith(final Person person) {
        if (!person.getEmail().equals(app.getYou().getEmail()))
            list.add(app.getYou());
        doNarrow(new NarrowFilterPM(list));
-        onNarrowFillSendBoxPrivate(new Person[]{person},false);
+        onNarrowFillSendBoxPrivate(new Person[]{person}, false);


Remove this too.

Same case of code formatting :)

nitish1211 · 2017-03-01T08:19:36Z

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

jainkuniya · 2017-03-01T13:15:53Z

I can't find #316 (comment)

Snackbar overlaps the FAB

nitish1211 · 2017-03-01T14:39:37Z

There is a new Behaviour file been addedRemoveFabOnScroll.java for the Fab which makes sure that the FAB scales out every time the snackbar comes into view.

nitish1211 · 2017-03-01T15:16:29Z

This is a demo of how it works currently. 🙂

saketkumar · 2017-03-02T21:47:33Z

@nitish1211 When narrowed to any stream , i got this even when i was connected to the internet.

Steps to reproduce :

Connect to the internet.
Narrow to the any stream.
Disconnect the internet, and reconnect again.
I waited till 1 min, but the it was showing waiting for network, even through the net was working.

nitish1211 · 2017-03-03T05:50:09Z

@saketkumar95 How long did you leave it disconnected?
Because the time to send connect polls increases the longer you leave it offline thus making it take longer to reconnect.
It increases to 33 seconds on just the 7th failure.
The solution to this would be to add a retry button in the snackbar. Which would allow the user to send the poll request when he pleases, instead of the app sending it after a set time period.
What are your thoughts on this?

Its working fine but takes a lot of time to reconnect:

nitish1211 · 2017-03-03T10:18:25Z

Added a Retry Button and removed the Connection Established SnackBar. I've observed in most of the Google apps that Snackbars are only used to show loss of connectivity and are dismissed when connectivity is reestablished.
@saketkumar95 what are your thoughts on this?

So this is how it currently works now:

saketkumar · 2017-03-05T06:19:32Z

@nitish1211 I don't think disabling the refresh button makes sense. If someone is running on slow internet connection , then they might want to refresh the app, but sometimes on slow connection maybe the refresh button gets disabled. This happens sometimes on slow connections. Instead we can have like when no connection is there then just display a snackbar or toast of network unavailable.
@kunall17 What do you think?

nitish1211 · 2017-03-05T07:36:20Z

That's the reason I've added the RETRY button to the snackbar so that people with slow network connections can hit retry to refresh their feed.

saketkumar · 2017-03-05T07:41:32Z

No need for that. Refresh can do that. I think you should remove retry and enable the refresh again. If network is not available we can have anythink like toast, snackbar , or an alert dialog with network unavailable message. Alert dialog is used in this cases in whatsapp.

nitish1211 · 2017-03-05T08:41:30Z

How about I keep both enabled the refresh and the RETRY button??
Makes it faster for the user to refresh feed. :)

saketkumar · 2017-03-05T08:48:25Z

yeah! That sounds good too. For me it's good to be implemented but lets first confirm with @kunall17 :)

nitish1211 · 2017-03-14T15:44:13Z

@saketkumar95 Just figured out that the refresh button should not be kept Enabled when offline, because on pressing the refresh button everything is reset. The Database is cleared and the the coonncetionState is reset. This causes the offline DB to be wiped out. Therefore I believe that the refresh button should be kept disabled when offline.

zulipbot · 2017-05-09T17:39:56Z

Heads up @nitish1211, we just merged some commits (latest: 68e3463) that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/master branch and resolve your pull request's merge conflicts accordingly.

nitish1211 force-pushed the network branch 2 times, most recently from b204acd to 47521cd Compare January 21, 2017 03:19

kunall17 reviewed Jan 21, 2017

View reviewed changes

kunall17 suggested changes Jan 21, 2017

View reviewed changes

nitish1211 force-pushed the network branch 3 times, most recently from c4b00d3 to 0243a2a Compare January 21, 2017 12:53

nitish1211 force-pushed the network branch 3 times, most recently from e92d3b2 to bf9b221 Compare January 24, 2017 16:02

nitish1211 force-pushed the network branch 2 times, most recently from 95c7c1c to 04c8a0e Compare January 26, 2017 17:37

nitish1211 force-pushed the network branch from 04c8a0e to 1fa3ce3 Compare January 29, 2017 06:32

nitish1211 force-pushed the network branch from 1fa3ce3 to 5d57e88 Compare January 29, 2017 09:32

nitish1211 force-pushed the network branch from f29a45f to af245b1 Compare January 29, 2017 10:38

saketkumar suggested changes Feb 28, 2017

View reviewed changes

saketkumar reviewed Feb 28, 2017

View reviewed changes

nitish1211 force-pushed the network branch 3 times, most recently from 0f79f03 to f241f94 Compare February 28, 2017 20:37

nitish1211 force-pushed the network branch from f241f94 to 6c75880 Compare March 1, 2017 14:39

nitish1211 force-pushed the network branch 2 times, most recently from 2eab021 to 23c47ee Compare March 1, 2017 15:06

nitish1211 force-pushed the network branch from 1513d5e to d3a2c16 Compare March 3, 2017 06:49

nitish1211 added 3 commits March 6, 2017 18:15

Show SnackBar on changing Network State.

2a98b47

Show messages even when Offline.

23160b6

Added retry button to connectivitySnackbar.

8bb7245

nitish1211 force-pushed the network branch from d3a2c16 to 8bb7245 Compare March 6, 2017 15:37

Sam1301 added the needs review label Mar 27, 2017

Connection Status: Show SnackBar on changing network state #316

Are you sure you want to change the base?

Connection Status: Show SnackBar on changing network state #316

Conversation

nitish1211 commented Jan 21, 2017

smarx commented Jan 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nitish1211 Jan 24, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kunall17 commented Jan 21, 2017

kunall17 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nitish1211 commented Jan 21, 2017

kunall17 commented Jan 21, 2017

nitish1211 commented Jan 21, 2017 • edited Loading

ShailyTyagi commented Jan 21, 2017

kunall17 commented Jan 24, 2017

nitish1211 commented Jan 24, 2017 • edited Loading

nitish1211 commented Jan 26, 2017

kunall17 commented Jan 28, 2017

nitish1211 commented Jan 29, 2017 • edited Loading

kunall17 commented Jan 29, 2017

nitish1211 commented Jan 29, 2017

kunall17 commented Jan 30, 2017

kunall17 commented Jan 31, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nitish1211 commented Mar 1, 2017 • edited Loading

jainkuniya commented Mar 1, 2017

nitish1211 commented Mar 1, 2017 • edited Loading

nitish1211 commented Mar 1, 2017

saketkumar commented Mar 2, 2017

nitish1211 commented Mar 3, 2017

nitish1211 commented Mar 3, 2017

saketkumar commented Mar 5, 2017 • edited Loading

nitish1211 commented Mar 5, 2017

saketkumar commented Mar 5, 2017

nitish1211 commented Mar 5, 2017

saketkumar commented Mar 5, 2017

nitish1211 commented Mar 14, 2017

zulipbot commented May 9, 2017

nitish1211 Jan 24, 2017 •

edited

Loading

kunall17 left a comment •

edited

Loading

nitish1211 commented Jan 21, 2017 •

edited

Loading

nitish1211 commented Jan 24, 2017 •

edited

Loading

nitish1211 commented Jan 29, 2017 •

edited

Loading

nitish1211 commented Mar 1, 2017 •

edited

Loading

nitish1211 commented Mar 1, 2017 •

edited

Loading

saketkumar commented Mar 5, 2017 •

edited

Loading