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
Fix ListWheelScrollView gestures and paint coordinates in tests #121342
Fix ListWheelScrollView gestures and paint coordinates in tests #121342
Conversation
f000764
to
e060330
Compare
tester.getTopLeft(find.widgetWithText(SizedBox, '0')), | ||
const Offset(0.0, 250.0), | ||
tester.getTopLeft(find.widgetWithText(SizedBox, '0')), | ||
offsetMoreOrLessEquals(const Offset(200.0, 250.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.
These changes are to be expected. Previously the position didn't account for transform applied to children.
200.0 = (800.0 (screen width) - 400.0 (item width in this test) ) / 2 (since the scroll view centers its children on screen)
This comment was marked as resolved.
This comment was marked as resolved.
c2935fb
to
b9e3155
Compare
/// | ||
/// Can be used to find the local bounds of this child in the viewport, | ||
/// and then use it, for example, in hit testing. | ||
Matrix4? transform; |
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.
For the docs, when would this be 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.
From what it seems, never. Added docs and assertions to ensure that.
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.
Wow! This is awesome, thank you for fixing such a highly requested feature!
(triage) @nt4f04uNd Do you still have plans to follow up on the feedback given above? |
5a1009d
to
177e4d4
Compare
Updated. I'm sorry this took a while. |
@@ -592,14 +592,26 @@ void main() { | |||
); | |||
}); | |||
|
|||
testWidgets('width of wheel in background does not increase at large widths', (WidgetTester tester) async { | |||
testWidgets('wheel does not bend outwards', (WidgetTester tester) async { |
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 test also didn't work properly because of the bug #121343
I rewrote it to actually verify the shape of the list wheel
2
decemberX = 218.5
octoberX = 218.5
So the distance in both cases was zero
In actuality, after the fix, these values were
262.6779112246874
250.75842555233146
distance 11.919485672355961
232.8608197943362
219.35910151060554
distance = 13.501718283730668
'$RenderListWheelViewport normally paints all of the children it has laid out. \n' | ||
'Did you forget to update the $ListWheelParentData.transform during the paint() call?. \n' | ||
'Did you forget to update the $ListWheelParentData.transform during the paint() call? \n' |
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.
At first I thought, what an interesting error message, but TIL we have a couple of error messages like this. Nice!
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.
auto label is removed for flutter/flutter, pr: 121342, due to - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label. |
…ter#121342) Fix ListWheelScrollView gestures and paint coordinates in tests
Fixes #38803
Fixes #121343
Pre-launch Checklist
///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.