Skip to content
This repository has been archived by the owner on Mar 23, 2023. It is now read-only.

Scrolling troubles #62

Closed
JakeWharton opened this issue Oct 31, 2020 · 12 comments
Closed

Scrolling troubles #62

JakeWharton opened this issue Oct 31, 2020 · 12 comments
Labels
help wanted Extra attention is needed

Comments

@JakeWharton
Copy link

I'm having a lot of trouble scrolling the example files. The inbox (RIP) recycler view action takes precedence somewhere around 50% of the time.

https://photos.app.goo.gl/v5DzRWvBJthfWoKi9

(I swear in the video scrolling was working about twice as much as it otherwise was.)

@saket
Copy link
Owner

saket commented Oct 31, 2020

Damn, issues that I can't reproduce on my phone are my least favorite ones. Are you able to consistently reproduce this (across app restarts)?

@JakeWharton
Copy link
Author

Yep. Android 11, Pixel 5. Every time, unfortunately.

@saket
Copy link
Owner

saket commented Nov 1, 2020

Can you please try out this APK? It prints a toast with debug info every time a scroll begins. A screenshot will hopefully give me some hints about the problem.

@JakeWharton
Copy link
Author

Sure, random person on the internet, I'll install your APK!

https://photos.app.goo.gl/AXtTfKVdPoR2Exut8

@JakeWharton
Copy link
Author

Show touches was off, but it was always done in the center-ish/middle-ish of the screen.

@saket
Copy link
Owner

saket commented Nov 2, 2020

Very interesting. InboxRecyclerView is incorrectly calculating the direction of gestures. I see canScrollVertically(1) on my phone vs -1 on yours when scrolling to the bottom of a note.

val canScrollFurther = view.canScrollVertically(directionInt)

I'm simply comparing MotionEvent.rawY values (source). Would you happen to know if we aren't supposed to use rawY?

fun onTouch(event: MotionEvent, consumeDowns: Boolean): Boolean {
    when (event.actionMasked) {
      ACTION_DOWN -> {
        downY = event.rawY
      }

      ACTION_MOVE -> {
        val totalSwipeDistanceY = event.rawY - downY
        val upwardSwipe = totalSwipeDistanceY < 0F
        ...

Sure, random person on the internet, I'll install your APK!

The joys of debugging UI issues 😔. The other alternatives are to buy a Pixel 5 for testing or ask you to help me debug the touch listener.

@saket
Copy link
Owner

saket commented Nov 3, 2020

I wonder if I should be using MotionEvent#y instead of raw coordinates. Will try out.

@saket saket added the help wanted Extra attention is needed label Jan 13, 2021
@SimonMarquis
Copy link

Hi @saket, I'm experiencing the same issue here on my Pixel 4 XL.
I think it's related to how you handle the deltaUpwardSwipe and it's even more likely to appear when you have a higher touch refresh rate (that might explain why you don't reproduce it).

On my device, swiping slowly will reproduce the issue, while swiping fast won't.

Here is what happen in PullToCollapseListener when I swipe upward fast:

MotionEvent { action=ACTION_DOWN, x[0]=1006.0, y[0]=2001.0 }
MotionEvent { action=ACTION_MOVE, x[0]=1041.0, y[0]=1476.0 }

The first ACTION_MOVE has a negative deltaY therefore the gesture will be intercepted by the nested view.

But here is what happen when I swipe upward slow:

MotionEvent { action=ACTION_DOWN, x[0]=508.0, y[0]=2088.0 }
MotionEvent { action=ACTION_MOVE, x[0]=508.0, y[0]=2088.0 }
MotionEvent { action=ACTION_MOVE, x[0]=508.0, y[0]=2079.5679 }

The first ACTION_MOVE has a deltaY equals to 0, and therefore the gesture will start the pull callback.

I think you might have to use the touchSlop on the vertical axis as well,

@saket
Copy link
Owner

saket commented Jan 17, 2021

Interesting. I don't think it's related to the refresh rate because I've been using a 120hz display to test Press. I ended up deciding to add support for nested scrolling to InboxRecyclerView so that I don't have to deal with coordinates on my own. Will have it ready soon. Thanks anyway @SimonMarquis!

@saket
Copy link
Owner

saket commented Jan 18, 2021

Should be fixed by a21985b. Nested scrolling FTW!

@saket saket closed this as completed Jan 18, 2021
@saket
Copy link
Owner

saket commented Apr 2, 2021

Released in v1.8: https://github.com/saket/press/releases/tag/v1.8. @JakeWharton @SimonMarquis I didn't have a Pixel 4/5 to test, but I'm hoping my fix will work. Please let me know if it still doesn't?

@SimonMarquis
Copy link

This version works flawlessly on my pixel 4 XL! 🙌

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants