Skip to content

Show "Connecting…" banner (or equivalent) when server data is stale #465

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

Closed
gnprice opened this issue Dec 27, 2023 · 7 comments · Fixed by #852
Closed

Show "Connecting…" banner (or equivalent) when server data is stale #465

gnprice opened this issue Dec 27, 2023 · 7 comments · Fixed by #852
Assignees
Labels
a-model Implementing our data model (PerAccountStore, etc.) a-sync Event queue; retry; local echo; races beta feedback Things beta users have specifically asked for

Comments

@gnprice
Copy link
Member

gnprice commented Dec 27, 2023

When we lose our real-time stream of events from the server, either because of network failures or because the server has expired the event queue and we need to request a new one:

we continue to show the latest server data we had, but it may now be stale. We should communicate to the user that that's what's happening.

In the existing zulip-mobile app, we do this with a banner that reads "Connecting…" and appears just under the app bar.

In this app, we could do the same thing, or we could find a different design. One downside of the "Connecting…" banner is that it occupies some vertical space, which means the rest of the content needs to move down to make room (or else let its top portion be obscured by the banner). Nevertheless it might be the best available option.

Even if we use the same design concept, we'll want to pin down the details: the text style, the layout, the banner's background, the specifics of how it animates in and out.

(In zulip-mobile the details are in src/common/LoadingBanner.js; there's no animation. But the conceptually similar "No Internet connection" banner, found in src/boot/OfflineNoticeProvider.js, does have an animation… on iOS only, because React Native's implementation breaks on Android. Here on Flutter, we should have no such trouble implementing whatever animation we choose.)

@gnprice gnprice added the a-model Implementing our data model (PerAccountStore, etc.) label Dec 27, 2023
@gnprice gnprice added this to the Beta 3 milestone Dec 27, 2023
@gnprice gnprice added the beta feedback Things beta users have specifically asked for label Mar 8, 2024
@gnprice gnprice modified the milestones: Beta 3, Beta 2 Mar 8, 2024
@gnprice
Copy link
Member Author

gnprice commented Mar 8, 2024

