tobiasrohloff / NestedScrollWebView

An Android WebView that implements NestedScrollingChild, in order to use it with CoordinatorLayout and AppBarLayout.
Other
334 stars 81 forks source link

Fixed dispatchNestedScroll behavior of onTouchEvent. #8

Closed ebonhart closed 7 years ago

ebonhart commented 7 years ago

Hello.

We recently implemented your NestedScrollWebView in our App to fix a problem with touch gestures not being recognized in embedded content. While this problem was fixed, we found that the NestScrollWebView introduced a new bug where our floating action button didn't behave as before. After some investigation, I found out that the dyConsumed values sent by NestedScrollWebView didn't fit to what our button expects. I closely analyzed NestedScrollWebView and some of the calculations in there didn't really make sense to me. Hence, I made a fix which works flawlessly for us. Here is the pull request. Maybe you find it useful as a general fix for the component:

Explanation:

First of all, the (delta < 0) check causes the NSWebView to only ever dispatch events when the user scrolls up. Our button expects events for either scroll direction, however (and I'd wager this is a general expectation).

Secondly, the calculation of the second parameter that is passed into dispatchNestedScroll didn't make sense to me. As far as I understood the documentation, the parameter should be the consumed pixels that were scrolled by the movement. I fixed and cleaned up the calculation of both, dyConsumed and dyUnconsumed and improved the variable naming, to improve readability of this part of the function.

PS: Please don't look at the first commit by itself. I still had some oversights in there.

tobiasrohloff commented 7 years ago

Thanks a lot for your PR. I will review and test soon!

tobiasrohloff commented 7 years ago

Sorry for the late response, works well for me. Thank you!