Skip to content

Better scroll animation when item expands (fixes #90 too) #102

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lorenzos
Copy link

This improves the scroll animation which occurs after an item has been expanded a lot, imho. Not only it solves #90, but also manage the special case in which the item is taller than the container ListView. In particular, after the item expands, it makes some computation and then:

  1. If the just expanded item is already fully visible, nothing happens.
  2. If it is bigger than the container ListView, it is scrolled so its top bound is on top of the list.
  3. If its top bound is out if the container ListView, it is scrolled so it's on top of the list.
  4. If its bottom bound is out if the container ListView, it is scrolled so it's at bottom of the list.

I made this edit particularly because of case 2: I'm using items that, when expanded, can be really tall. Not sure if other cases were already covered, but I'm quite sure I experienced issues also when items were smaller.

I tested it on the most recent API and on API 8 and 10. It works as expected in all cases. I made the PR on top of your last commit and in a separate brach, so you can merge with no problems.

Further improvement to make it perfect could be starting the scroll animation together with the expand animation (i.e. in the onAnimationStart callback instead of onAnimationEnd). Unfortunately this requires not only to get the end height in advance (from the ExpandCollapseAnimation instance for example), but also to take account of the old expanded item which, in the meantime, could be collapsing. I'll work on it if I'll have some spare time, it's not critical for me atm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant