Add applyFocusChangeIfNeeded, have menus restore focus before activating (#130536)
## Description
This modifies the `MenuAnchor` `onPressed` activation to delay until after the current frame is built, and resolve any focus changes before it invokes the `onPressed`, so that actions that operate on the `primaryFocus` can have a chance of working on the focused item they were meant to work on.
## Related Issues
- Fixes https://github.com/flutter/flutter/issues/118731
## Tests
- No tests yet (hence draft still)
diff --git a/dev/manual_tests/lib/menu_anchor.dart b/dev/manual_tests/lib/menu_anchor.dart
index b6f98d3..11a5ebd 100644
--- a/dev/manual_tests/lib/menu_anchor.dart
+++ b/dev/manual_tests/lib/menu_anchor.dart
@@ -706,6 +706,12 @@
TestMenu.mainMenu3,
menuChildren: <Widget>[
menuItemButton(TestMenu.subMenu8),
+ MenuItemButton(
+ onPressed: () {
+ debugPrint('Focused Item: $primaryFocus');
+ },
+ child: const Text('Print Focused Item'),
+ )
],
),
submenuButton(
@@ -734,7 +740,11 @@
submenuButton(
TestMenu.subSubMenu3,
menuChildren: <Widget>[
- menuItemButton(TestMenu.subSubSubMenu1),
+ for (int i=0; i < 100; ++i)
+ MenuItemButton(
+ onPressed: () {},
+ child: Text('Menu Item $i'),
+ ),
],
),
],
diff --git a/examples/api/test/material/menu_anchor/checkbox_menu_button.0_test.dart b/examples/api/test/material/menu_anchor/checkbox_menu_button.0_test.dart
index 278d19a..76d27b5 100644
--- a/examples/api/test/material/menu_anchor/checkbox_menu_button.0_test.dart
+++ b/examples/api/test/material/menu_anchor/checkbox_menu_button.0_test.dart
@@ -13,13 +13,13 @@
);
await tester.tap(find.byType(TextButton));
- await tester.pump();
+ await tester.pumpAndSettle();
expect(find.text('Show Message'), findsOneWidget);
expect(find.text(example.MenuApp.kMessage), findsNothing);
await tester.tap(find.text('Show Message'));
- await tester.pump();
+ await tester.pumpAndSettle();
expect(find.text('Show Message'), findsNothing);
expect(find.text(example.MenuApp.kMessage), findsOneWidget);
diff --git a/examples/api/test/material/menu_anchor/menu_anchor.0_test.dart b/examples/api/test/material/menu_anchor/menu_anchor.0_test.dart
index 54016dc..dcbabb2 100644
--- a/examples/api/test/material/menu_anchor/menu_anchor.0_test.dart
+++ b/examples/api/test/material/menu_anchor/menu_anchor.0_test.dart
@@ -41,7 +41,7 @@
await tester.sendKeyEvent(LogicalKeyboardKey.arrowUp);
await tester.sendKeyEvent(LogicalKeyboardKey.enter);
- await tester.pump();
+ await tester.pumpAndSettle();
expect(find.text(example.MenuApp.kMessage), findsOneWidget);
expect(find.text('Last Selected: ${example.MenuEntry.showMessage.label}'), findsOneWidget);
diff --git a/examples/api/test/material/menu_anchor/menu_anchor.1_test.dart b/examples/api/test/material/menu_anchor/menu_anchor.1_test.dart
index b9dc670..0ac0e43 100644
--- a/examples/api/test/material/menu_anchor/menu_anchor.1_test.dart
+++ b/examples/api/test/material/menu_anchor/menu_anchor.1_test.dart
@@ -20,7 +20,7 @@
await tester.pumpWidget(const example.ContextMenuApp());
await tester.tapAt(const Offset(100, 200), buttons: kSecondaryButton);
- await tester.pump();
+ await tester.pumpAndSettle();
expect(tester.getRect(findMenu()), equals(const Rect.fromLTRB(100.0, 200.0, 433.0, 360.0)));
// Make sure tapping in a different place causes the menu to move.
@@ -46,7 +46,7 @@
expect(find.text('Background Color'), findsOneWidget);
await tester.tap(find.text('Background Color'));
- await tester.pump();
+ await tester.pumpAndSettle();
expect(find.text(example.MenuEntry.colorRed.label), findsOneWidget);
expect(find.text(example.MenuEntry.colorGreen.label), findsOneWidget);
@@ -54,7 +54,7 @@
await tester.sendKeyEvent(LogicalKeyboardKey.arrowUp);
await tester.sendKeyEvent(LogicalKeyboardKey.enter);
- await tester.pump();
+ await tester.pumpAndSettle();
expect(find.text(example.ContextMenuApp.kMessage), findsOneWidget);
expect(find.text('Last Selected: ${example.MenuEntry.showMessage.label}'), findsOneWidget);
diff --git a/examples/api/test/material/menu_anchor/menu_bar.0_test.dart b/examples/api/test/material/menu_anchor/menu_bar.0_test.dart
index b508ba4..ccfe4c9 100644
--- a/examples/api/test/material/menu_anchor/menu_bar.0_test.dart
+++ b/examples/api/test/material/menu_anchor/menu_bar.0_test.dart
@@ -20,7 +20,7 @@
final Finder menuButtonFinder = find.byType(SubmenuButton).first;
await tester.tap(menuButtonFinder);
- await tester.pump();
+ await tester.pumpAndSettle();
expect(find.text('About'), findsOneWidget);
expect(find.text('Show Message'), findsOneWidget);
@@ -34,7 +34,7 @@
await tester.sendKeyEvent(LogicalKeyboardKey.arrowDown);
await tester.sendKeyEvent(LogicalKeyboardKey.arrowDown);
await tester.sendKeyEvent(LogicalKeyboardKey.arrowDown);
- await tester.pump();
+ await tester.pumpAndSettle();
expect(find.text('About'), findsOneWidget);
expect(find.text('Show Message'), findsOneWidget);
@@ -46,7 +46,7 @@
await tester.sendKeyEvent(LogicalKeyboardKey.arrowUp);
await tester.sendKeyEvent(LogicalKeyboardKey.enter);
- await tester.pump();
+ await tester.pumpAndSettle();
expect(find.text(example.MenuBarApp.kMessage), findsOneWidget);
expect(find.text('Last Selected: Show Message'), findsOneWidget);
diff --git a/examples/api/test/material/menu_anchor/radio_menu_button.0_test.dart b/examples/api/test/material/menu_anchor/radio_menu_button.0_test.dart
index ee33556..2b88a77 100644
--- a/examples/api/test/material/menu_anchor/radio_menu_button.0_test.dart
+++ b/examples/api/test/material/menu_anchor/radio_menu_button.0_test.dart
@@ -24,7 +24,7 @@
expect(tester.widget<Container>(find.byType(Container)).color, equals(Colors.red));
await tester.tap(find.text('Green Background'));
- await tester.pump();
+ await tester.pumpAndSettle();
expect(tester.widget<Container>(find.byType(Container)).color, equals(Colors.green));
});
diff --git a/packages/flutter/lib/src/material/menu_anchor.dart b/packages/flutter/lib/src/material/menu_anchor.dart
index 7980783..37aca52 100644
--- a/packages/flutter/lib/src/material/menu_anchor.dart
+++ b/packages/flutter/lib/src/material/menu_anchor.dart
@@ -1099,10 +1099,16 @@
void _handleSelect() {
assert(_debugMenuInfo('Selected ${widget.child} menu'));
- widget.onPressed?.call();
if (widget.closeOnActivate) {
_MenuAnchorState._maybeOf(context)?._root._close();
}
+ // Delay the call to onPressed until post-frame so that the focus is
+ // restored to what it was before the menu was opened before the action is
+ // executed.
+ SchedulerBinding.instance.addPostFrameCallback((Duration _) {
+ FocusManager.instance.applyFocusChangesIfNeeded();
+ widget.onPressed?.call();
+ });
}
void _createInternalFocusNodeIfNeeded() {
diff --git a/packages/flutter/lib/src/widgets/focus_manager.dart b/packages/flutter/lib/src/widgets/focus_manager.dart
index b167ed1..f534b99 100644
--- a/packages/flutter/lib/src/widgets/focus_manager.dart
+++ b/packages/flutter/lib/src/widgets/focus_manager.dart
@@ -8,6 +8,7 @@
import 'package:flutter/foundation.dart';
import 'package:flutter/gestures.dart';
import 'package:flutter/painting.dart';
+import 'package:flutter/scheduler.dart';
import 'package:flutter/services.dart';
import 'binding.dart';
@@ -1601,10 +1602,32 @@
return;
}
_haveScheduledUpdate = true;
- scheduleMicrotask(_applyFocusChange);
+ scheduleMicrotask(applyFocusChangesIfNeeded);
}
- void _applyFocusChange() {
+ /// Applies any pending focus changes and notifies listeners that the focus
+ /// has changed.
+ ///
+ /// Must not be called during the build phase. This method is meant to be
+ /// called in a post-frame callback or microtask when the pending focus
+ /// changes need to be resolved before something else occurs.
+ ///
+ /// It can't be called during the build phase because not all listeners are
+ /// safe to be called with an update during a build.
+ ///
+ /// Typically, this is called automatically by the [FocusManager], but
+ /// sometimes it is necessary to ensure that no focus changes are pending
+ /// before executing an action. For example, the [MenuAnchor] class uses this
+ /// to make sure that the previous focus has been restored before executing a
+ /// menu callback when a menu item is selected.
+ ///
+ /// It is safe to call this if no focus changes are pending.
+ void applyFocusChangesIfNeeded() {
+ assert(
+ SchedulerBinding.instance.schedulerPhase != SchedulerPhase.persistentCallbacks,
+ 'applyFocusChangesIfNeeded() should not be called during the build phase.'
+ );
+
_haveScheduledUpdate = false;
final FocusNode? previousFocus = _primaryFocus;
diff --git a/packages/flutter/test/material/menu_anchor_test.dart b/packages/flutter/test/material/menu_anchor_test.dart
index e48e855..6de3e26 100644
--- a/packages/flutter/test/material/menu_anchor_test.dart
+++ b/packages/flutter/test/material/menu_anchor_test.dart
@@ -486,6 +486,53 @@
);
}, variant: TargetPlatformVariant.desktop());
+ testWidgets('focus is returned to previous focus before invoking onPressed', (WidgetTester tester) async {
+ final FocusNode buttonFocus = FocusNode(debugLabel: 'Button Focus');
+ FocusNode? focusInOnPressed;
+
+ void onMenuSelected(TestMenu item) {
+ focusInOnPressed = FocusManager.instance.primaryFocus;
+ }
+
+ await tester.pumpWidget(
+ MaterialApp(
+ home: Material(
+ child: Column(
+ children: <Widget>[
+ MenuBar(
+ controller: controller,
+ children: createTestMenus(
+ onPressed: onMenuSelected,
+ ),
+ ),
+ ElevatedButton(
+ autofocus: true,
+ onPressed: () {},
+ focusNode: buttonFocus,
+ child: const Text('Press Me'),
+ ),
+ ],
+ ),
+ ),
+ ),
+ );
+
+ await tester.pump();
+ expect(FocusManager.instance.primaryFocus, equals(buttonFocus));
+
+ await tester.tap(find.text(TestMenu.mainMenu1.label));
+ await tester.pump();
+
+ await tester.tap(find.text(TestMenu.subMenu11.label));
+ await tester.pump();
+
+ await tester.tap(find.text(TestMenu.subSubMenu110.label));
+ await tester.pump();
+
+ expect(focusInOnPressed, equals(buttonFocus));
+ expect(FocusManager.instance.primaryFocus, equals(buttonFocus));
+ });
+
group('Menu functions', () {
testWidgets('basic menu structure', (WidgetTester tester) async {
await tester.pumpWidget(
@@ -3064,7 +3111,7 @@
child: Center(
child: MenuItemButton(
style: MenuItemButton.styleFrom(fixedSize: const Size(88.0, 36.0)),
- onPressed: () { },
+ onPressed: () {},
child: const Text('ABC'),
),
),
@@ -3072,27 +3119,30 @@
);
// The flags should not have SemanticsFlag.isButton
- expect(semantics, hasSemantics(
- TestSemantics.root(
- children: <TestSemantics>[
- TestSemantics.rootChild(
- actions: <SemanticsAction>[
- SemanticsAction.tap,
- ],
- label: 'ABC',
- rect: const Rect.fromLTRB(0.0, 0.0, 88.0, 48.0),
- transform: Matrix4.translationValues(356.0, 276.0, 0.0),
- flags: <SemanticsFlag>[
- SemanticsFlag.hasEnabledState,
- SemanticsFlag.isEnabled,
- SemanticsFlag.isFocusable,
- ],
- textDirection: TextDirection.ltr,
- ),
- ],
+ expect(
+ semantics,
+ hasSemantics(
+ TestSemantics.root(
+ children: <TestSemantics>[
+ TestSemantics.rootChild(
+ actions: <SemanticsAction>[
+ SemanticsAction.tap,
+ ],
+ label: 'ABC',
+ rect: const Rect.fromLTRB(0.0, 0.0, 88.0, 48.0),
+ transform: Matrix4.translationValues(356.0, 276.0, 0.0),
+ flags: <SemanticsFlag>[
+ SemanticsFlag.hasEnabledState,
+ SemanticsFlag.isEnabled,
+ SemanticsFlag.isFocusable,
+ ],
+ textDirection: TextDirection.ltr,
+ ),
+ ],
+ ),
+ ignoreId: true,
),
- ignoreId: true,
- ));
+ );
semantics.dispose();
});
@@ -3114,22 +3164,23 @@
);
// The flags should not have SemanticsFlag.isButton
- expect(semantics, hasSemantics(
- TestSemantics.root(
- children: <TestSemantics>[
- TestSemantics.rootChild(
- label: 'ABC',
- rect: const Rect.fromLTRB(0.0, 0.0, 88.0, 48.0),
- transform: Matrix4.translationValues(356.0, 276.0, 0.0),
- flags: <SemanticsFlag>[
- SemanticsFlag.hasEnabledState,
- ],
- textDirection: TextDirection.ltr,
- ),
- ],
+ expect(
+ semantics,
+ hasSemantics(
+ TestSemantics.root(
+ children: <TestSemantics>[
+ TestSemantics(
+ rect: const Rect.fromLTRB(0.0, 0.0, 88.0, 48.0),
+ flags: <SemanticsFlag>[SemanticsFlag.hasEnabledState],
+ label: 'ABC',
+ textDirection: TextDirection.ltr,
+ ),
+ ],
+ ),
+ ignoreTransform: true,
+ ignoreId: true,
),
- ignoreId: true,
- ));
+ );
semantics.dispose();
});
