-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
Carousel and slides migration #354
Conversation
let minDelta = 0; | ||
let maxDelta = 0; | ||
let prevTr = tr.NULL; | ||
let nextTr = tr.NULL; |
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.
any chance we can rename the NULL
function into noop
or NOOP
since i was confused at first why we had a NULL
sentinel of some sort.
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.
Great idea! Done.
/cc @cramforce. for any after review comments |
this.pos_ = e.position; | ||
this.commitSwitch_(e.startPosition, this.pos_, dir); | ||
}); | ||
this.setupGestures_(); |
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.
can we move the setupGestures_
call into BaseCarousel
and add an interface to override.
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.
Done.
just move |
bf7cba5
to
6cb7fda
Compare
Carousel and slides migration
This is a big PR, but it's necessary. The main change is that the legacy SwipeX is replaced with new gestures. The gestures and motion are tuned up for responsiveness and smoothness.