Skip to content
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

Setting refresh initial offset keeps loading animation after setRefreshing(false) is called #11

Closed
branislav-zlatkovic opened this issue Jul 21, 2021 · 6 comments
Labels
bug Something isn't working

Comments

@branislav-zlatkovic
Copy link

branislav-zlatkovic commented Jul 21, 2021

Refresh animation still visible after setRefreshing(false) if there's setRefreshInitialOffset() applied.

Steps to reproduce the behavior:

  1. init with the following:
    swipeRefresh.setRepeatMode(SSPullToRefreshLayout.RepeatMode.REPEAT)
    swipeRefresh.setRepeatCount(SSPullToRefreshLayout.RepeatCount.INFINITE)
    and wrap around a recyclerview that has clipToPadding = false and a top padding at a double size of a standard toolbar height
  2. set the initial offset
    swipeRefresh.setRefreshInitialOffset(/half the measured height of the above recyclerview padding/)
  3. attach a listener in which loading will be initiated
    swipeRefresh.setOnRefreshListener.... { loadData() }
  4. set setRefreshing(false) when data load is finished, observe the loading animation is still present

Expected behavior
Loading animation is gone after calling setRefreshing(false) with initial refresh offset set, just as it would be gone if there's no initial refresh offset set - it would probably just be enough to set visibility = GONE to mRefreshView

@branislav-zlatkovic branislav-zlatkovic added the bug Something isn't working label Jul 21, 2021
@branislav-zlatkovic
Copy link
Author

After some debugging, I see that reset() is being called, from the mResetListener->onAnimationEnd which in turn sets the mRefreshView.visibility = GONE, however mRefreshView visibility is set to VISIBLE again right after in setTargetOrRefreshViewOffsetY, particularly here:
if (mRefreshView.visibility != VISIBLE) {
mRefreshView.visibility = VISIBLE
}
now, a quick modification to the condition like this:
if (mIsRefreshing && mRefreshView.visibility != VISIBLE) {
mRefreshView.visibility = VISIBLE
}
seems to fix the issue for me - now the refresh view is really gone once setRefreshing(false) is called... haven't checked though if this change affects anything else

@nishchal-v
Copy link
Collaborator

@branislav-zlatkovic thank you for posting the issue,
I will look into this issue, and try to fix it as soon as possible
P.S. your solution looks good and seems like it won't affect other functionality, but need to check it thoroughly.

@branislav-zlatkovic
Copy link
Author

thanks for a swift reply @nishchal-v-simformsolutions !

The above solution proposal seem to work great as long the mRefreshView is not scrolled back into it's original position by user scrolling the recyclerview content, then it again 'get stuck' in being visible after setRefreshing(false) is being called.

In other words, if loading takes a while and user starts scrolling up while loading still is in progress, once loading finish after mRefreshView has been put into it's original position by the scroll, the mRefreshView won't disappear (visibility != GONE).

Additional debugging shows that in this case the following if statement fulfills the condition, preventing the animation's listener ultimately calling reset() when things are done:
animateOffsetToStartPosition() ->

if (computeAnimateToStartDuration(from.toFloat()) <= 0) {
mAnimateToStartAnimation.cancel()
return
}

My quick solution inserts reset() call from within the condition, like this:

if (computeAnimateToStartDuration(from.toFloat()) <= 0) {
mAnimateToStartAnimation.cancel()
reset()
return
}

This fix seem to work great as a complementary to the above proposed solution, but haven't fully checked if it affect anything else.

@branislav-zlatkovic
Copy link
Author

After some more testing of the above proposed solution, turns out that
if (mIsRefreshing && mRefreshView.visibility != VISIBLE) {
mRefreshView.visibility = VISIBLE
}
does have a negative impact which manifests so that once refresh animation is done and gone, the next pull to refresh needs to actually start refreshing so the animation appears on the screen again.
A new solution proposal checks whether the touch offset is not 0, like this:
if (mCurrentTouchOffsetY != 0f && mRefreshView.visibility != VISIBLE) {
mRefreshView.visibility = VISIBLE
}
this way the animation view always appears on user scroll, but not once the refresh is done and reset() is called.

@ShwetaChauhan18
Copy link
Contributor

Thanks @branislav-zlatkovic. We will try to fix it ASAP

nishchal-v added a commit that referenced this issue Aug 5, 2021
nishchal-v added a commit that referenced this issue Aug 5, 2021
@nishchal-v
Copy link
Collaborator

@branislav-zlatkovic fixed the issue you posted, and thanks for the suggestion of yours

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants