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

163 - Improve connection errors UX (Webview) #165

Merged
merged 5 commits into from
Mar 25, 2021
Merged

Conversation

mrsarm
Copy link
Contributor

@mrsarm mrsarm commented Mar 19, 2021

Activity + Layout to catch connection errors and give the user the ability to retry after fix the connection, and the ability to leave the application only if it doesn't cause the user to lost data (when user is migrating the app).

Issue: #163

Implementation

A new activity and layout were implemented to catch connection errors and avoid the not so friendly Chrome view error screen. The implementation only covers Webview (and therefore the migration from XWalk to Webview). I tried to use the same implementation in XWalk (though I needed to make some adaptations of how XWalk catch errors) and it worked, but for some weird error it cause the app to fail after restart if the user leaves the app with the error activity, so taking into account that is not a priority and we are going to drop XWalk soon, I left Xwalk error management as it is now.

Improvements

  • Spinner while the app retries the connection.
  • Always show a button "Retry" so the user can retry after recover the connection. Previously the button was not displayed some times as show in the screenshot from the ticket.
  • User can retry many times without problems. Previously retrying with the html button "Retry" many times caused the button to disappear at some point, some times even after the first try.
  • Allow to close the app pressing back in the login workflow. In the previous implementation leaving the app and entering again caused the user to see again the connection error until force the full close of the app from the app manager of Android. Now because pressing back really close the app, the second time is opened the loading starts again.
  • In the migration process the error activity does not allow the user to leave the app because testing that scenario in my device caused some times to lose the data, so instead the user sees a "toast" message saying that it needs to wait until the migration process finish.
  • A button "More info" is provided to see the error code that caused the connection error.

Testing

Login page

Here is a recording with the user trying to load the app and sign-in for the first time. It shows different scenarios: user entering without connection, then retry again without connection. User press back living the app. Then enter again into the app, the app allows to sign-in without force closing the app when the connection is recovered.

Medic Android - No Internet - Login 2

Migration process

In the recording below the app was installed on top of the Xwalk version. It shows how the migration popup is shown and immediately the lack of connection is displayed by the connection error activity. The user then reconnects the WiFi and the process continue without problems.

Medic Android - No Internet - Migration running 2

Below the user tries to leave the app pressing back but the exit is prevented and a "toast" message is displayed asking the user to wait the migration process to finish:

Medic Android - No Internet - Migration running - Wait

Finally, here is the button "More info" showing the error code:

Medic Android - No Internet - More Info

Notes

  • The activity only intercepts connection errors when the app fails to load an URL (login) or reload the current page (migration process, or when the user press "Retry"). Ajax calls from the webapp are not intercepted, and are handled by the CHT webapp itself, nothing change there.
  • When a page fails to load like the login page, the old Chrome error page is visible for a fraction of seconds until is hidden by the connection error activity on top, we cannot avoid that 😞 .
  • A phone with black theme was used for the recording (Android 6). I also tested the features with an Android 11 phone with normal theme and the new activity layout has white background with black text as the mockup from the ticket.
  • To test the migration workflow, check the steps here: 134 - Improve UX of crosswalk to webview migration #162 (comment).

@mrsarm mrsarm force-pushed the 163-imp-conn-errors-ux branch 3 times, most recently from 1b1f390 to aa7091d Compare March 19, 2021 20:13
@mrsarm mrsarm requested a review from garethbowen March 19, 2021 21:21
@mrsarm mrsarm marked this pull request as ready for review March 19, 2021 21:22
Copy link
Contributor

@garethbowen garethbowen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice! A couple of comments inline, but this is an awesome improvement.

A couple of additional questions:

  1. Why add the double back to exit feature? Is it just to block the default behaviour of the back button? I think it'd be cleaner to let a single back press behave normally unless migrating.
  2. I'm concerned that you found a way to lose data and that this is just a patch for a larger issue. For example, does it also happen if I shut down my phone mid-migration, or switch to another app in some other way?

Comment on lines 47 to 52
new Handler().postDelayed(new Runnable() {
@Override public void run() {
// After WAIT_MILLIS clean the back pressed history
backToExitPressedOnce = false;
}
}, WAIT_MILLIS);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of clearing it, why not just store the last date that the back was pressed? Then when it's called again you can check if the date < 2 seconds ago, and if not just update the date to now. This is a bit easier to read and avoids callbacks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, your implementation seems simpler, I just made in this way because I copy & pasted the solution from Stackoverflow 😁

Why add the double back to exit feature? Is it just to block the default behaviour of the back button? I think it'd be cleaner to let a single back press behave normally unless migrating.

I've seen this behavior in Android when the app is in the middle of a process, because the user may react pressing back when an error is shown, but without a real intent to exit the app.

In the case of the login page, the user can enter the app again without losing any state or data, so going to remove the double press behavior for the login, and keep the rejection of leaving the app when the connection error happens while migrating.

@mrsarm mrsarm force-pushed the 163-imp-conn-errors-ux branch from e77c18f to c75ba44 Compare March 23, 2021 12:15
@mrsarm mrsarm requested a review from garethbowen March 23, 2021 19:10
@mrsarm mrsarm force-pushed the 163-imp-conn-errors-ux branch from 30e78dc to d538da3 Compare March 23, 2021 21:27
Copy link
Contributor

@garethbowen garethbowen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! Thanks for the quick turnaround.

Just waiting on a green build and then it's ready for AT.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants