Closed Bug 1038797 Opened 6 years ago Closed 5 years ago

Search activity: Error state when network is not available

Categories

(Firefox for Android Graveyard :: Search Activity, defect, P1)

All
Android
defect

Tracking

(firefox34 disabled, firefox35 verified)

VERIFIED FIXED
Firefox 35
Tracking Status
firefox34 --- disabled
firefox35 --- verified

People

(Reporter: eedens, Assigned: Margaret)

References

Details

(Whiteboard: shovel-ready)

Attachments

(5 files, 2 obsolete files)

What is the behavior for no network connectivity? How should we message this to users?
Flags: needinfo?(alam)
Here's a quick wireframe sketch to start with. http://cl.ly/image/0A081y293O0z

I think we can keep this pretty simple -- inform people of the problem, and give them something actionable to do about it. In this case, provide a link that launches the device's Settings so they can check their wifi / network settings

Eric, we'll still want Anthony to refine the visuals here but maybe this is enough to get something rough standing up in the activity.
Attaching current WIP. I think this page could use a bit more personality and whimsy but I wanted to post something so we had at least a placeholder for it for now.
Flags: needinfo?(alam)
(In reply to Ian Barlow (:ibarlow) from comment #1)
> Here's a quick wireframe sketch to start with.
> http://cl.ly/image/0A081y293O0z
> 
> I think we can keep this pretty simple -- inform people of the problem, and
> give them something actionable to do about it. In this case, provide a link
> that launches the device's Settings so they can check their wifi / network
> settings

This sounds pretty clean, and I like that it's actionable.

The mockup shows a search query; were you envisioning this notification happening in lieu of the search results page? Alternatively, we can alert the user when they first open the search activity, possibly bypassing the frustration of a half-completed search.
Flags: needinfo?(ibarlow)
In the event that the user actually drops from their connection or whatnot while they're receiving suggestions, I thought it would be kinda neat to show a "stuff is working in the background" indicator. 

For this "working" animation, I tried to keep it basic while providing a little feeling of whimsy as well. The motions are designed to be more humanistic as well with regards to bounce and easing so it looks and feels more natural.

Also experimenting with this as a part of a larger effort for loading animations in general and bug 1044262
Priority: -- → P1
Whiteboard: shovel-ready
Flags: needinfo?(ibarlow)
Assignee: nobody → margaret.leibovic
Duplicate of this bug: 1060657
antlam, can I have image resources for the icon in the mock-up?

Also, for the "Tap here to check your network settings link", I'm not sure which settings page to open. We can show wifi settings or wireless/network settings, but it's hard to know which a user would want depending on how they're using their device. We could also just open the root setting page, which shows wifi/network stuff right at the top.

Alternately, we could always just say "Please go check your network settings" without providing a link :)
Flags: needinfo?(alam)
Attached file img_xwifi.zip
(In reply to :Margaret Leibovic from comment #6)
> antlam, can I have image resources for the icon in the mock-up?
> 
> Also, for the "Tap here to check your network settings link", I'm not sure
> which settings page to open. We can show wifi settings or wireless/network
> settings, but it's hard to know which a user would want depending on how
> they're using their device. We could also just open the root setting page,
> which shows wifi/network stuff right at the top.

That works! I'd be open to it but I think we should continue to try to be helpful where we can. If it doesn't work so well in practice, we could dumb it down then. 

Here are the images as well
Flags: needinfo?(alam)
Attached image screenshot (obsolete) —
I used the same layout file that I created for bug 1049206, but changed the text color for the link. I took this color from our fennec styles, but maybe we want a lighter color here.
Attachment #8485645 - Flags: feedback?(alam)
Depends on: 1049206
Attached patch Add network error page (obsolete) — Splinter Review
This builds on top of my patch for bug 1049206, since I wanted to re-use the empty view layout.

I decided to add a boolean flag to ResultsWebViewClient to keep track of the error state in order to avoid flashing the web view when retrying to load the results (at first I was just always showing the web view/hiding the error view in onPageStarted, but that looked bad). This means that when you finally get connectivity back, you'll have to wait a little extra time for the results page to be visible, but I think that's an okay trade-off.
Attachment #8485647 - Flags: review?(bnicholson)
Comment on attachment 8485645 [details]
screenshot

Looks good, could we use #0092DB for the link?

I originally thought we would need this on white since the background for the results page is white but I like the way this looks more.
Attachment #8485645 - Flags: feedback?(alam) → feedback-
Attachment #8485645 - Attachment is obsolete: true
Attachment #8486382 - Flags: feedback?(alam)
Updated to include new link color.
Attachment #8485647 - Attachment is obsolete: true
Attachment #8485647 - Flags: review?(bnicholson)
Attachment #8486384 - Flags: review?(bnicholson)
Comment on attachment 8486382 [details]
screenshot w/ updated link color

awesome!
Attachment #8486382 - Flags: feedback?(alam) → feedback+
Comment on attachment 8486384 [details] [diff] [review]
Add network error page

Review of attachment 8486384 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/search/java/org/mozilla/search/PostSearchFragment.java
@@ +132,5 @@
>              });
>          }
> +
> +        @Override
> +        public void onPageFinished(WebView view, String url) {

Nit: I'd move this after onRecievedError to keep these in order they occur.

@@ +136,5 @@
> +        public void onPageFinished(WebView view, String url) {
> +            // Make sure the error view is hidden if the network error was fixed.
> +            if (errorView != null && !networkError) {
> +                errorView.setVisibility(View.GONE);
> +                webview.setVisibility(View.VISIBLE);

Rather than updating the visibility in two different methods, can you just do it all here? That is:

if (errorView != null) {
    errorView.setVisibility(networkError ? View.VISIBLE : View.GONE);
    webview.setVisibility(networkError ? View.GONE : View.VISIBLE);
}

@@ +159,5 @@
> +                message.setTextColor(getResources().getColor(R.color.network_error_link));
> +                message.setOnClickListener(new View.OnClickListener() {
> +                    @Override
> +                    public void onClick(View v) {
> +                        startActivity(new Intent(Settings.ACTION_SETTINGS));

Shouldn't this be ACTION_WIRELESS_SETTINGS/ACTION_WIFI_SETTINGS? Though according to the docs, these may not exist, so you'd still want to fall back to ACTION_SETTINGS just in case.
Attachment #8486384 - Flags: review?(bnicholson) → review+
(In reply to Brian Nicholson (:bnicholson) from comment #14)

> @@ +136,5 @@
> > +        public void onPageFinished(WebView view, String url) {
> > +            // Make sure the error view is hidden if the network error was fixed.
> > +            if (errorView != null && !networkError) {
> > +                errorView.setVisibility(View.GONE);
> > +                webview.setVisibility(View.VISIBLE);
> 
> Rather than updating the visibility in two different methods, can you just
> do it all here? That is:
> 
> if (errorView != null) {
>     errorView.setVisibility(networkError ? View.VISIBLE : View.GONE);
>     webview.setVisibility(networkError ? View.GONE : View.VISIBLE);
> }

Sounds good. I think I had my patch doing this at one point, but changed my mind for some reason.

> @@ +159,5 @@
> > +                message.setTextColor(getResources().getColor(R.color.network_error_link));
> > +                message.setOnClickListener(new View.OnClickListener() {
> > +                    @Override
> > +                    public void onClick(View v) {
> > +                        startActivity(new Intent(Settings.ACTION_SETTINGS));
> 
> Shouldn't this be ACTION_WIRELESS_SETTINGS/ACTION_WIFI_SETTINGS? Though
> according to the docs, these may not exist, so you'd still want to fall back
> to ACTION_SETTINGS just in case.

This is the issue I brought up in comment 6. Choosing one of those might be confusing, since we don't necessarily know if the user is having a wifi problem or a mobile network problem, and the settings for those two things are in different settings screens. That's why I decided to just go with top-level settings.
https://hg.mozilla.org/mozilla-central/rev/79e600c682d8
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
Verified as fixed in build 35.0a1 (2014-09-15)
Device: Google Nexus 7 (Android 4.4.4)
Status: RESOLVED → VERIFIED
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.