Skip to content
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

Show user if the app is offline #141

Closed
1 task done
bmarty opened this issue Mar 6, 2023 · 31 comments
Closed
1 task done

Show user if the app is offline #141

bmarty opened this issue Mar 6, 2023 · 31 comments
Labels
A-Offline T-User Story Team: QA X-Needs-Signoff Stories and Epics which are ready for review by product, design and QA

Comments

@bmarty
Copy link
Member

bmarty commented Mar 6, 2023

These are the different states we have on Element Android:

image

  • loading: which is displayed when starting the app, to indicate that a sync is in progress. This is not displayed for the further sync request.
  • network error, when a sync is not possible
  • airplane mode, when a sync is not possible and the device detects that airplane mode is on.

Also this indicator is displayed in the following screens, to let the user know it the data are not up to date / cannot be up to date:

  • The room list screen
  • The timeline screen

This is EA behavior, we will first need to decide what we want to do on EAx. Also the Rust SDK must provide an API to be able to know the sync state.

Checks

Preview Give feedback
  • Design sign-off.
  • QA sign-off.
  • Product sign-off.
@manuroe
Copy link
Member

manuroe commented Mar 14, 2023

@amshakal I added the X-Needs-Design label to revisit the issue to describe what we want on EAX. I am pretty sure we do not want to display a sync status banner.

FTR, we have this offline banner on EIX but I failed to find the associated figma:

IMG_5734

@amshakal
Copy link

For android, we have been using the standard material snack bar. The only limitation is that only one can b e displayed at a time so it might be worth discussing if there will be scenarios in EX when more than one status needs to be shown.
Here is an example:
Screenshot 2023-03-16 at 7 02 58 pm

@amshakal
Copy link

amshakal commented Mar 16, 2023

Benoit,
For loading, can we use the native spinner for material? https://m3.material.io/components/progress-indicators/guidelines

And for network and airplane mode, can we combine them into one and call it - you are offline or something like that? Then we can have one snackbar at the bottom

And hopefully it will all feel very native.

@manuroe
Copy link
Member

manuroe commented Mar 16, 2023

@amshakal, I made a mistake in my previous comment. I forgot the not word. I edited this way:
I am pretty sure we do not want to display a sync status banner.

Sorry for the bad guidance.

@jmartinesp
Copy link
Member

@amshakal I don't think I'd use a snackbar for that, for a few of reasons:

  1. There could be several snackbars at the same time (although not currently), and they would either cancel the previous one or overlap, which would look quite terrible.
  2. If I had no network connection, I'd expect to see some 'banner' or 'chip' or some kind of indicator at the top, I wouldn't look for that at the bottom of the screen (where the snackbar will actually be).
  3. Having a permanent snackbar could make the items underneath impossible to see/use. With a room list of 6 elements, the 6th room is completely overlayed. In the chat screen, maybe you'd miss the latest message in the room.

There is no official component for an offline state, each app displays that as they want. I'd vote to use something similar to iOS (I don't think it looks that alien on Android, it looks like a 'chip' component), but as that might also overlay with some other components (search bar in room list, room name in chat screen, etc.) maybe it's better to add some small 'full width' banner like we had in the previous app that moves the content a bit to the bottom/resizes it? Something like:

| Status Bar |
|------------|
|   Offline  |
|------------|
|   Content  |

I am pretty sure we do not want to display a sync status banner.

That makes things a lot easier, as we wouldn't have to rely on the Rust SDK.

@jmartinesp
Copy link
Member

Actually, there's also the option of displaying it similar to the iOS indicator or a banner below the top app bar and above the actual content of the screen (sorry for my terrible screen editing skills 😅) :

Chip Banner
offline-chip offline-banner

@amshakal
Copy link

amshakal commented Mar 17, 2023

Oh interesting. I was going by google and material's recommendation. Eg. in gmail or spotify, when you don't have internet connection, you get a little snackbar at the bottom to indicate that. But yes, guidelines do say that you can only have one at a time and that the dismiss on their own usually. I was wondering how many sceanios would we have in which there would be more than one update to give?

