-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
[ci] Roll Flutter to f842ed91 #4513
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
c40173f114fa8b830531578586f4f4eedd2b2c1f | ||
f842ed916514879fe6898b2a5a4053c63c3308fe |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,17 +3,17 @@ | |
// found in the LICENSE file. | ||
|
||
import 'package:example/wrap_layout_example.dart'; | ||
import 'package:flutter/foundation.dart'; | ||
import 'package:flutter/material.dart'; | ||
import 'package:flutter_test/flutter_test.dart'; | ||
|
||
void main() { | ||
testWidgets('Check that the children are layed out.', | ||
(WidgetTester tester) async { | ||
await tester.pumpWidget( | ||
const MaterialApp( | ||
home: WrapExample(), | ||
), | ||
const MaterialApp app = MaterialApp( | ||
home: WrapExample(), | ||
); | ||
await tester.pumpWidget(app); | ||
await tester.pumpAndSettle(); | ||
|
||
// See if there are children layed out. | ||
|
@@ -23,10 +23,29 @@ void main() { | |
expect(find.text('Index 3'), findsOneWidget); | ||
expect(find.text('Index 4'), findsOneWidget); | ||
|
||
// Material 3 changes the expected layout positioning. | ||
final bool usesMaterial3 = (app.theme ?? ThemeData.light()).useMaterial3; | ||
final Offset offset0 = usesMaterial3 | ||
? Offset(0.0, _getExpectedYOffset(91.0)) | ||
: const Offset(0.0, 103.0); | ||
final Offset offset1 = usesMaterial3 | ||
? Offset(65.0, _getExpectedYOffset(121.0)) | ||
: const Offset(66.0, 124.0); | ||
final Offset offset3 = usesMaterial3 | ||
? Offset(270.0, _getExpectedYOffset(171.0)) | ||
: const Offset(271.0, 174.0); | ||
final Offset offset4 = usesMaterial3 | ||
? Offset(380.0, _getExpectedYOffset(221.0)) | ||
: const Offset(381.0, 224.0); | ||
|
||
// See if they are in expected position. | ||
expect(tester.getTopLeft(find.text('Index 0')), const Offset(0.0, 103.0)); | ||
expect(tester.getTopLeft(find.text('Index 1')), const Offset(66.0, 124.0)); | ||
expect(tester.getTopLeft(find.text('Index 3')), const Offset(271.0, 174.0)); | ||
expect(tester.getTopLeft(find.text('Index 4')), const Offset(381.0, 224.0)); | ||
expect(tester.getTopLeft(find.text('Index 0')), offset0); | ||
expect(tester.getTopLeft(find.text('Index 1')), offset1); | ||
expect(tester.getTopLeft(find.text('Index 3')), offset3); | ||
expect(tester.getTopLeft(find.text('Index 4')), offset4); | ||
}); | ||
} | ||
|
||
double _getExpectedYOffset(double nonWeb) { | ||
return kIsWeb ? nonWeb - 0.5 : nonWeb; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yikes! Is this a "web is rounding wrong" bug? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmmm maybe @yjbanov would know? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I brought this up to the team chat as well! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I vaguely recall we had a special-case for an off-by-one issue in text for Firefox, but not generally. @mdebbar might know more. |
||
} |
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, I double checked. The slight change in offset is expected.
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.
Perfect, thanks for verifying!