The PopupMenuButton should not steal focus from the TextField when it appears. (#150568)

Fixes: #24843
Fixes: #50567
This commit is contained in:
yim 2024-08-09 00:09:18 +08:00 committed by GitHub
parent 51606f99a4
commit 14cd5fa30a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 80 additions and 4 deletions

View File

@ -282,6 +282,7 @@ class CupertinoPageRoute<T> extends PageRoute<T> with CupertinoRouteTransitionMi
required this.builder,
this.title,
super.settings,
super.requestFocus,
this.maintainState = true,
super.fullscreenDialog,
super.allowSnapshotting = true,
@ -1076,6 +1077,7 @@ class CupertinoModalPopupRoute<T> extends PopupRoute<T> {
bool semanticsDismissible = false,
super.filter,
super.settings,
super.requestFocus,
this.anchorPoint,
}) : _barrierDismissible = barrierDismissible,
_semanticsDismissible = semanticsDismissible;
@ -1387,6 +1389,7 @@ class CupertinoDialogRoute<T> extends RawDialogRoute<T> {
super.transitionDuration = const Duration(milliseconds: 250),
this.transitionBuilder,
super.settings,
super.requestFocus,
super.anchorPoint,
}) : super(
pageBuilder: (BuildContext context, Animation<double> animation, Animation<double> secondaryAnimation) {

View File

@ -868,6 +868,7 @@ class ModalBottomSheetRoute<T> extends PopupRoute<T> {
required this.isScrollControlled,
this.scrollControlDisabledMaxHeightRatio = _defaultScrollControlDisabledMaxHeightRatio,
super.settings,
super.requestFocus,
this.transitionAnimationController,
this.anchorPoint,
this.useSafeArea = false,

View File

@ -1580,6 +1580,7 @@ class DialogRoute<T> extends RawDialogRoute<T> {
String? barrierLabel,
bool useSafeArea = true,
super.settings,
super.requestFocus,
super.anchorPoint,
super.traversalEdgeBehavior,
}) : super(

View File

@ -37,6 +37,7 @@ class MaterialPageRoute<T> extends PageRoute<T> with MaterialRouteTransitionMixi
MaterialPageRoute({
required this.builder,
super.settings,
super.requestFocus,
this.maintainState = true,
super.fullscreenDialog,
super.allowSnapshotting = true,

View File

@ -859,6 +859,7 @@ class _PopupMenuRoute<T> extends PopupRoute<T> {
this.constraints,
required this.clipBehavior,
super.settings,
super.requestFocus,
this.popUpAnimationStyle,
}) : itemSizes = List<Size?>.filled(items.length, null),
// Menus always cycle focus through their items irrespective of the
@ -1023,6 +1024,9 @@ class _PopupMenuRoute<T> extends PopupRoute<T> {
/// The `clipBehavior` argument is used to clip the shape of the menu. Defaults to
/// [Clip.none].
///
/// The `requestFocus` argument specifies whether the menu should request focus
/// when it appears. If it is null, [Navigator.requestFocus] is used instead.
///
/// See also:
///
/// * [PopupMenuItem], a popup menu entry for a single value.
@ -1049,6 +1053,7 @@ Future<T?> showMenu<T>({
Clip clipBehavior = Clip.none,
RouteSettings? routeSettings,
AnimationStyle? popUpAnimationStyle,
bool? requestFocus,
}) {
assert(items.isNotEmpty);
assert(debugCheckHasMaterialLocalizations(context));
@ -1084,6 +1089,7 @@ Future<T?> showMenu<T>({
clipBehavior: clipBehavior,
settings: routeSettings,
popUpAnimationStyle: popUpAnimationStyle,
requestFocus: requestFocus,
));
}
@ -1217,6 +1223,7 @@ class PopupMenuButton<T> extends StatefulWidget {
this.popUpAnimationStyle,
this.routeSettings,
this.style,
this.requestFocus,
}) : assert(
!(child != null && icon != null),
'You can only pass [child] or [icon], not both.',
@ -1438,6 +1445,11 @@ class PopupMenuButton<T> extends StatefulWidget {
/// Null by default.
final ButtonStyle? style;
/// Whether to request focus when the menu appears.
///
/// If null, [Navigator.requestFocus] will be used instead.
final bool? requestFocus;
@override
PopupMenuButtonState<T> createState() => PopupMenuButtonState<T>();
}
@ -1501,6 +1513,7 @@ class PopupMenuButtonState<T> extends State<PopupMenuButton<T>> {
useRootNavigator: widget.useRootNavigator,
popUpAnimationStyle: widget.popUpAnimationStyle,
routeSettings: widget.routeSettings,
requestFocus: widget.requestFocus,
)
.then<void>((T? newValue) {
if (!mounted) {

View File

@ -162,7 +162,14 @@ abstract class Route<T> extends _RoutePlaceholder {
///
/// If the [settings] are not provided, an empty [RouteSettings] object is
/// used instead.
Route({ RouteSettings? settings }) : _settings = settings ?? const RouteSettings() {
///
/// If [requestFocus] is not provided, the value of [Navigator.requestFocus] is
/// used instead.
Route({
RouteSettings? settings,
bool? requestFocus,
}) : _settings = settings ?? const RouteSettings(),
_requestFocus = requestFocus {
if (kFlutterMemoryAllocationsEnabled) {
FlutterMemoryAllocations.instance.dispatchObjectCreated(
library: 'package:flutter/widgets.dart',
@ -172,6 +179,12 @@ abstract class Route<T> extends _RoutePlaceholder {
}
}
/// When the route state is updated, request focus if the current route is at the top.
///
/// If not provided in the constructor, [Navigator.requestFocus] is used instead.
bool get requestFocus => _requestFocus ?? navigator?.widget.requestFocus ?? false;
final bool? _requestFocus;
/// The navigator that the route is in, if any.
NavigatorState? get navigator => _navigator;
NavigatorState? _navigator;
@ -256,7 +269,7 @@ abstract class Route<T> extends _RoutePlaceholder {
@mustCallSuper
TickerFuture didPush() {
return TickerFuture.complete()..then<void>((void _) {
if (navigator?.widget.requestFocus ?? false) {
if (requestFocus) {
navigator!.focusNode.enclosingScope?.requestFocus();
}
});
@ -272,7 +285,7 @@ abstract class Route<T> extends _RoutePlaceholder {
@protected
@mustCallSuper
void didAdd() {
if (navigator?.widget.requestFocus ?? false) {
if (requestFocus) {
// This TickerFuture serves two purposes. First, we want to make sure that
// animations triggered by other operations will finish before focusing
// the navigator. Second, navigator.focusNode might acquire more focused
@ -1760,6 +1773,9 @@ class Navigator extends StatefulWidget {
/// Whether or not the navigator and it's new topmost route should request focus
/// when the new route is pushed onto the navigator.
///
/// If [Route.requestFocus] is set on the topmost route, that will take precedence
/// over this value.
///
/// Defaults to true.
final bool requestFocus;

View File

@ -24,6 +24,7 @@ abstract class PageRoute<T> extends ModalRoute<T> {
/// Creates a modal route that replaces the entire screen.
PageRoute({
super.settings,
super.requestFocus,
this.fullscreenDialog = false,
this.allowSnapshotting = true,
bool barrierDismissible = false,
@ -81,6 +82,7 @@ class PageRouteBuilder<T> extends PageRoute<T> {
/// Creates a route that delegates to builder callbacks.
PageRouteBuilder({
super.settings,
super.requestFocus,
required this.pageBuilder,
this.transitionsBuilder = _defaultTransitionsBuilder,
this.transitionDuration = const Duration(milliseconds: 300),

View File

@ -57,6 +57,7 @@ abstract class OverlayRoute<T> extends Route<T> {
/// Creates a route that knows how to interact with an [Overlay].
OverlayRoute({
super.settings,
super.requestFocus,
});
/// Subclasses should override this getter to return the builders for the overlay.
@ -115,6 +116,7 @@ abstract class TransitionRoute<T> extends OverlayRoute<T> implements PredictiveB
/// Creates a route that animates itself when it is pushed or popped.
TransitionRoute({
super.settings,
super.requestFocus,
});
/// This future completes only once the transition itself has finished, after
@ -1028,7 +1030,7 @@ class _ModalScopeState<T> extends State<_ModalScope<T>> {
}
bool get _shouldRequestFocus {
return widget.route.navigator!.widget.requestFocus;
return widget.route.requestFocus;
}
// This should be called to wrap any changes to route.isCurrent, route.canPop,
@ -1140,6 +1142,7 @@ abstract class ModalRoute<T> extends TransitionRoute<T> with LocalHistoryRoute<T
/// Creates a route that blocks interaction with previous routes.
ModalRoute({
super.settings,
super.requestFocus,
this.filter,
this.traversalEdgeBehavior,
});
@ -2093,6 +2096,7 @@ abstract class PopupRoute<T> extends ModalRoute<T> {
/// Initializes the [PopupRoute].
PopupRoute({
super.settings,
super.requestFocus,
super.filter,
super.traversalEdgeBehavior,
});
@ -2294,6 +2298,7 @@ class RawDialogRoute<T> extends PopupRoute<T> {
Duration transitionDuration = const Duration(milliseconds: 200),
RouteTransitionsBuilder? transitionBuilder,
super.settings,
super.requestFocus,
this.anchorPoint,
super.traversalEdgeBehavior,
}) : _pageBuilder = pageBuilder,

View File

@ -4285,6 +4285,40 @@ void main() {
inkWell = tester.widget<InkWell>(find.byType(InkWell));
expect(inkWell.borderRadius, borderRadius);
});
testWidgets('If requestFocus is false, the original focus should be preserved upon menu appearance.', (WidgetTester tester) async {
final FocusNode fieldFocusNode = FocusNode();
await tester.pumpWidget(
MaterialApp(
home: Scaffold(
body: Column(
children: <Widget>[
TextField(focusNode: fieldFocusNode, autofocus: true),
PopupMenuButton<int>(
style: const ButtonStyle(
iconColor: MaterialStatePropertyAll<Color>(Colors.red),
),
itemBuilder: (BuildContext context) {
return <PopupMenuItem<int>>[
const PopupMenuItem<int>(
value: 1,
child: Text('One'),
),
];
},
requestFocus: false,
child: const Text('click here'),
)
],
),
),
),
);
expect(fieldFocusNode.hasFocus, isTrue);
await tester.tap(find.text('click here'));
await tester.pump();
expect(fieldFocusNode.hasFocus, isTrue);
});
}
Matcher overlaps(Rect other) => OverlapsMatcher(other);