-
Notifications
You must be signed in to change notification settings - Fork 29.5k
Let iOS have a minimum scroll movement threshold to break before motion starts #13166
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
| assert(details != null), | ||
| assert( | ||
| motionStartDistanceThreshold == null || motionStartDistanceThreshold > 0.0, | ||
| 'Must be a positive number or 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.
This should say what must be positive or null. The error message will have no context.
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.
Seems like asserts with optional messages actually shows both the assert test code and the message.
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.
Yes, but experience shows our users don't understand the assertion test code. They only read the text (if we're lucky). (The framework actually flips the message around so the message comes first and the assert code is second, now.)
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
| @required DragStartDetails details, | ||
| this.onDragCanceled, | ||
| this.carriedVelocity, | ||
| this.motionStartDistanceThreshold |
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.
nit: trailing comma
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
| /// Returns false either way if there's no offset. | ||
| bool _breakMotionStartThreshold(double offset, Duration timestamp) { | ||
| if (timestamp == null) { | ||
| // If we can't track time, we can't apply thresholds. Should never happen. |
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.
If it should never happen, assert instead?
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.
Clarified comment. Can be null with accessibility gestures
…on starts (flutter#13166) * Add a minimum distance that needs breaking on iOS each time scrolls stopped. * Testing and tests * tweak docs * review
Fixes #12181
Mimics native behaviour. Android unaffected.