Enable swipe-to-refresh for browser tabs

[Usiel]

• Including SwipeRefreshLayout dependencies & adapted fragment

• Minor fix for DuckDuckGoWebView to work with SwipeRefreshLayout

Resolves: #33

Description:
Based on PR by @malmstein #750. Fixes the weird swipe behavior in conjunction with the custom WebView (was too sensitive and did not block WebView scrolling/tapping during refresh swipe).
Some concerns and open issues:

• Refresh triggered by swipe does not change or block the toolbar refresh button. Can be added if required. I left it out for now as there's no behavior so far on that button (can be tapped multiple times).
• Had some problems with the swipe refresh indicator's edge/shadow showing at the toolbar when one releases early. Solved this with l. 858 BrowserTabFragment - there's probably a nicer way to do this? Sorry I'm not really much of an Android dev.
• Could not find a way to handle a scrollable area (e.g. textarea) at the top of the page, not sure if this a blocker? Here's an example https://jsfiddle.net/4koayw5g/show - Chrome seems to handle this. WebView does not seem to support telling us whether we are scrolling some element on the page.

[cmonfortep]

@malmstein do you want to take this PR since it's based on your previous code?
Please check if previous UX feedback from @CDRussell (posted in #750) also applies here.

[malmstein]

Thanks for this @Usiel! I'll take a look in the following days and get back to you

[amaury901130]

Suggested change

	if (!viewState.isLoading) {
	swipeRefreshContainer.isRefreshing = false
	}
	swipeRefreshContainer.isRefreshing = viewState.isLoading

[Usiel]

Since this callback is used for other loading operations (e.g. loading a link), we would show the refresh loading indicator in these cases too with the suggested change.

[amaury901130]

I suggest to use a color resource.

[Usiel]

Thanks, will adapt this assuming the blocker (see comment below) can be resolved.

[Usiel]

I found a blocker related to the scrollable content within the WebView unfortunately (see last point in the PR description). Same problem appears also when the page contains a full screen map or similar content. The user will not be able to drag up as the refresh will be triggered. Try https://www.openstreetmap.org/ for example.

I'll have a look in a bit if I can find any solutions for this.

[Usiel]

OK found something that works. There's still some work to do (overscroll indicators), but happy to hear whether the approach is fine by you.

New approach somewhat emulates Chrome's behavior:
If user swipes up we only trigger the swipe-refresh if the user was at the top of the WebView before. We keep track of the clampedY state basically. Works nicely on pages such as openstreetmap.org. I'll have some more time tomorrow hopefully to test a bit more.

Edit: All done, there might still be a small issue here or there (inactive pointers). I can work on those once we can confirm this swipe behavior is something DuckDuckGo would be happy with.

[malmstein]

Great job @Usiel! I like the implementation and the feeling after your latest changes, but I'm going to ask for internal feedback before moving forward and get back to you.

Thanks for taking care of it!

[malmstein]

@Usiel besides the overscroll indicators, I've found something that would love to have fixed. Some sites have already implemented their own version of Pull to Refresh (if you navigate to twitter.com from our app in Google Play and pull to refresh you'll see their own version) and we'd like that our implementation does not override the one from the site.

A few ideas on how to overcome this:

• Detect if a site has pull to refresh implemented
• Map the host url of a few sites that have it implemented to the current url and use a flag in the BrowserViewState that indicates if PTR should be enabled or not.

What do you think?

[Usiel]

@malmstein I agree that should definitely be fixed. I think a combination of your ideas could make sense. It would be great if less known sites with PTR would also still work.
Detection of PTR or similar: I tinkered a bit with Javascript injection & interfaces just now. That would allow us to get additional information about events, which are not available via the WebView/WebViewClient. With that I managed to allow the Twitter PTR - though the native PTR can still be triggered: Trigger Twitter PTR -> their JS stops handling events -> native PTR can be triggered.
I feel like there will always be these edge cases, so that's why a combination of your ideas makes sense to me; ensuring a great experience on popular sites and a good experience on less popular sites.
Pushed some code on my fork on feature/usiel/swipe_to_refresh_javascript. I think I'll only be able to dig a bit deeper & polish the code on the weekend though.

