readium
Move the activity indicator from the navigator to the testapp
Right now an activity indicator is displayed inside the navigator while a resource (from the readingOrder) is loaded.
https://github.com/readium/r2-navigator-swift/blob/1f80d58fba510d62b0d2fed5cd43632576552a8b/r2-navigator-swift/EPUB/DocumentWebView.swift#L502
This is a problem because the test app should be in control of everything related to the UX and style.
I suggest moving this into the testapp itself, and add events in the navigator API to notify the testapp that a resource is being loaded.
Note that this pose an issue because the test app can't directly put an activity indicator on the resource view that is being scrolled, it will be positioned absolutely above the navigator and not scroll with the document. Maybe we need an API from the navigator to expose the view of the resource, so that the testapp can add it directly on it.
As an alternative, the testapp could provide an "activity indicator factory" that will be called by the navigator, and the navigator will automatically add/remove the activity indicator on the resource when relevant.
This was contributed by Bookbeat, might be worth pinging them as well. cc @ullstrm
This would indeed be the better architecture. But as you say the best placement of the loading indicator is in the webview itself. Probably there could be some kind of optional loading view creation block (i.e. the factory), but it's not a trivial change to make if one would allow full customization of both the style and placement of the activity indicator. However - I'm all for this change.
A possible solution that allows all the customization needed would be this delegate to be implemented by the host app (possibly the navigator can provide a default implementation using the current UIActivityIndicatorView)
func addActivityIndicator(in contentView: UIView) -> UIView?
The host app can create an activity view, add it and position it freely inside the contentView, and then return the newly created activity view. The navigator is then responsible to remove the view from the contentView once the document is loaded.
We probably need to cover that issue in our Navigator APIs before we start working on this.
A loading indicator remains essential for most apps, delegating this to app-level code would imply:
- providing a good example in the test app
- and ideally mentioning this feature in our README/doc for the navigator