From d274a2126fc9ff888334b38d6fcaf312e9232bfb Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Fri, 26 Apr 2024 10:17:19 -0700 Subject: [PATCH] Refactor route focus node creation (#147390) ## Description This fixes an issue in the creation of the `FocusScope` in a route: the route should be creating the `FocusScope` widget it has with `withExternalFocusNode`, since it is modifying the node attributes in a builder. Also modified some `AnimatedBuilder`s to be `ListenableBuilder`s, since they're not using animations (no functionality change there, since the implementation of the two is identical). ## Related Issues - #147256 - Fixes #146844 ## Tests - Updated example test. --- .../transitions/listenable_builder.1_test.dart | 2 +- packages/flutter/lib/src/widgets/routes.dart | 18 +++++++++--------- packages/flutter/test/widgets/routes_test.dart | 2 +- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/examples/api/test/widgets/transitions/listenable_builder.1_test.dart b/examples/api/test/widgets/transitions/listenable_builder.1_test.dart index 00f18d1c28b..9b33fb6f021 100644 --- a/examples/api/test/widgets/transitions/listenable_builder.1_test.dart +++ b/examples/api/test/widgets/transitions/listenable_builder.1_test.dart @@ -10,7 +10,7 @@ void main() { testWidgets('Tapping FAB increments counter', (WidgetTester tester) async { await tester.pumpWidget(const example.ListenableBuilderExample()); - String getCount() => (tester.widget(find.descendant(of: find.byType(ListenableBuilder), matching: find.byType(Text))) as Text).data!; + String getCount() => (tester.widget(find.descendant(of: find.byType(ListenableBuilder).last, matching: find.byType(Text))) as Text).data!; expect(find.text('Current counter value:'), findsOneWidget); expect(find.text('0'), findsOneWidget); diff --git a/packages/flutter/lib/src/widgets/routes.dart b/packages/flutter/lib/src/widgets/routes.dart index e6232dfb09b..89e3af2d8ed 100644 --- a/packages/flutter/lib/src/widgets/routes.dart +++ b/packages/flutter/lib/src/widgets/routes.dart @@ -1022,6 +1022,8 @@ class _ModalScopeState extends State<_ModalScope> { @override Widget build(BuildContext context) { + // Only top most route can participate in focus traversal. + focusScopeNode.skipTraversal = !widget.route.isCurrent; return AnimatedBuilder( animation: widget.route.restorationScopeId, builder: (BuildContext context, Widget? child) { @@ -1048,24 +1050,22 @@ class _ModalScopeState extends State<_ModalScope> { }, child: PrimaryScrollController( controller: primaryScrollController, - child: FocusScope( - node: focusScopeNode, // immutable - // Only top most route can participate in focus traversal. - skipTraversal: !widget.route.isCurrent, + child: FocusScope.withExternalFocusNode( + focusScopeNode: focusScopeNode, // immutable child: RepaintBoundary( - child: AnimatedBuilder( - animation: _listenable, // immutable + child: ListenableBuilder( + listenable: _listenable, // immutable builder: (BuildContext context, Widget? child) { return widget.route.buildTransitions( context, widget.route.animation!, widget.route.secondaryAnimation!, - // This additional AnimatedBuilder is include because if the + // This additional ListenableBuilder is include because if the // value of the userGestureInProgressNotifier changes, it's // only necessary to rebuild the IgnorePointer widget and set // the focus node's ability to focus. - AnimatedBuilder( - animation: widget.route.navigator?.userGestureInProgressNotifier ?? ValueNotifier(false), + ListenableBuilder( + listenable: widget.route.navigator?.userGestureInProgressNotifier ?? ValueNotifier(false), builder: (BuildContext context, Widget? child) { final bool ignoreEvents = _shouldIgnoreFocusRequest; focusScopeNode.canRequestFocus = !ignoreEvents; diff --git a/packages/flutter/test/widgets/routes_test.dart b/packages/flutter/test/widgets/routes_test.dart index 49508884e74..3c8cd459b1d 100644 --- a/packages/flutter/test/widgets/routes_test.dart +++ b/packages/flutter/test/widgets/routes_test.dart @@ -1736,7 +1736,7 @@ void main() { semantics.dispose(); }, variant: const TargetPlatformVariant({TargetPlatform.iOS})); - testWidgets('focus traverse correct when pop multiple page simultaneously', (WidgetTester tester) async { + testWidgets('focus traversal is correct when popping multiple pages simultaneously', (WidgetTester tester) async { // Regression test: https://github.com/flutter/flutter/issues/48903 final GlobalKey navigatorKey = GlobalKey(); await tester.pumpWidget(MaterialApp(