By the way if it's more in line with the DDG workflow we can also make this PR a draft or close it & discuss on the issue if you prefer. I suspect there will be a few more cases that need to be handled to make this perfect. Apparently there's a reason I couldn't find any open source browser (using WebView) that implements this properly ;)

[Usiel]

OK hope this one should do it. Solution is based on the CSS overscrollBehavior prop. Here's some additional information on that: https://developers.google.com/web/updates/2017/11/overscroll-behavior

[malmstein]

This is a very interesting approach @Usiel, thanks for taking the time to writing it. Although I'm not a huge fan of adding Javascript interfaces, I appreciate why this could be a good solution. Let me think about it and discuss with the rest of the team.

Nonetheless, I really appreciate your effort here. Good job!

[Usiel]

We can avoid the JS interface with evaluateJavascript, which is slightly nicer I guess. Other than that I couldn't find any way to access the computed CSS style unfortunately.

[malmstein]

Hey @Usiel I really like how this is looking!

Using evaluateJavascript makes more sense to me than the JS interface. Just a comment on that, I'd prefer to have that method in the DuckDuckGoWebView instead. Something like:

/**
 * Allows us to determine whether to (de)activate Swipe to Refresh behavior for the current page content,
 * e.g. if page implements a swipe behavior of its own already (see twitter.com).
 *
 * https://developers.google.com/web/updates/2017/11/overscroll-behavior
 */
fun detectOverscrollBehavior() {
    evaluateJavascript("(function() { return getComputedStyle(document.querySelector('body')).overscrollBehaviorY; })();") { behavior ->
        setContentAllowsSwipeToRefresh(behavior.replace("\"", "") == "auto")
    }
}

And then from the BrowseTabFragment we'd call:

if (!viewState.isLoading) {
    swipeRefreshContainer.isRefreshing = false
    webView?.detectOverscrollBehavior()
}

After testing with different websites I've only found one issues. If you merge the current develop branch with yours, do a fresh install and launch a query you'll get to this:

Blank Browser

Steps to test this PR:

1. Fresh install the app
2. Open it
3. Launch a query
4. See that Browser is blank

If you then close the app and open it again the results are there
If instead you navigate back and then forward again the results are there.

I looked with the LayoutInspector and nothing seems odd, but this behavior doesn't happen with the current develop. Can you take a look?

[Usiel]

Thanks for the refactor suggestion @malmstein. Added that, seems much cleaner.

Regarding the blank screen: I was not able to reproduce this on my physical device and a few virtual devices (API 25, 26, 28). Did you encounter the issue on a specific device?
Here are the steps for me with a fresh install with the latest code (up-to-date with this repo's develop):

[malmstein]

Thanks for the refactor @Usiel!

The issue appeared in one of the emulators API 25 and my Nexus 3a. Will continue to test with your changes and come back to you. Other than that, this is looking good and. Thanks for putting in the work!

[malmstein]

Thanks for this work @Usiel. Great addition to the app and nicely implemented! @cmonfortep we can move to Product Review after this!

[malmstein]

@Usiel thanks for your work on this! This feature will be part of the next release that will likely be available next week. Good job!

[Usiel]

Great to hear! Thanks for the review @malmstein. By the way I have some additional changes lying around to allow cancelling a refresh/load, similar to Chrome. I can create an issue + PR for that in the coming days if you'd like.

[malmstein]

That sounds good @Usiel, I'll make sure to take a look once it's opened.

[summersab]

I'm trying to eliminate my dependencies on Google by installing apps from F-Droid, instead. I noticed that the F-Droid version lacks swipe-to-refresh due to a non-free library. I have a few questions/suggeations:

1. Is there an alternate way of implementing this feature for the FOSS version?
2. If no to Empty repo? #1, you might consider adding a note to the F-Droid and/or GitHub pages to break down this and other potential differences.
3. Also if no to Empty repo? #1, would you consider hosting your own F-Droid repository for those of us who want to avoid Google Play but are willing to accept the non-free libraries? You could then add a note regarding the repository to the F-Droid description and GitGub README.
4. If no to Empty repo? #1 and Feature to hide advertisement. #3, would you consider creating a self-updating APK and hosting it on your website? This is something that NewPipe does as well as Signal (albeit begrudgingly): https://signal.org/android/apk/

Thanks for everything!!!