Would it be better to use snackbar for quick status updates which get dismissed on their own. And top ones for static updates?

I am not opposed to having it on top like iOS. Can explore a few options.

@jmartinesp
Copy link
Member

I haven't tried Spotify, but on Gmail the snackbar is not permanent and appears as a response to a user initiated action like reloading your inbox. If we want it to work in a similar way, that's a possibility, but if you want to display it permanently it's probably not a good option.

@amshakal
Copy link

Ah you are right! I think that makes sense. Shall we set the rule to:

Banner -> Permanant things
Snackbar -> Task completion/status updates etc

@amshakal
Copy link

Looking at the screenshot shared above by Benoit from the legacy app - I think that we don't need both - airplane mode and no internet connection. For all things offline, we can have one banner.

@bmarty
Copy link
Member Author

bmarty commented Mar 17, 2023

I think it's nice to help the user to understand why the server is not reachable. But I agree it can be a simple unique banner.

The banner can even be clickable to troubleshoot the connection? In our mobile and multi server environment it can help many people.

@jmartinesp
Copy link
Member

Have we agreed on some design for this? If not, I can start working on integrating the connectivity check logic and once we have the designs I can implement them on the needed screens.

@amshakal
Copy link

Yeah, I need confirmation from the design systems team on the designs. But happy for it to be a banner. I am also wondering why Manu was against an iOS style banner, like he shared above.

@manuroe
Copy link
Member

manuroe commented Mar 21, 2023

I am fine with banners. My main concern about them is that we do not want a banner saying "Syncing..." but we want one saying "Offline".

@jmartinesp
Copy link
Member

@manuroe just to confirm, do we want to check it the device has an internet connection or if it can connect to the home server? Depending on the answer we can just use Android's ConnectivityManager or we'll have to implement some custom code or port them from EA.

@manuroe
Copy link
Member

manuroe commented Mar 21, 2023

EIX is only checking the internet connectivity using system methods. I guess we can have the same for Android.

@janogarcia
Copy link

@amshakal Agree on using a banner (somewhere on the top app bar) for permanent states and a snackbar for temporary notifications. Also agree on using a single message for offline / network connectivity issues. I've shared more details on the related internal thread, some examples, and covered some details on how we could improve on the legacy banner.

@bmarty
Copy link
Member Author

bmarty commented Mar 21, 2023

@jmartinesp we should avoid using system method to detect network or not. It's not reliable enough. In the past we had some trouble with that in particular with air-gapped environment, server behind a proxy, network switch, private Wi-Fi with local homerver, etc. What is important (and what we are relying on currently on EA) is the fact that the homeserver is reachable. Even when not reachable ("offline"), when the app is on foreground, we check if the server can be reach on a regular basis (every 10s IIRC).

@jmartinesp
Copy link
Member

@jmartinesp we should avoid using system method to detect network or not. It's not reliable enough. In the past we had some trouble with that in particular with air-gapped environment, server behind a proxy, network switch, private Wi-Fi with local homerver, etc. What is important (and what we are relying on currently on EA) is the fact that the homeserver is reachable. Even when not reachable ("offline"), when the app is on foreground, we check if the server can be reach on a regular basis (every 10s IIRC).

I feared as much, that's why I asked Manu above, but it seems like there are conflicting views on this. Maybe we can use the NetworkMonitor only to detect if there is WiFi or Mobile network and then have a background coroutine periodically pingin the homeserver?

@bmarty
Copy link
Member Author

bmarty commented Mar 21, 2023

I'd say it's up to the Rust SDK to tell if the server is reachable or not.

@jmartinesp
Copy link
Member

I'd say it's up to the Rust SDK to tell if the server is reachable or not.

That would be ideal, but I don't think there's something like that implemented right now.

@jmartinesp
Copy link
Member

