ResourceStatusOverlay displays error even when cached data is available about siesta

[pcantrell]

Yes, this is something I’d wondered about, but I wanted to make sure somebody was actually using ResourceStatusOverlay before addressing it. (That question is now answered!)

I don’t think there’s one behavior that’s right for everyone. Certainly for a view that polls in the background, what you want makes sense. If a load is user-initiated, prioritizing the error might make sense. Depending on the UI, it might be better not to even show the loading indicator if data is present.

All that is complicated by the fact that you can use your own layout with the class — and if, for example, the error view is a little unobtrusive thing at the top, then it might make sense to show it even when polling in the background. Twitter & Slack both do something along those lines, IIRC.

I’d like to solve this with something very simple, general, and flexible. My vague thought was that there are three things the view can show, namely data, error, and loading, and you should be able to prioritize them in a list. Something like this:

overlayView.displayPriority = [.Loading, .Error, .Data]  // What it does now

overlayView.displayPriority = [.Data, .Loading, .Error]  // What I think you want?

overlayView.displayPriority = [.Data, .Error, .Loading]  // No spinner except on first request

overlayView.displayPriority = [.Data, .Error]  // Maybe you never want a spinner

I’m fuzzy on the details. I’ve sketched it out like it’s an enum, but maybe those are little strategy structs instead, where the first one in the list to say “I’m it” wins, and you can provide your own.

Thoughts on that — as a user, as someone looking at the code?

[annicaburns]

I like your proposed solution a lot. I had not yet considered the pull-to-refresh use case, but would have run into it sooner rather than later!

I'm partial to the "enum" version of your solution because it's so simple.

However, I am struggling with how to handle the fact that we will want the ResourceStatusOverlay to act DIFFERENTLY if the load is user-initiated than if it's an automatic background refresh. We will want to display an unobtrusive error notice to the user after a pull-to-refresh, while continuing to make the cached data available. However, if an error is returned from an automatic background refresh we won't notify the user that a request has even been made. It's not clear to me if your proposed solution would make possible a ResourceStatusOverlay that acted differently depending on the type of request (user-initiated or not).

[pcantrell]

Good point about pull to refresh. Hmmm.

I think if you wanted one request to be specially privileged over others, you’d use request hooks instead of the observer mechanism to manage the UI for just that request. Something like this:

var resource: Resource
var overlay: ResourceStatusOverlay
var manualRefresh: Request?
var manualRefreshView: UIWhateverView

func doSetupSortsOfThings()
    {
    ...
    resource.addObserver(self)
            .addObserver(overlay)
    ...
    }

func triggerManualRefresh()
    {
    guard manualRefresh == nil else
        { return }

    manualRefreshView.hidden = false
    manualRefresh = resource.load()
        .completion
            {
            _ in
            manualRefresh = nil
            manualRefreshView.hidden = true
            }
    }

Note that triggerManualRefresh() doesn’t have to worry about actually doing anything with the response data — we’re already observing it. It just ties a special “request in progress” indicator to one specific request.

Another related question is how you might show a small, unobtrusive error if a background refresh failed and cached data is unavailable, but the standard full-size overlay if there is no data. In that case, I think you’d have two separate observer views: one that’s the standard overlay with [.Data, .Loading, .Error], and a second custom view that shows the error only if data is also present.

In general, I imagine ResourceStatusOverlay being layered with other things in one UI — not managing the whole screen at once. That eases the burden somewhat.

Talking through this makes me think that the enum probably is sufficient, and little customizable strategy structs is overkill. Other use cases to consider?

[pcantrell]

I took a first crack at this, pretty much following this discussion. It’s on master. For your case, I think you want:

statusOverlay.displayPriority = [.Retrying, .AnyData, .Error, .Loading]

The .Retrying condition refers specifically to a manual refresh initiated by the user tapping the retry button in the status overlay’s error view, whereas .Loading means any load request initiated by anyone.

The “any” in .AnyData means “any of the possibly multiple resources this overlay is observing.” There’s also an .AllData.

Give it a try when you get a chance, and let me know how it works and what it needs.

[annicaburns]

Many Thanks, Paul! I pulled your new code and set the displayPriority property on all of my ResourceStatusOverlay views to: [.AnyData, .Error, .Loading]

Everything now works as hoped/expected.

I also tried the setting you suggest above - [.Retrying, .AnyData, .Error, .Loading] - but did not get the results I needed. When I downloaded a resource and then went offline, the StatusOverlay displayed an error (rather than the stale data) the next time a request was made.

Thanks also for your suggestions and code samples above regarding how to communicate differently to the user after a user-initiated request using two separate observer views and request hooks. That all makes sense and should work for us (although I haven't attempted to implement it yet).

I really appreciate such prompt attention to this issue.

[pcantrell]

Thanks for trying it out, Annica!

I’m puzzled about the behavior you saw with [.Retrying, .AnyData, .Error, .Loading]. With that setting, it certainly shouldn’t show an error if there is data. I tried this myself — refresh timer, app gets data, network that goes offline, refresh hits again — and wasn’t able to reproduce what you saw.

Would you mind double checking that you’re getting that behavior even when .AnyData comes before .Error in the list? If so, could you post some of your log output with the categories set to LogCategory.common? That would help narrow it down.

[annicaburns]

My apologies, Paul. It's working perfectly with [.Retrying, .AnyData, .Error, .Loading]

I must have seen the error before I over-rode the displayPriority property on all of my ViewControlers. I see there is a default setting that would have caused the problem. Thanks for pushing back. Sorry to waste your time. Please close this issue!

Have you been able to spend any time on the Realm-Siesta integration problem that we exchanged emails about?

[pcantrell]

No apologies necessary. “Measure twice, cut once” is just what it takes to make good software. Glad it works!

I got halfway into the Realm-Siesta question before coming down with a really nasty cold. Now catching back up. I’ll be in touch!