From c7fa8e5f7d1a27d502c62de29542f76b565fe240 Mon Sep 17 00:00:00 2001 From: Qun Cheng <36861262+QuncCccccc@users.noreply.github.com> Date: Thu, 2 Mar 2023 10:34:15 -0800 Subject: [PATCH] Revert "Add visual density for menu default style (#114878)" (#121810) This reverts commit 37be384205778c94f0d34e92499ae0766f842f8a. --- dev/tools/gen_defaults/lib/menu_template.dart | 43 +++---- .../flutter/lib/src/material/menu_anchor.dart | 43 +++---- .../test/material/menu_anchor_test.dart | 105 +----------------- 3 files changed, 36 insertions(+), 155 deletions(-) diff --git a/dev/tools/gen_defaults/lib/menu_template.dart b/dev/tools/gen_defaults/lib/menu_template.dart index 99cd923d20..f62dccd5b4 100644 --- a/dev/tools/gen_defaults/lib/menu_template.dart +++ b/dev/tools/gen_defaults/lib/menu_template.dart @@ -43,15 +43,15 @@ class _MenuBarDefaultsM3 extends MenuStyle { @override MaterialStateProperty? get padding { - return const MaterialStatePropertyAll( + return MaterialStatePropertyAll( EdgeInsetsDirectional.symmetric( - horizontal: _kTopLevelMenuHorizontalMinPadding + horizontal: math.max( + _kTopLevelMenuHorizontalMinPadding, + 2 + Theme.of(context).visualDensity.baseSizeAdjustment.dx, + ), ), ); } - - @override - VisualDensity get visualDensity => Theme.of(context).visualDensity; } class _MenuButtonDefaultsM3 extends ButtonStyle { @@ -188,25 +188,10 @@ class _MenuButtonDefaultsM3 extends ButtonStyle { // The horizontal padding number comes from the spec. EdgeInsetsGeometry _scaledPadding(BuildContext context) { - VisualDensity visualDensity = Theme.of(context).visualDensity; - // When horizontal VisualDensity is greater than zero, set it to zero - // because the [ButtonStyleButton] has already handle the padding based on the density. - // However, the [ButtonStyleButton] doesn't allow the [VisualDensity] adjustment - // to reduce the width of the left/right padding, so we need to handle it here if - // the density is less than zero, such as on desktop platforms. - if (visualDensity.horizontal > 0) { - visualDensity = VisualDensity(vertical: visualDensity.vertical); - } return ButtonStyleButton.scaledPadding( - EdgeInsets.symmetric(horizontal: math.max( - _kMenuViewPadding, - _kLabelItemDefaultSpacing + visualDensity.baseSizeAdjustment.dx, - )), - EdgeInsets.symmetric(horizontal: math.max( - _kMenuViewPadding, - 8 + visualDensity.baseSizeAdjustment.dx, - )), - const EdgeInsets.symmetric(horizontal: _kMenuViewPadding), + const EdgeInsets.symmetric(horizontal: 12), + const EdgeInsets.symmetric(horizontal: 8), + const EdgeInsets.symmetric(horizontal: 4), MediaQuery.maybeTextScaleFactorOf(context) ?? 1, ); } @@ -244,13 +229,15 @@ class _MenuDefaultsM3 extends MenuStyle { @override MaterialStateProperty? get padding { - return const MaterialStatePropertyAll( - EdgeInsetsDirectional.symmetric(vertical: _kMenuVerticalMinPadding), + return MaterialStatePropertyAll( + EdgeInsetsDirectional.symmetric( + vertical: math.max( + _kMenuVerticalMinPadding, + 2 + Theme.of(context).visualDensity.baseSizeAdjustment.dy, + ), + ), ); } - - @override - VisualDensity get visualDensity => Theme.of(context).visualDensity; } '''; } diff --git a/packages/flutter/lib/src/material/menu_anchor.dart b/packages/flutter/lib/src/material/menu_anchor.dart index 6359377373..0024c492ac 100644 --- a/packages/flutter/lib/src/material/menu_anchor.dart +++ b/packages/flutter/lib/src/material/menu_anchor.dart @@ -3619,15 +3619,15 @@ class _MenuBarDefaultsM3 extends MenuStyle { @override MaterialStateProperty? get padding { - return const MaterialStatePropertyAll( + return MaterialStatePropertyAll( EdgeInsetsDirectional.symmetric( - horizontal: _kTopLevelMenuHorizontalMinPadding + horizontal: math.max( + _kTopLevelMenuHorizontalMinPadding, + 2 + Theme.of(context).visualDensity.baseSizeAdjustment.dx, + ), ), ); } - - @override - VisualDensity get visualDensity => Theme.of(context).visualDensity; } class _MenuButtonDefaultsM3 extends ButtonStyle { @@ -3764,25 +3764,10 @@ class _MenuButtonDefaultsM3 extends ButtonStyle { // The horizontal padding number comes from the spec. EdgeInsetsGeometry _scaledPadding(BuildContext context) { - VisualDensity visualDensity = Theme.of(context).visualDensity; - // When horizontal VisualDensity is greater than zero, set it to zero - // because the [ButtonStyleButton] has already handle the padding based on the density. - // However, the [ButtonStyleButton] doesn't allow the [VisualDensity] adjustment - // to reduce the width of the left/right padding, so we need to handle it here if - // the density is less than zero, such as on desktop platforms. - if (visualDensity.horizontal > 0) { - visualDensity = VisualDensity(vertical: visualDensity.vertical); - } return ButtonStyleButton.scaledPadding( - EdgeInsets.symmetric(horizontal: math.max( - _kMenuViewPadding, - _kLabelItemDefaultSpacing + visualDensity.baseSizeAdjustment.dx, - )), - EdgeInsets.symmetric(horizontal: math.max( - _kMenuViewPadding, - 8 + visualDensity.baseSizeAdjustment.dx, - )), - const EdgeInsets.symmetric(horizontal: _kMenuViewPadding), + const EdgeInsets.symmetric(horizontal: 12), + const EdgeInsets.symmetric(horizontal: 8), + const EdgeInsets.symmetric(horizontal: 4), MediaQuery.maybeTextScaleFactorOf(context) ?? 1, ); } @@ -3820,13 +3805,15 @@ class _MenuDefaultsM3 extends MenuStyle { @override MaterialStateProperty? get padding { - return const MaterialStatePropertyAll( - EdgeInsetsDirectional.symmetric(vertical: _kMenuVerticalMinPadding), + return MaterialStatePropertyAll( + EdgeInsetsDirectional.symmetric( + vertical: math.max( + _kMenuVerticalMinPadding, + 2 + Theme.of(context).visualDensity.baseSizeAdjustment.dy, + ), + ), ); } - - @override - VisualDensity get visualDensity => Theme.of(context).visualDensity; } // END GENERATED TOKEN PROPERTIES - Menu diff --git a/packages/flutter/test/material/menu_anchor_test.dart b/packages/flutter/test/material/menu_anchor_test.dart index ab24b201d5..4fa005b5db 100644 --- a/packages/flutter/test/material/menu_anchor_test.dart +++ b/packages/flutter/test/material/menu_anchor_test.dart @@ -147,99 +147,6 @@ void main() { ); } - testWidgets('Menu responds to density changes', (WidgetTester tester) async { - Widget buildMenu({VisualDensity? visualDensity = VisualDensity.standard}) => MaterialApp( - theme: ThemeData(visualDensity: visualDensity), - home: Material( - child: Column( - children: [ - MenuBar( - children: createTestMenus(onPressed: onPressed), - ), - const Expanded(child: Placeholder()), - ], - ), - ), - ); - - await tester.pumpWidget(buildMenu()); - await tester.pump(); - - expect(tester.getRect(find.byType(MenuBar)), equals(const Rect.fromLTRB(145.0, 0.0, 655.0, 48.0))); - - // Open and make sure things are the right size. - await tester.tap(find.text(TestMenu.mainMenu1.label)); - await tester.pump(); - - expect(tester.getRect(find.byType(MenuBar)), equals(const Rect.fromLTRB(145.0, 0.0, 655.0, 48.0))); - expect( - tester.getRect(find.widgetWithText(MenuItemButton, TestMenu.subMenu10.label)), - equals(const Rect.fromLTRB(257.0, 56.0, 471.0, 104.0)), - ); - expect( - tester.getRect( - find.ancestor(of: find.text(TestMenu.subMenu10.label), matching: find.byType(Material)).at(1), - ), - equals(const Rect.fromLTRB(257.0, 48.0, 471.0, 208.0)), - ); - - // Test compact visual density (-2, -2) - await tester.pumpWidget(Container()); - await tester.pumpWidget(buildMenu(visualDensity: VisualDensity.compact)); - await tester.pump(); - - // The original horizontal padding with standard visual density for menu buttons are 12 px, and the total length - // for the menu bar is (655 - 145) = 510. - // There are 4 buttons in the test menu bar, and with compact visual density, - // the padding will reduce by abs(2 * (-2)) = 4. So the total length - // now should reduce by abs(4 * 2 * (-4)) = 32, which would be 510 - 32 = 478, and - // 478 = 639 - 161 - expect(tester.getRect(find.byType(MenuBar)), equals(const Rect.fromLTRB(161.0, 0.0, 639.0, 40.0))); - - // Open and make sure things are the right size. - await tester.tap(find.text(TestMenu.mainMenu1.label)); - await tester.pump(); - - expect(tester.getRect(find.byType(MenuBar)), equals(const Rect.fromLTRB(161.0, 0.0, 639.0, 40.0))); - expect( - tester.getRect(find.widgetWithText(MenuItemButton, TestMenu.subMenu10.label)), - equals(const Rect.fromLTRB(265.0, 40.0, 467.0, 80.0)), - ); - expect( - tester.getRect( - find.ancestor(of: find.text(TestMenu.subMenu10.label), matching: find.byType(Material)).at(1), - ), - equals(const Rect.fromLTRB(265.0, 40.0, 467.0, 160.0)), - ); - - await tester.pumpWidget(Container()); - await tester.pumpWidget(buildMenu(visualDensity: const VisualDensity(horizontal: 2.0, vertical: 2.0))); - await tester.pump(); - - // Similarly, there are 4 buttons in the test menu bar, and with (2, 2) visual density, - // the padding will increase by abs(2 * 4) = 8. So the total length for buttons - // should increase by abs(4 * 2 * 8) = 64. The horizontal padding for the menu bar - // increases by 2 * 8, so the total width increases to 510 + 64 + 16 = 590, and - // 590 = 695 - 105 - expect(tester.getRect(find.byType(MenuBar)), equals(const Rect.fromLTRB(105.0, 0.0, 695.0, 72.0))); - - // Open and make sure things are the right size. - await tester.tap(find.text(TestMenu.mainMenu1.label)); - await tester.pump(); - - expect(tester.getRect(find.byType(MenuBar)), equals(const Rect.fromLTRB(105.0, 0.0, 695.0, 72.0))); - expect( - tester.getRect(find.widgetWithText(MenuItemButton, TestMenu.subMenu10.label)), - equals(const Rect.fromLTRB(249.0, 80.0, 483.0, 136.0)), - ); - expect( - tester.getRect( - find.ancestor(of: find.text(TestMenu.subMenu10.label), matching: find.byType(Material)).at(1), - ), - equals(const Rect.fromLTRB(241.0, 64.0, 491.0, 264.0)), - ); - }); - testWidgets('menu defaults colors', (WidgetTester tester) async { final ThemeData themeData = ThemeData(); await tester.pumpWidget( @@ -2275,9 +2182,9 @@ void main() { expect( collectSubmenuRects(), equals(const [ - Rect.fromLTRB(161.0, 0.0, 639.0, 40.0), - Rect.fromLTRB(265.0, 40.0, 467.0, 160.0), - Rect.fromLTRB(467.0, 72.0, 707.0, 232.0), + 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), ]), ); }); @@ -2291,9 +2198,9 @@ void main() { expect( collectSubmenuRects(), equals(const [ - Rect.fromLTRB(161.0, 0.0, 639.0, 40.0), - Rect.fromLTRB(333.0, 40.0, 535.0, 160.0), - Rect.fromLTRB(93.0, 72.0, 333.0, 232.0), + 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), ]), ); });