Skip to content

Conversation

@xster
Copy link
Member

@xster xster commented Nov 23, 2017

Fixes #12181

Mimics native behaviour. Android unaffected.

assert(details != null),
assert(
motionStartDistanceThreshold == null || motionStartDistanceThreshold > 0.0,
'Must be a positive number or null'
Copy link
Contributor

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.

Copy link
Member Author

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.

Copy link
Contributor

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.)

Copy link
Member Author

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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: trailing comma

Copy link
Member Author

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.
Copy link
Contributor

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?

Copy link
Member Author

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

@Hixie
Copy link
Contributor

Hixie commented Nov 29, 2017

LGTM

@xster xster merged commit 738e62c into flutter:master Nov 30, 2017
@xster xster deleted the scroll-threshold branch November 30, 2017 19:11
DaveShuckerow pushed a commit to DaveShuckerow/flutter that referenced this pull request May 14, 2018
…on starts (flutter#13166)

* Add a minimum distance that needs breaking on iOS each time scrolls stopped.

* Testing and tests

* tweak docs

* review
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

After drag and stop, finger lift can still cause a small position shift

3 participants