Bumping this in priority because even after #185 and #184, we're still sometimes showing the symptom of lacking an event queue, even when the device has a fine internet connection:
https://chat.zulip.org/#narrow/stream/48-mobile/topic/Only.20.22all.20messages.22.20updating/near/1754151
(I guess Alex didn't explicitly say the last part, but I definitely had a fine connection when I observed similar symptoms the other day.)

And as Alex said there:

silently not having updates means I cannot trust the app. Having some user feedback seems very important, here.

@chrisbobbe
Copy link
Collaborator

chrisbobbe commented Mar 8, 2024

Maybe something like a toast?

Or a thin, linear indeterminate progress indicator at the bottom of the app bar (LinearProgressIndicator), like we do when logging you in? (It disappears quickly in this video, but it's there.)

RPReplay_Final1709858536.mov

(Huh, the screen capture seems to have included audio, so you can hear the music I was listening to at the time. 😆)

@gnprice
Copy link
Member Author

gnprice commented Mar 8, 2024

a thin, linear indeterminate progress indicator at the bottom of the app bar (LinearProgressIndicator), like we do when logging you in?

Yeah, that sounds like the easiest thing to do that remains pretty clean-looking. We can always revise it to another design later.

(Going with either LinearProgressIndicator, or a toast, for now will mean we spend basically zero effort on implementing the specifics of the UI — so there'll be essentially zero wasted work if we later go with another design.)

I'm about to file another issue (→#555) for a toast with more detailed information, which might be too technical-looking for a general release but is fine for this beta and may help us pin down remaining issues in failing to connect.

@chrisbobbe
Copy link
Collaborator

chrisbobbe commented Mar 8, 2024

A toast might be easier to apply in all the right places than the LinearProgressIndicator solution.

The toast solution requires fewer things of the surrounding UI, I think. To show a toast, you just need to know which account (if any) the foregrounded page belongs to and whether its data is stale. But to show a LinearProgressIndicator, you need to know that plus (if the page is per-account) you need the page to have an app bar. And for a nice experience, you might need to adapt to specific things about the app bar, like what color it is. Also I could imagine a future where some app bars use Material widgets and some don't, and that sounds potentially annoying to manage.

I guess concretely, consider the lightbox, which is a per-account page: a saturated blue loading indicator might look jarring on the gray, semitransparent surface of the lightbox app bar, so we might want to tone that down, which might an annoying obstacle to making cleanly reusable code. Also, the lightbox app bar isn't even always visible, because you can tap it to make it go away. I think something like a toast might be the only thing that really works on the lightbox page, actually. And if we're doing it on that page, we might as well do it everywhere; what do you think?

@gnprice
Copy link
Member Author

gnprice commented Mar 8, 2024

The toast solution requires fewer things of the surrounding UI, I think. To show a toast, you just need to know which account (if any) the foregrounded page belongs to and whether its data is stale. But to show a LinearProgressIndicator, you need to know that plus (if the page is per-account) you need the page to have an app bar. And for a nice experience, you might need to adapt to specific things about the app bar, like what color it is.

True, yeah.

@satyam372

This comment was marked as outdated.

@gnprice

This comment was marked as outdated.

satyam372 added a commit to satyam372/zulip-flutter that referenced this issue May 14, 2024
 The widget, comprising a StatefulWidget (SnackBarPage) and its corresponding state class (SnackBarPageState)

Previously, the absence of a dedicated mechanism to display connection-related messages resulted in users experiencing ambiguity regarding Data Staleness.

By leveraging the initState and didUpdateWidget methods, the SnackBar is displayed post-build and upon isStale updates.

Fixes zulip#465
satyam372 added a commit to satyam372/zulip-flutter that referenced this issue May 15, 2024
Previously, the absence of a dedicated mechanism to display connection-related messages resulted in users experiencing ambiguity regarding Data Staleness.

By leveraging the initState and didUpdateWidget methods, the connecting SnackBar is displayed post-build and upon isStale updates.

Fixes zulip#465
PIG208 added a commit to PIG208/zulip-flutter that referenced this issue Jul 30, 2024
Ideally we may have test to exhaustively ensure that all pages
specific to a single PerAccountStore use ZulipAppBar.

Some pages with `AppBar`s are skipped, such as AboutZulip and
lightboxes.

Fixes zulip#465.

Signed-off-by: Zixuan James Li <[email protected]>
PIG208 added a commit to PIG208/zulip-flutter that referenced this issue Jul 30, 2024
Ideally we may have test to exhaustively ensure that all pages
specific to a single PerAccountStore use ZulipAppBar.

Some pages with `AppBar`s are skipped, such as AboutZulip and
lightboxes.

Fixes zulip#465.

Signed-off-by: Zixuan James Li <[email protected]>
PIG208 added a commit to PIG208/zulip-flutter that referenced this issue Jul 30, 2024
Ideally we may have test to exhaustively ensure that all pages
specific to a single PerAccountStore use ZulipAppBar.

Some pages with `AppBar`s are skipped, such as AboutZulip and
lightboxes.

Fixes zulip#465.

Signed-off-by: Zixuan James Li <[email protected]>
PIG208 added a commit to PIG208/zulip-flutter that referenced this issue Jul 31, 2024
Ideally we may have test to exhaustively ensure that all pages
specific to a single PerAccountStore use ZulipAppBar.

Some pages with `AppBar`s are skipped, such as AboutZulip and
lightboxes.

Fixes zulip#465.

Signed-off-by: Zixuan James Li <[email protected]>
PIG208 added a commit to PIG208/zulip-flutter that referenced this issue Jul 31, 2024
Ideally we may have test to exhaustively ensure that all pages
specific to a single PerAccountStore use ZulipAppBar.

Some pages with `AppBar`s are skipped, such as AboutZulip and
lightboxes.

Fixes zulip#465.

Signed-off-by: Zixuan James Li <[email protected]>
PIG208 added a commit to PIG208/zulip-flutter that referenced this issue Aug 2, 2024
Ideally we may have test to exhaustively ensure that all pages
specific to a single PerAccountStore use ZulipAppBar.

Some pages with `AppBar`s are skipped, such as AboutZulip (no
PerAccountStore access) and lightboxes (progress indicator is occupied
for other purposes, and the AppBar can be hidden).

Fixes zulip#465.

Signed-off-by: Zixuan James Li <[email protected]>
PIG208 added a commit to PIG208/zulip-flutter that referenced this issue Aug 5, 2024
Ideally we may have test to exhaustively ensure that all pages
specific to a single PerAccountStore use ZulipAppBar.

Some pages with `AppBar`s are skipped, such as AboutZulip (no
PerAccountStore access) and lightboxes (progress indicator is occupied
for other purposes, and the AppBar can be hidden).

Fixes zulip#465.

Signed-off-by: Zixuan James Li <[email protected]>
PIG208 added a commit to PIG208/zulip-flutter that referenced this issue Aug 6, 2024
Ideally we may have test to exhaustively ensure that all pages
specific to a single PerAccountStore use ZulipAppBar.

Some pages with `AppBar`s are skipped, such as AboutZulip (no
PerAccountStore access) and lightboxes (progress indicator is occupied
for other purposes, and the AppBar can be hidden).

Fixes zulip#465.

Signed-off-by: Zixuan James Li <[email protected]>
PIG208 added a commit to PIG208/zulip-flutter that referenced this issue Aug 8, 2024
Ideally we may have test to exhaustively ensure that all pages
specific to a single PerAccountStore use ZulipAppBar.

Some pages with `AppBar`s are skipped, such as AboutZulip (no
PerAccountStore access) and lightboxes (progress indicator is occupied
for other purposes, and the AppBar can be hidden).

Fixes zulip#465.

Signed-off-by: Zixuan James Li <[email protected]>
@PIG208 PIG208 self-assigned this Aug 8, 2024
@gnprice gnprice closed this as completed in a9d9ef8 Aug 9, 2024
@gnprice gnprice added the a-sync Event queue; retry; local echo; races label Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-model Implementing our data model (PerAccountStore, etc.) a-sync Event queue; retry; local echo; races beta feedback Things beta users have specifically asked for
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants