Fix MenuAnchor padding (#116573)

* Fix MenuAnchor padding

* Add tests

packages/flutter/lib/src/material/menu_anchor.dart

@@ -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;
     }
 
@@ -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() {
@@ -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) {
@@ -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(
@@ -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;

packages/flutter/test/material/menu_anchor_test.dart

@@ -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) {
@@ -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(
@@ -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 {
@@ -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 {
@@ -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 {
@@ -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),
+        ]),
+      );
     });
   });