From 4b39f071f3f992b718182c05367ccf539bbfe997 Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Tue, 18 Apr 2023 14:09:53 -0700 Subject: [PATCH] Add controller argument to SubmenuButton (#125000) ## Description This adds an optional argument to the `SubmenuButton` that allows the creator to supply a `MenuController` for controlling the menu. ## Related Issues - Fixes https://github.com/flutter/flutter/issues/124988 ## Tests - Added tests for new argument. --- .../flutter/lib/src/material/menu_anchor.dart | 36 +- .../test/material/menu_anchor_test.dart | 382 ++++++++++-------- 2 files changed, 247 insertions(+), 171 deletions(-) diff --git a/packages/flutter/lib/src/material/menu_anchor.dart b/packages/flutter/lib/src/material/menu_anchor.dart index 793f7cad479..66348b0044b 100644 --- a/packages/flutter/lib/src/material/menu_anchor.dart +++ b/packages/flutter/lib/src/material/menu_anchor.dart @@ -1548,6 +1548,7 @@ class SubmenuButton extends StatefulWidget { this.onFocusChange, this.onOpen, this.onClose, + this.controller, this.style, this.menuStyle, this.alignmentOffset, @@ -1578,6 +1579,9 @@ class SubmenuButton extends StatefulWidget { /// A callback that is invoked when the menu is closed. final VoidCallback? onClose; + /// An optional [MenuController] for this submenu. + final MenuController? controller; + /// Customizes this button's appearance. /// /// Non-null properties of this style override the corresponding properties in @@ -1760,7 +1764,8 @@ class SubmenuButton extends StatefulWidget { class _SubmenuButtonState extends State { FocusNode? _internalFocusNode; bool _waitingToFocusMenu = false; - final MenuController _menuController = MenuController(); + MenuController? _internalMenuController; + MenuController get _menuController => widget.controller ?? _internalMenuController!; _MenuAnchorState? get _anchor => _MenuAnchorState._maybeOf(context); FocusNode get _buttonFocusNode => widget.focusNode ?? _internalFocusNode!; bool get _enabled => widget.menuChildren.isNotEmpty; @@ -1777,12 +1782,15 @@ class _SubmenuButtonState extends State { return true; }()); } + if (widget.controller == null) { + _internalMenuController = MenuController(); + } _buttonFocusNode.addListener(_handleFocusChange); } @override void dispose() { - _internalFocusNode?.removeListener(_handleFocusChange); + _buttonFocusNode.removeListener(_handleFocusChange); _internalFocusNode?.dispose(); _internalFocusNode = null; super.dispose(); @@ -1810,6 +1818,9 @@ class _SubmenuButtonState extends State { } _buttonFocusNode.addListener(_handleFocusChange); } + if (widget.controller != oldWidget.controller) { + _internalMenuController = (oldWidget.controller == null) ? null : MenuController(); + } } @override @@ -1836,7 +1847,16 @@ class _SubmenuButtonState extends State { alignmentOffset: menuPaddingOffset, clipBehavior: widget.clipBehavior, onClose: widget.onClose, - onOpen: widget.onOpen, + onOpen: () { + if (!_waitingToFocusMenu) { + SchedulerBinding.instance.addPostFrameCallback((_) { + _menuController._anchor?._focusButton(); + _waitingToFocusMenu = false; + }); + _waitingToFocusMenu = true; + } + widget.onOpen?.call(); + }, style: widget.menuStyle, builder: (BuildContext context, MenuController controller, Widget? child) { // Since we don't want to use the theme style or default style from the @@ -1857,16 +1877,6 @@ class _SubmenuButtonState extends State { controller.close(); } else { controller.open(); - if (!_waitingToFocusMenu) { - // Only schedule this if it's not already scheduled. - SchedulerBinding.instance.addPostFrameCallback((Duration _) { - // This has to happen in the next frame because the menu bar is - // not focusable until the first menu is open. - controller._anchor?._focusButton(); - _waitingToFocusMenu = false; - }); - _waitingToFocusMenu = true; - } } } diff --git a/packages/flutter/test/material/menu_anchor_test.dart b/packages/flutter/test/material/menu_anchor_test.dart index 0acd34706ba..0703202aae5 100644 --- a/packages/flutter/test/material/menu_anchor_test.dart +++ b/packages/flutter/test/material/menu_anchor_test.dart @@ -142,19 +142,21 @@ 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()), - ], + Widget buildMenu({VisualDensity? visualDensity = VisualDensity.standard}) { + return 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(); @@ -947,27 +949,27 @@ void main() { testWidgets('MenuAnchor clip behavior', (WidgetTester tester) async { await tester.pumpWidget( - MaterialApp( - home: Material( - child: Center( - child: MenuAnchor( - menuChildren: const [ - MenuItemButton( - child: Text('Button 1'), - ), - ], - builder: (BuildContext context, MenuController controller, Widget? child) { - return FilledButton( - onPressed: () { - controller.open(); - }, - child: const Text('Tap me'), - ); - }, - ), - ) - ) - ) + MaterialApp( + home: Material( + child: Center( + child: MenuAnchor( + menuChildren: const [ + MenuItemButton( + child: Text('Button 1'), + ), + ], + builder: (BuildContext context, MenuController controller, Widget? child) { + return FilledButton( + onPressed: () { + controller.open(); + }, + child: const Text('Tap me'), + ); + }, + ), + ), + ), + ), ); await tester.tap(find.text('Tap me')); await tester.pump(); @@ -977,28 +979,28 @@ void main() { await tester.tapAt(const Offset(10.0, 10.0)); await tester.pumpAndSettle(); await tester.pumpWidget( - MaterialApp( - home: Material( - child: Center( - child: MenuAnchor( - clipBehavior: Clip.antiAlias, - menuChildren: const [ - MenuItemButton( - child: Text('Button 1'), - ), - ], - builder: (BuildContext context, MenuController controller, Widget? child) { - return FilledButton( - onPressed: () { - controller.open(); - }, - child: const Text('Tap me'), - ); - }, - ), - ) - ) - ) + MaterialApp( + home: Material( + child: Center( + child: MenuAnchor( + clipBehavior: Clip.antiAlias, + menuChildren: const [ + MenuItemButton( + child: Text('Button 1'), + ), + ], + builder: (BuildContext context, MenuController controller, Widget? child) { + return FilledButton( + onPressed: () { + controller.open(); + }, + child: const Text('Tap me'), + ); + }, + ), + ), + ), + ), ); await tester.tap(find.text('Tap me')); await tester.pump(); @@ -1589,14 +1591,23 @@ void main() { int acceleratorIndex = -1; int count = 0; for (final String key in expected.keys) { - expect(MenuAcceleratorLabel.stripAcceleratorMarkers(key, setIndex: (int index) { + expect( + MenuAcceleratorLabel.stripAcceleratorMarkers(key, setIndex: (int index) { acceleratorIndex = index; - }), equals(expected[key]), - reason: "'$key' label doesn't match ${expected[key]}"); - expect(acceleratorIndex, equals(expectedIndices[count]), - reason: "'$key' index doesn't match ${expectedIndices[count]}"); - expect(MenuAcceleratorLabel(key).hasAccelerator, equals(expectedHasAccelerator[count]), - reason: "'$key' hasAccelerator isn't ${expectedHasAccelerator[count]}"); + }), + equals(expected[key]), + reason: "'$key' label doesn't match ${expected[key]}", + ); + expect( + acceleratorIndex, + equals(expectedIndices[count]), + reason: "'$key' index doesn't match ${expectedIndices[count]}", + ); + expect( + MenuAcceleratorLabel(key).hasAccelerator, + equals(expectedHasAccelerator[count]), + reason: "'$key' hasAccelerator isn't ${expectedHasAccelerator[count]}", + ); count += 1; } }); @@ -2064,6 +2075,63 @@ void main() { expect(find.text('trailingIcon'), findsOneWidget); }); + testWidgets('SubmenuButton uses supplied controller', (WidgetTester tester) async { + final MenuController submenuController = MenuController(); + await tester.pumpWidget( + MaterialApp( + home: Material( + child: MenuBar( + controller: controller, + children: [ + SubmenuButton( + controller: submenuController, + menuChildren: [ + MenuItemButton( + child: Text(TestMenu.subMenu00.label), + ), + ], + child: Text(TestMenu.mainMenu0.label), + ), + ], + ), + ), + ), + ); + + submenuController.open(); + await tester.pump(); + expect(find.text(TestMenu.subMenu00.label), findsOneWidget); + + submenuController.close(); + await tester.pump(); + expect(find.text(TestMenu.subMenu00.label), findsNothing); + + // Now remove the controller and try to control it. + await tester.pumpWidget( + MaterialApp( + home: Material( + child: MenuBar( + controller: controller, + children: [ + SubmenuButton( + menuChildren: [ + MenuItemButton( + child: Text(TestMenu.subMenu00.label), + ), + ], + child: Text(TestMenu.mainMenu0.label), + ), + ], + ), + ), + ), + ); + + await expectLater(() => submenuController.open(), throwsAssertionError); + await tester.pump(); + expect(find.text(TestMenu.subMenu00.label), findsNothing); + }); + testWidgets('diagnostics', (WidgetTester tester) async { final ButtonStyle style = ButtonStyle( shape: MaterialStateProperty.all(const StadiumBorder()), @@ -2123,6 +2191,78 @@ void main() { ), ); }); + + testWidgets('MenuItemButton respects closeOnActivate property', (WidgetTester tester) async { + final MenuController controller = MenuController(); + await tester.pumpWidget(MaterialApp( + home: Material( + child: Center( + child: MenuAnchor( + controller: controller, + menuChildren: [ + MenuItemButton( + onPressed: () {}, + child: const Text('Button 1'), + ), + ], + builder: (BuildContext context, MenuController controller, Widget? child) { + return FilledButton( + onPressed: () { + controller.open(); + }, + child: const Text('Tap me'), + ); + }, + ), + ), + ), + )); + + await tester.tap(find.text('Tap me')); + await tester.pump(); + expect(find.byType(MenuItemButton), findsNWidgets(1)); + + // Taps the MenuItemButton which should close the menu + await tester.tap(find.text('Button 1')); + await tester.pump(); + expect(find.byType(MenuItemButton), findsNWidgets(0)); + + await tester.pumpAndSettle(); + + await tester.pumpWidget(MaterialApp( + home: Material( + child: Center( + child: MenuAnchor( + controller: controller, + menuChildren: [ + MenuItemButton( + closeOnActivate: false, + onPressed: () {}, + child: const Text('Button 1'), + ), + ], + builder: (BuildContext context, MenuController controller, Widget? child) { + return FilledButton( + onPressed: () { + controller.open(); + }, + child: const Text('Tap me'), + ); + }, + ), + ), + ), + )); + + await tester.tap(find.text('Tap me')); + await tester.pump(); + expect(find.byType(MenuItemButton), findsNWidgets(1)); + + // Taps the MenuItemButton which shouldn't close the menu + await tester.tap(find.text('Button 1')); + await tester.pump(); + expect(find.byType(MenuItemButton), findsNWidgets(1)); + }); }); group('Layout', () { @@ -2338,18 +2478,17 @@ void main() { child: Align( alignment: Alignment.topLeft, child: MenuAnchor( - menuChildren: const [ + menuChildren: const [ SubmenuButton( alignmentOffset: Offset(10, 0), - menuChildren: [ + menuChildren: [ SubmenuButton( - menuChildren: [ + menuChildren: [ SubmenuButton( alignmentOffset: Offset(10, 0), - menuChildren: [ + menuChildren: [ SubmenuButton( - menuChildren: [ - ], + menuChildren: [], child: Text('SubMenuButton4'), ), ], @@ -2415,18 +2554,17 @@ void main() { child: Align( alignment: Alignment.topRight, child: MenuAnchor( - menuChildren: const [ + menuChildren: const [ SubmenuButton( alignmentOffset: Offset(10, 0), - menuChildren: [ + menuChildren: [ SubmenuButton( - menuChildren: [ + menuChildren: [ SubmenuButton( alignmentOffset: Offset(10, 0), - menuChildren: [ + menuChildren: [ SubmenuButton( - menuChildren: [ - ], + menuChildren: [], child: Text('SubMenuButton4'), ), ], @@ -2492,8 +2630,9 @@ void main() { child: Align( alignment: Alignment.bottomLeft, child: MenuAnchor( - menuChildren: const [ - MenuItemButton(child: Text('Button1'), + menuChildren: const [ + MenuItemButton( + child: Text('Button1'), ), ], builder: (BuildContext context, MenuController controller, Widget? child) { @@ -2530,7 +2669,8 @@ void main() { ); }); - testWidgets('vertically constrained menus are positioned above the anchor with the provided offset', (WidgetTester tester) async { + testWidgets('vertically constrained menus are positioned above the anchor with the provided offset', + (WidgetTester tester) async { await changeSurfaceSize(tester, const Size(800, 600)); await tester.pumpWidget( MaterialApp( @@ -2542,8 +2682,9 @@ void main() { alignment: Alignment.bottomLeft, child: MenuAnchor( alignmentOffset: const Offset(0, 50), - menuChildren: const [ - MenuItemButton(child: Text('Button1'), + menuChildren: const [ + MenuItemButton( + child: Text('Button1'), ), ], builder: (BuildContext context, MenuController controller, Widget? child) { @@ -2580,7 +2721,8 @@ void main() { ); }); - Future buildDensityPaddingApp(WidgetTester tester, { + Future buildDensityPaddingApp( + WidgetTester tester, { required TextDirection textDirection, VisualDensity visualDensity = VisualDensity.standard, EdgeInsetsGeometry? menuPadding, @@ -2595,8 +2737,8 @@ void main() { children: [ MenuBar( style: menuPadding != null - ? MenuStyle(padding: MaterialStatePropertyAll(menuPadding)) - : null, + ? MenuStyle(padding: MaterialStatePropertyAll(menuPadding)) + : null, children: createTestMenus(onPressed: onPressed), ), const Expanded(child: Placeholder()), @@ -2898,82 +3040,6 @@ void main() { expect(radioValue, 1); }); }); - - testWidgets('MenuItemButton respects closeOnActivate property', (WidgetTester tester) async { - final MenuController controller = MenuController(); - await tester.pumpWidget( - MaterialApp( - home: Material( - child: Center( - child: MenuAnchor( - controller: controller, - menuChildren: [ - MenuItemButton( - onPressed: () {}, - child: const Text('Button 1'), - ), - ], - builder: (BuildContext context, MenuController controller, Widget? child) { - return FilledButton( - onPressed: () { - controller.open(); - }, - child: const Text('Tap me'), - ); - }, - ), - ), - ), - ) - ); - - await tester.tap(find.text('Tap me')); - await tester.pump(); - expect(find.byType(MenuItemButton), findsNWidgets(1)); - - // Taps the MenuItemButton which should close the menu - await tester.tap(find.text('Button 1')); - await tester.pump(); - expect(find.byType(MenuItemButton), findsNWidgets(0)); - - await tester.pumpAndSettle(); - - await tester.pumpWidget( - MaterialApp( - home: Material( - child: Center( - child: MenuAnchor( - controller: controller, - menuChildren: [ - MenuItemButton( - closeOnActivate: false, - onPressed: () {}, - child: const Text('Button 1'), - ), - ], - builder: (BuildContext context, MenuController controller, Widget? child) { - return FilledButton( - onPressed: () { - controller.open(); - }, - child: const Text('Tap me'), - ); - }, - ), - ), - ), - ) - ); - - await tester.tap(find.text('Tap me')); - await tester.pump(); - expect(find.byType(MenuItemButton), findsNWidgets(1)); - - // Taps the MenuItemButton which shouldn't close the menu - await tester.tap(find.text('Button 1')); - await tester.pump(); - expect(find.byType(MenuItemButton), findsNWidgets(1)); - }); } List createTestMenus({