-
Notifications
You must be signed in to change notification settings - Fork 594
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Swipe to refresh] Indicator stuck on Android 12 stretch overscroll effect #1059
Comments
Thanks for the report. For the proposed solution, wouldn't you still need overscroll for when you hit the bottom of the list? |
Oh sorry I misread, it's only while a swipe is in progress. That does seem like a good fix at second glance. Feel free to put up a PR and I will review! |
@bentrengrove I'm working on the PR now but got one question; This basically leads to the issue with my suggested fix that the overscroll is also disabled when the user reaches the end of the list at the bottom. |
Simply wrapping the if (available.y > 0) {
state.isSwipeInProgress = true
} or moving it out of |
So basically the if (available.y > 0) {
state.isSwipeInProgress = true
} else if (state.indicatorOffset.roundToInt() == 0) {
state.isSwipeInProgress = false
} But again with #541 we would need to take the direction into account. @bentrengrove should i do two separate PR's for this as these are basically two separate issues or should i do the fix with the PR for this issue? |
Your logic seems sounds to me. Do all the tests pass? I didn't write this code though so I can't say if this was done on purpose. Don't worry about the other PR for now, best to fix the bug first. |
This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days. |
This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days. |
Why it is closed? Issue is actual. |
Just tested new build with fix, issue is not fixed fully, sometimes circle still freezing allowing overscroll list animation :( |
^^^ |
This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days. |
Any news about this issue? It's still happening for me. |
This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days. |
Description
If you stop the pull motion briefly and then resume the motion, the indicator will stay at the same place and the motion propagates down to the
LazyColumn
to trigger the overscroll effect.Note: this does not happen with the legacy overscroll effect (prior to android 12)! It only happens with the "new" stretch overscroll effect.
This only happens if the motion is interrupted and the finger stays on the screen without any movement and then resumes.
Video:
screen-20220302-090219.mp4
Steps to reproduce
Expected behavior
Upon resume of the motion, the indication should continue it's movement and continue the pull to refresh motion/action.
Additional context
This problem can be worked around by wrapping the content of the
SwipeRefresh
composable with aCompositionLocalProvider
which providesnull
asLocalOverScrollConfiguration
whilestate.isSwipeInProgress
istrue
:Maybe this is also the solution to take as a PR?
Personally i see no reason why the underlying scroll view should have any overscroll effect while
isSwipeInProgress
is true as theSwipeRefresh
composable should handle all motion events.The text was updated successfully, but these errors were encountered: