Skip to content

Commit

Permalink
Fix MenuAnchor padding (#116573)
Browse files Browse the repository at this point in the history 
* Fix MenuAnchor padding

* Add tests
  • Loading branch information
@gspencergoog
gspencergoog committed Dec 6, 2022
1 parent 76f0335 commit 577a88b
Show file tree
Hide file tree
Showing 2 changed files with 205 additions and 58 deletions.
58 changes: 25 additions & 33 deletions packages/flutter/lib/src/material/menu_anchor.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1796,21 +1796,23 @@ class _SubmenuButtonState extends State<SubmenuButton> {

@override
Widget build(BuildContext context) {
final Offset menuPaddingOffset;
Offset menuPaddingOffset = widget.alignmentOffset ?? Offset.zero;
final EdgeInsets menuPadding = _computeMenuPadding(context);
switch (_anchor?._root._orientation ?? Axis.vertical) {
// Move the submenu over by the size of the menu padding, so that
// the first menu item aligns with the submenu button that opens it.
switch (_anchor?._orientation ?? Axis.vertical) {
case Axis.horizontal:
switch (Directionality.of(context)) {
case TextDirection.rtl:
menuPaddingOffset = widget.alignmentOffset ?? Offset(-menuPadding.right, 0);
menuPaddingOffset += Offset(menuPadding.right, 0);
break;
case TextDirection.ltr:
menuPaddingOffset = widget.alignmentOffset ?? Offset(-menuPadding.left, 0);
menuPaddingOffset += Offset(-menuPadding.left, 0);
break;
}
break;
case Axis.vertical:
menuPaddingOffset = widget.alignmentOffset ?? Offset(0, -menuPadding.top);
menuPaddingOffset += Offset(0, -menuPadding.top);
break;
}

Expand Down Expand Up @@ -1900,24 +1902,13 @@ class _SubmenuButtonState extends State<SubmenuButton> {
}

EdgeInsets _computeMenuPadding(BuildContext context) {
final MenuStyle? themeStyle = MenuTheme.of(context).style;
final MenuStyle defaultStyle = _MenuDefaultsM3(context);

T? effectiveValue<T>(T? Function(MenuStyle? style) getProperty) {
return getProperty(widget.menuStyle) ?? getProperty(themeStyle) ?? getProperty(defaultStyle);
}

T? resolve<T>(MaterialStateProperty<T>? Function(MenuStyle? style) getProperty) {
return effectiveValue(
(MenuStyle? style) {
return getProperty(style)?.resolve(widget.statesController?.value ?? const <MaterialState>{});
},
);
}

return resolve<EdgeInsetsGeometry?>(
(MenuStyle? style) => style?.padding,
)?.resolve(Directionality.of(context)) ?? EdgeInsets.zero;
final MaterialStateProperty<EdgeInsetsGeometry?> insets =
widget.menuStyle?.padding ??
MenuTheme.of(context).style?.padding ??
_MenuDefaultsM3(context).padding!;
return insets
.resolve(widget.statesController?.value ?? const <MaterialState>{})!
.resolve(Directionality.of(context));
}

void _handleFocusChange() {
Expand Down Expand Up @@ -3190,14 +3181,15 @@ class _MenuLayout extends SingleChildLayoutDelegate {

@override
bool shouldRelayout(_MenuLayout oldDelegate) {
return anchorRect != oldDelegate.anchorRect ||
textDirection != oldDelegate.textDirection ||
alignment != oldDelegate.alignment ||
alignmentOffset != oldDelegate.alignmentOffset ||
menuPosition != oldDelegate.menuPosition ||
orientation != oldDelegate.orientation ||
parentOrientation != oldDelegate.parentOrientation ||
!setEquals(avoidBounds, oldDelegate.avoidBounds);
return anchorRect != oldDelegate.anchorRect
|| textDirection != oldDelegate.textDirection
|| alignment != oldDelegate.alignment
|| alignmentOffset != oldDelegate.alignmentOffset
|| menuPosition != oldDelegate.menuPosition
|| menuPadding != oldDelegate.menuPadding
|| orientation != oldDelegate.orientation
|| parentOrientation != oldDelegate.parentOrientation
|| !setEquals(avoidBounds, oldDelegate.avoidBounds);
}

Rect _closestScreen(Iterable<Rect> screens, Offset point) {
Expand Down Expand Up @@ -3301,7 +3293,7 @@ class _MenuPanelState extends State<_MenuPanel> {
final double dy = densityAdjustment.dy;
final double dx = math.max(0, densityAdjustment.dx);
final EdgeInsetsGeometry resolvedPadding = padding
.add(EdgeInsets.fromLTRB(dx, dy, dx, dy))
.add(EdgeInsets.symmetric(horizontal: dx, vertical: dy))
.clamp(EdgeInsets.zero, EdgeInsetsGeometry.infinity); // ignore_clamp_double_lint

BoxConstraints effectiveConstraints = visualDensity.effectiveConstraints(
Expand Down Expand Up @@ -3430,7 +3422,7 @@ class _Submenu extends StatelessWidget {
);

final VisualDensity visualDensity =
effectiveValue((MenuStyle? style) => style?.visualDensity) ?? VisualDensity.standard;
effectiveValue((MenuStyle? style) => style?.visualDensity) ?? Theme.of(context).visualDensity;
final AlignmentGeometry alignment = effectiveValue((MenuStyle? style) => style?.alignment)!;
final BuildContext anchorContext = anchor._anchorKey.currentContext!;
final RenderBox overlay = Overlay.of(anchorContext).context.findRenderObject()! as RenderBox;
Expand Down
205 changes: 180 additions & 25 deletions packages/flutter/test/material/menu_anchor_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1812,7 +1812,7 @@ void main() {
});

group('Layout', () {
List<Rect> collectMenuRects() {
List<Rect> collectMenuItemRects() {
final List<Rect> menuRects = <Rect>[];
final List<Element> candidates = find.byType(SubmenuButton).evaluate().toList();
for (final Element candidate in candidates) {
Expand All @@ -1824,6 +1824,18 @@ void main() {
return menuRects;
}

List<Rect> collectSubmenuRects() {
final List<Rect> menuRects = <Rect>[];
final List<Element> candidates = findMenuPanels().evaluate().toList();
for (final Element candidate in candidates) {
final RenderBox box = candidate.renderObject! as RenderBox;
final Offset topLeft = box.localToGlobal(box.size.topLeft(Offset.zero));
final Offset bottomRight = box.localToGlobal(box.size.bottomRight(Offset.zero));
menuRects.add(Rect.fromPoints(topLeft, bottomRight));
}
return menuRects;
}

testWidgets('unconstrained menus show up in the right place in LTR', (WidgetTester tester) async {
await changeSurfaceSize(tester, const Size(800, 600));
await tester.pumpWidget(
Expand Down Expand Up @@ -1855,12 +1867,16 @@ void main() {

expect(find.byType(MenuItemButton), findsNWidgets(6));
expect(find.byType(SubmenuButton), findsNWidgets(5));
final List<Rect> menuRects = collectMenuRects();
expect(menuRects[0], equals(const Rect.fromLTRB(4.0, 0.0, 112.0, 48.0)));
expect(menuRects[1], equals(const Rect.fromLTRB(112.0, 0.0, 220.0, 48.0)));
expect(menuRects[2], equals(const Rect.fromLTRB(220.0, 0.0, 328.0, 48.0)));
expect(menuRects[3], equals(const Rect.fromLTRB(328.0, 0.0, 506.0, 48.0)));
expect(menuRects[4], equals(const Rect.fromLTRB(112.0, 104.0, 326.0, 152.0)));
expect(
collectMenuItemRects(),
equals(const <Rect>[
Rect.fromLTRB(4.0, 0.0, 112.0, 48.0),
Rect.fromLTRB(112.0, 0.0, 220.0, 48.0),
Rect.fromLTRB(220.0, 0.0, 328.0, 48.0),
Rect.fromLTRB(328.0, 0.0, 506.0, 48.0),
Rect.fromLTRB(112.0, 104.0, 326.0, 152.0),
]),
);
});

testWidgets('unconstrained menus show up in the right place in RTL', (WidgetTester tester) async {
Expand Down Expand Up @@ -1897,12 +1913,16 @@ void main() {

expect(find.byType(MenuItemButton), findsNWidgets(6));
expect(find.byType(SubmenuButton), findsNWidgets(5));
final List<Rect> menuRects = collectMenuRects();
expect(menuRects[0], equals(const Rect.fromLTRB(688.0, 0.0, 796.0, 48.0)));
expect(menuRects[1], equals(const Rect.fromLTRB(580.0, 0.0, 688.0, 48.0)));
expect(menuRects[2], equals(const Rect.fromLTRB(472.0, 0.0, 580.0, 48.0)));
expect(menuRects[3], equals(const Rect.fromLTRB(294.0, 0.0, 472.0, 48.0)));
expect(menuRects[4], equals(const Rect.fromLTRB(474.0, 104.0, 688.0, 152.0)));
expect(
collectMenuItemRects(),
equals(const <Rect>[
Rect.fromLTRB(688.0, 0.0, 796.0, 48.0),
Rect.fromLTRB(580.0, 0.0, 688.0, 48.0),
Rect.fromLTRB(472.0, 0.0, 580.0, 48.0),
Rect.fromLTRB(294.0, 0.0, 472.0, 48.0),
Rect.fromLTRB(474.0, 104.0, 688.0, 152.0),
]),
);
});

testWidgets('constrained menus show up in the right place in LTR', (WidgetTester tester) async {
Expand Down Expand Up @@ -1937,12 +1957,16 @@ void main() {

expect(find.byType(MenuItemButton), findsNWidgets(6));
expect(find.byType(SubmenuButton), findsNWidgets(5));
final List<Rect> menuRects = collectMenuRects();
expect(menuRects[0], equals(const Rect.fromLTRB(4.0, 0.0, 112.0, 48.0)));
expect(menuRects[1], equals(const Rect.fromLTRB(112.0, 0.0, 220.0, 48.0)));
expect(menuRects[2], equals(const Rect.fromLTRB(220.0, 0.0, 328.0, 48.0)));
expect(menuRects[3], equals(const Rect.fromLTRB(328.0, 0.0, 506.0, 48.0)));
expect(menuRects[4], equals(const Rect.fromLTRB(86.0, 104.0, 300.0, 152.0)));
expect(
collectMenuItemRects(),
equals(const <Rect>[
Rect.fromLTRB(4.0, 0.0, 112.0, 48.0),
Rect.fromLTRB(112.0, 0.0, 220.0, 48.0),
Rect.fromLTRB(220.0, 0.0, 328.0, 48.0),
Rect.fromLTRB(328.0, 0.0, 506.0, 48.0),
Rect.fromLTRB(86.0, 104.0, 300.0, 152.0),
]),
);
});

testWidgets('constrained menus show up in the right place in RTL', (WidgetTester tester) async {
Expand Down Expand Up @@ -1977,12 +2001,143 @@ void main() {

expect(find.byType(MenuItemButton), findsNWidgets(6));
expect(find.byType(SubmenuButton), findsNWidgets(5));
final List<Rect> menuRects = collectMenuRects();
expect(menuRects[0], equals(const Rect.fromLTRB(188.0, 0.0, 296.0, 48.0)));
expect(menuRects[1], equals(const Rect.fromLTRB(80.0, 0.0, 188.0, 48.0)));
expect(menuRects[2], equals(const Rect.fromLTRB(-28.0, 0.0, 80.0, 48.0)));
expect(menuRects[3], equals(const Rect.fromLTRB(-206.0, 0.0, -28.0, 48.0)));
expect(menuRects[4], equals(const Rect.fromLTRB(0.0, 104.0, 214.0, 152.0)));
expect(
collectMenuItemRects(),
equals(const <Rect>[
Rect.fromLTRB(188.0, 0.0, 296.0, 48.0),
Rect.fromLTRB(80.0, 0.0, 188.0, 48.0),
Rect.fromLTRB(-28.0, 0.0, 80.0, 48.0),
Rect.fromLTRB(-206.0, 0.0, -28.0, 48.0),
Rect.fromLTRB(0.0, 104.0, 214.0, 152.0)
]),
);
});

Future<void> buildDensityPaddingApp(WidgetTester tester, {
required TextDirection textDirection,
VisualDensity visualDensity = VisualDensity.standard,
EdgeInsetsGeometry? menuPadding,
}) async {
await tester.pumpWidget(
MaterialApp(
theme: ThemeData.light().copyWith(visualDensity: visualDensity),
home: Directionality(
textDirection: textDirection,
child: Material(
child: Column(
children: <Widget>[
MenuBar(
style: menuPadding != null
? MenuStyle(padding: MaterialStatePropertyAll<EdgeInsetsGeometry>(menuPadding))
: null,
children: createTestMenus(onPressed: onPressed),
),
const Expanded(child: Placeholder()),
],
),
),
),
),
);
await tester.pump();
await tester.tap(find.text(TestMenu.mainMenu1.label));
await tester.pump();
await tester.tap(find.text(TestMenu.subMenu11.label));
await tester.pump();
}

testWidgets('submenus account for density in LTR', (WidgetTester tester) async {
await buildDensityPaddingApp(
tester,
textDirection: TextDirection.ltr,
);
expect(
collectSubmenuRects(),
equals(const <Rect>[
Rect.fromLTRB(145.0, 0.0, 655.0, 48.0),
Rect.fromLTRB(257.0, 48.0, 471.0, 208.0),
Rect.fromLTRB(471.0, 96.0, 719.0, 304.0),
]),
);
});

testWidgets('submenus account for menu density in RTL', (WidgetTester tester) async {
await buildDensityPaddingApp(
tester,
textDirection: TextDirection.rtl,
);
expect(
collectSubmenuRects(),
equals(const <Rect>[
Rect.fromLTRB(145.0, 0.0, 655.0, 48.0),
Rect.fromLTRB(329.0, 48.0, 543.0, 208.0),
Rect.fromLTRB(81.0, 96.0, 329.0, 304.0),
]),
);
});

testWidgets('submenus account for compact menu density in LTR', (WidgetTester tester) async {
await buildDensityPaddingApp(
tester,
visualDensity: VisualDensity.compact,
textDirection: TextDirection.ltr,
);
expect(
collectSubmenuRects(),
equals(const <Rect>[
Rect.fromLTRB(145.0, 0.0, 655.0, 40.0),
Rect.fromLTRB(257.0, 40.0, 467.0, 176.0),
Rect.fromLTRB(467.0, 80.0, 715.0, 256.0),
]),
);
});

testWidgets('submenus account for compact menu density in RTL', (WidgetTester tester) async {
await buildDensityPaddingApp(
tester,
visualDensity: VisualDensity.compact,
textDirection: TextDirection.rtl,
);
expect(
collectSubmenuRects(),
equals(const <Rect>[
Rect.fromLTRB(145.0, 0.0, 655.0, 40.0),
Rect.fromLTRB(333.0, 40.0, 543.0, 176.0),
Rect.fromLTRB(85.0, 80.0, 333.0, 256.0),
]),
);
});

testWidgets('submenus account for padding in LTR', (WidgetTester tester) async {
await buildDensityPaddingApp(
tester,
menuPadding: const EdgeInsetsDirectional.only(start: 10, end: 11, top: 12, bottom: 13),
textDirection: TextDirection.ltr,
);
expect(
collectSubmenuRects(),
equals(const <Rect>[
Rect.fromLTRB(138.5, 0.0, 661.5, 73.0),
Rect.fromLTRB(256.5, 60.0, 470.5, 220.0),
Rect.fromLTRB(470.5, 108.0, 718.5, 316.0),
]),
);
});

testWidgets('submenus account for padding in RTL', (WidgetTester tester) async {
await buildDensityPaddingApp(
tester,
menuPadding: const EdgeInsetsDirectional.only(start: 10, end: 11, top: 12, bottom: 13),
textDirection: TextDirection.rtl,
);
expect(
collectSubmenuRects(),
equals(const <Rect>[
Rect.fromLTRB(138.5, 0.0, 661.5, 73.0),
Rect.fromLTRB(329.5, 60.0, 543.5, 220.0),
Rect.fromLTRB(81.5, 108.0, 329.5, 316.0),
]),
);
});
});

Expand Down

0 comments on commit 577a88b

Please sign in to comment.