Fix: Menu anchor respect alignment offset for right #164858
Conversation
41c35b8 to
4a8f410
Compare
| await changeSurfaceSize(tester, const Size(800, 600)); | ||
| await tester.pumpWidget( | ||
| MaterialApp( | ||
| theme: ThemeData(useMaterial3: false), |
There was a problem hiding this comment.
This can be omitted if this is not specific to M2.
There was a problem hiding this comment.
I wanted to be consistent with other tests, specifically alignment test of menu anchor.
There was a problem hiding this comment.
They seem like old tests, not yet migrated to M3.
For your new tests, if the results are different in M2 and M3, please split them two tests. Each test should you should the following naming scheme.
Material 2 - xxx
Material3 - xxx
Otherwise, if the results are same in both M2 and M3 then you can remove ThemeData(useMaterial3: false) from this test.
| home: Builder( | ||
| builder: (BuildContext context) { | ||
| return Directionality( | ||
| textDirection: TextDirection.ltr, |
There was a problem hiding this comment.
Directionality is provided by MaterialApp, this can be skipped.
| expect(find.byType(MenuItemButton), findsOneWidget); | ||
| expect( | ||
| collectSubmenuRects(), | ||
| equals(const <Rect>[Rect.fromLTRB(634.0, 488.0, 756.0, 552.0)]), |
There was a problem hiding this comment.
Could please add a comment on how these numbers verify "menus should respect overflow on right with the provided offset"?
4a8f410 to
4e37fcd
Compare
There was a problem hiding this comment.
Running test code without this PR and with this PR has no difference. In fact, the PR test also passes without the fix.
Code Sample
expand to view the code sample
import 'package:flutter/material.dart';
void main() => runApp(const MyApp());
class MyApp extends StatelessWidget {
const MyApp({super.key});
@override
Widget build(BuildContext context) {
return MaterialApp(
theme: ThemeData(useMaterial3: false),
home: Align(
alignment: Alignment.bottomRight,
child: MenuAnchor(
alignmentOffset: const Offset(-50, 0),
menuChildren: const <Widget>[MenuItemButton(child: Text('Button1'))],
builder: (BuildContext context, MenuController controller, Widget? child) {
return FilledButton(
onPressed: () {
if (controller.isOpen) {
controller.close();
} else {
controller.open();
}
},
child: const Text('Tap me'),
);
},
),
),
);
}
}
Please make sure the fix is effective. Update PR description with before and after to showcase the fix visually.
| await changeSurfaceSize(tester, const Size(800, 600)); | ||
| await tester.pumpWidget( | ||
| MaterialApp( | ||
| theme: ThemeData(useMaterial3: false), |
There was a problem hiding this comment.
They seem like old tests, not yet migrated to M3.
For your new tests, if the results are different in M2 and M3, please split them two tests. Each test should you should the following naming scheme.
Material 2 - xxx
Material3 - xxx
Otherwise, if the results are same in both M2 and M3 then you can remove ThemeData(useMaterial3: false) from this test.
Ohh, I think this need to have one change to see effect that offset should be positive. |
be239dd to
119e0cf
Compare
119e0cf to
20778fa
Compare
Fix: Menu anchor respect alignment offset for right
fixes: #159413
Pre-launch Checklist
///).