Are there any news about this?

  • Can we confirm if we're sticking with only checking internet connection (which is easy, but can be troublesome if proxies are involved) or we want to do something in the Rust SDK side that's exposed to all platforms?
  • Do we have a design proposal yet?

@amshakal
Copy link

Here is the figma file

Going with the chip approach and combining all the states in one. Similar to iOS

@janogarcia
Copy link

janogarcia commented Mar 28, 2023

@amshakal Even if we go that way for now, I'd still recommend that we rethink / iterate on this. Given that it's a permanent state, it can potentially block the information and actions displayed underneath on the top app bar / navigation bar.

For reference, all the examples I shared in the "Notification · Top" section integrates the offline status message in a way that doesn't obfuscate contextual information or block interaction:

https://www.figma.com/file/dVRbSAZ25fNNEPonJjDEH4/Offline-State?node-id=2-76&t=1QC67nBo85N0cLgw-0

@jmartinesp
Copy link
Member

@callumu @kittykat @VolkerJunginger this is ready to be tested, it should be included in the latest nightly.

@callumu
Copy link

callumu commented Apr 19, 2023

@callumu @kittykat @VolkerJunginger this is ready to be tested, it should be included in the latest nightly.

I've had a look and I have a few comments:

  1. The 'Offline' text weight should be Medium not Regular
  2. The offline indicator doesn't show on some pages such as Settings and Create a Room flow - should this show on all pages?

@jmartinesp
Copy link
Member

I've had a look and I have a few comments:

  1. The 'Offline' text weight should be Medium not Regular
  2. The offline indicator doesn't show on some pages such as Settings and Create a Room flow - should this show on all pages?

I'll take a look at the font issue.

About the indicator being displayed only in the room list and chat screens, I followed this from the description of the issue:

Also this indicator is displayed in the following screens, to let the user know it the data are not up to date / cannot be up to date:

  • The room list screen
  • The timeline screen

I just now realised there were also designs for the settings and room details screens. If it needs to be displayed in almost all screens, we might need to re-work it a bit to make it more automatic.

@callumu
Copy link

callumu commented Apr 19, 2023

I feel like it would be useful to have it on all screens, but if that's what was in the issue description then I'm happy to stick to those two pages for now.

@kittykat kittykat assigned ghost May 9, 2023
@kittykat kittykat added signoff X-Needs-Signoff Stories and Epics which are ready for review by product, design and QA and removed signoff labels May 26, 2023
@ghost
Copy link

ghost commented Jul 5, 2023

I was doing some basic testing of this.

On iOS when I switched on airplane mode I immediately saw an OFFLINE pill at the top of the UI, however after about a minute the app started to try syncing. It's been sitting there with a spinning Syncing pill for a few minutes now.

After a few minutes of that I tapped on a room and it displayed an error at the top saying it failed to load messages with an X button on it, however I cannot dismiss this message.
IMG_3826

When I turned airplane mode off the connection was restored and the message disappeared but now I had a large gap between two of the rooms in the room list view, this gap disappeared after several seconds.
Photo 2023-07-05 04 17 43 PM

On Android I see a wifi logo with a line through it and the word Offline at the top of the screen on the room list and timeline views. Switching airplane mode off, the connection is restored and the Offline notification disappears. The two screenshotted issues above on iOS are not present on Android.

Please advise if these need separate issues filed. Leaving this as QA in progress for now and holding off on giving sign-off for the time being.

@manuroe
Copy link
Member

manuroe commented Jul 5, 2023

@ashughes1 We need separate issues. For iOS, the last one is known and tracked here. Others are nice catch 👍 .

@ghost
Copy link

ghost commented Jul 5, 2023

@ashughes1 We need separate issues. For iOS, the last one is known and tracked here. Others are nice catch +1 .

Done.

See element-hq/element-x-ios#1256 and element-hq/element-x-ios#1257

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Offline T-User Story Team: QA X-Needs-Signoff Stories and Epics which are ready for review by product, design and QA
Projects
None yet
Development

No branches or pull requests

7 participants