-
Notifications
You must be signed in to change notification settings - Fork 28.5k
Add a snapAnimationDuration param in DraggableScrollableSheet #107396
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
Conversation
slow_snap_animation.mp4 |
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 look good, just a few trivialities to deal with.
packages/flutter/lib/src/widgets/draggable_scrollable_sheet.dart
Outdated
Show resolved
Hide resolved
/// | ||
/// If it's not set, then the animation duration is the distance to the snap | ||
/// target divided by the velocity of the widget. | ||
final Duration? snapAnimationDuration; |
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.
What if this value is zero? Do we want to support that to indicate "no animation"?
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.
I think it's unlikely to be a use case to snap the bottom sheet with no animation. So The negative/zero/less than 1 milliseconds durations can be considered as unset.
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 can support the no animation snap use case when there are new feature requests.
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.
< 1 seems a bit arbitrary. Since 1 frame is typically 16ms, it might make more sense to use that, since a duration of less than a frame is effectively 0.
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.
Thinking about this again; I think it would be best to just assert that if a duration was specified it must be > 0.
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.
Assertion added
|
||
// The duration of the snap animation is defined. | ||
if(snapAnimationDuration != null) { | ||
velocity = (_pixelSnapSize - position) * 1000 / snapAnimationDuration.inMilliseconds; |
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.
What if the duration is zero?
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.
Ditto, added a check.
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.
LGTM
…r#107396) * Add a snapAnimationDuration param in DraggableScrollableSheet * snapAnimationDuration.inMilliseconds > 0 * Update draggable_scrollable_sheet.dart
Add a new param so user can define the duration of the snap animation of a DraggableScrollableSheet.
video attached with snap animations duration = 0.5S
Fixed: #107033
Pre-launch Checklist
///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.