PageView ballistics overshoot the page on some devices #12884
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
On some devices, such as Cupertino “Plus”-sized devices, scrolling left on the first page of a PageView will overshoot the first page and land on the second page. The issue is that applyContentDimensions incorrectly detects a content size change due to a floating point comparison on certain screen sizes (18257.400000000005 vs 18257.4) To fix this, perform a nearEqual comparison in applyContentDimensions.
|
@HansMuller is probably a good reviewer. Thanks for the patch! |
Hixie
reviewed
Nov 9, 2017
| return (a == b); | ||
| } else { | ||
| return (a > (b - epsilon)) && (a < (b + epsilon)); | ||
| } |
There was a problem hiding this comment.
Please write this as:
/// Whether two doubles are within a given distance of each other.
///
/// The `epsilon` argument must be positive and not null.
/// The `a` and `b` arguments may be null. A null value is
/// only considered near-equal to another null value.
 bool nearEqual(double a, double b, double epsilon) {
assert(epsilon != null);
 assert(epsilon >= 0.0);
if (a == null || b == null)
return a == b;
return (a > (b - epsilon)) && (a < (b + epsilon));
}Thanks!
|
I don't fully understand why the maths doesn't work out, but this seems reasonable.
|
|
And thanks for the tests! |
|
@jwmcglynn Please let us know when you've updated the patch as suggested, and then we can land it. Thanks again! |
|
Hey @Hixie, I pushed a new commit that addresses your feedback. Thanks for the review! |
|
Thanks for the contribution! |
DaveShuckerow
pushed a commit
to DaveShuckerow/flutter
that referenced
this pull request
May 14, 2018
* PageView ballistics overshoot the page on some devices On some devices, such as Cupertino “Plus”-sized devices, scrolling left on the first page of a PageView will overshoot the first page and land on the second page. The issue is that applyContentDimensions incorrectly detects a content size change due to a floating point comparison on certain screen sizes (18257.400000000005 vs 18257.4) To fix this, perform a nearEqual comparison in applyContentDimensions. * Apply style changes to nearEqual for code review feedback.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
On some devices, such as Cupertino “Plus”-sized devices, scrolling left on the first page of a PageView will overshoot the first page and land on the second page. Here is a gif that shows the problem:
The issue is that applyContentDimensions incorrectly detects a content size change due to a floating point comparison on certain screen sizes (18257.400000000005 vs 18257.4)
To fix this, perform a nearEqual comparison in applyContentDimensions.