-
Notifications
You must be signed in to change notification settings - Fork 754
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
Disabling automatic carousel transitions on user interaction #4914
Disabling automatic carousel transitions on user interaction #4914
Conversation
- extracts all the logic to its own extension
override fun onPageSelected(position: Int) { | ||
scheduledTransition?.cancel() | ||
override fun onPageScrolled(position: Int, positionOffset: Float, positionOffsetPixels: Int) { | ||
hasUserManuallyInteractedWithCarousel = !isFakeDragging |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we check if the scroll was performed by a fake drag, if so we infer that it was the automatic transition and allow the next transition to be scheduled
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Just one small remark
override fun onPageSelected(position: Int) { | ||
scheduledTransition?.cancel() | ||
if (hasUserManuallyInteractedWithCarousel) { | ||
// stop the automatic transitions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is fine but maybe avoid having empty block. I think lint/compiler will also complain. Better to write if(!...)
and just add the comment above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in this case the comment counts as a non empty block, will invert the condition and remove the branch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Matrix SDKIntegration Tests Results:
|
Part of #4584