From 6495d3775d99668fda6febf3e73562e729fa2750 Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Mon, 13 Jan 2020 11:56:47 -0800 Subject: [PATCH] Allow requestFocus on an unattached FocusNode to create a deferred focus request (#48589) This changes the behavior of requestFocus when it is called on a FocusNode that does not yet have a parent, so that it defers requesting focus until it receives a parent. Before this change, calling requestFocus before it had a parent was a no-op. This allows scenarios where a widget is newly added and wishes to immediately request the focus. Previously, it was very hard to make that work because requesting focus before the widget's focus node had a parent was ignored, so the developer had to wait until two frames later to request focus (one for the widget's node to be added to the focus tree, and one to request the focus). Now, in order to have a widget be focused when initially added, you just need to call requestFocus on its node when you create it, and as soon as it is added, it will automatically request focus. This is different from the autofocus attribute on the Focus widget, because it unconditionally requests focus when added (autofocus will only request focus if nothing else in the scope has focus). --- .../lib/src/widgets/focus_manager.dart | 32 +++++++++++++++- .../flutter/lib/src/widgets/focus_scope.dart | 2 +- .../test/widgets/focus_manager_test.dart | 19 ++++++++++ .../test/widgets/focus_scope_test.dart | 38 +++++++++++++++++++ 4 files changed, 89 insertions(+), 2 deletions(-) diff --git a/packages/flutter/lib/src/widgets/focus_manager.dart b/packages/flutter/lib/src/widgets/focus_manager.dart index 5ba3129b03..a5dede7da6 100644 --- a/packages/flutter/lib/src/widgets/focus_manager.dart +++ b/packages/flutter/lib/src/widgets/focus_manager.dart @@ -367,6 +367,8 @@ class FocusNode with DiagnosticableTreeMixin, ChangeNotifier { /// Creates a focus node. /// /// The [debugLabel] is ignored on release builds. + /// + /// The [skipTraversal] and [canRequestFocus] arguments must not be null. FocusNode({ String debugLabel, FocusOnKeyCallback onKey, @@ -776,6 +778,10 @@ class FocusNode with DiagnosticableTreeMixin, ChangeNotifier { if (oldScope != null && child.context != null && child.enclosingScope != oldScope) { DefaultFocusTraversal.of(child.context, nullOk: true)?.changedScope(node: child, oldScope: oldScope); } + if (child._requestFocusWhenReparented) { + child._doRequestFocus(); + child._requestFocusWhenReparented = false; + } } /// Called by the _host_ [StatefulWidget] to attach a [FocusNode] to the @@ -818,7 +824,10 @@ class FocusNode with DiagnosticableTreeMixin, ChangeNotifier { /// Requests the primary focus for this node, or for a supplied [node], which /// will also give focus to its [ancestors]. /// - /// If called without a node, request focus for this node. + /// If called without a node, request focus for this node. If the node hasn't + /// been added to the focus tree yet, then defer the focus request until it + /// is, allowing newly created widgets to request focus as soon as they are + /// added. /// /// If the given [node] is not yet a part of the focus tree, then this method /// will add the [node] as a child of this node before requesting focus. @@ -849,6 +858,13 @@ class FocusNode with DiagnosticableTreeMixin, ChangeNotifier { assert(_focusDebug('Node NOT requesting focus because canRequestFocus is false: $this')); return; } + // If the node isn't part of the tree, then we just defer the focus request + // until the next time it is reparented, so that it's possible to focus + // newly added widgets. + if (_parent == null) { + _requestFocusWhenReparented = true; + return; + } _setAsFocusedChild(); if (hasPrimaryFocus) { return; @@ -858,6 +874,20 @@ class FocusNode with DiagnosticableTreeMixin, ChangeNotifier { _markAsDirty(newFocus: this); } + // If set to true, the node will request focus on this node the next time + // this node is reparented in the focus tree. + // + // Once requestFocus has been called at the next reparenting, this value + // will be reset to false. + // + // This will only force a call to requestFocus for the node once the next time + // the node is reparented. After that, _requestFocusWhenReparented would need + // to be set to true again to have it be focused again on the next + // reparenting. + // + // This is used when requestFocus is called and there is no parent yet. + bool _requestFocusWhenReparented = false; + /// Sets this node as the [FocusScopeNode.focusedChild] of the enclosing /// scope. /// diff --git a/packages/flutter/lib/src/widgets/focus_scope.dart b/packages/flutter/lib/src/widgets/focus_scope.dart index e73787e5bb..3714fa54e9 100644 --- a/packages/flutter/lib/src/widgets/focus_scope.dart +++ b/packages/flutter/lib/src/widgets/focus_scope.dart @@ -344,7 +344,6 @@ class _FocusState extends State { // _createNode is overridden in _FocusScopeState. _internalNode ??= _createNode(); } - _focusAttachment = focusNode.attach(context, onKey: widget.onKey); if (widget.skipTraversal != null) { focusNode.skipTraversal = widget.skipTraversal; } @@ -353,6 +352,7 @@ class _FocusState extends State { } _canRequestFocus = focusNode.canRequestFocus; _hasPrimaryFocus = focusNode.hasPrimaryFocus; + _focusAttachment = focusNode.attach(context, onKey: widget.onKey); // Add listener even if the _internalNode existed before, since it should // not be listening now if we're re-using a previous one because it should diff --git a/packages/flutter/test/widgets/focus_manager_test.dart b/packages/flutter/test/widgets/focus_manager_test.dart index 621085ef62..1ce95c2e71 100644 --- a/packages/flutter/test/widgets/focus_manager_test.dart +++ b/packages/flutter/test/widgets/focus_manager_test.dart @@ -163,6 +163,25 @@ void main() { expect(child2.hasFocus, isTrue); expect(child2.hasPrimaryFocus, isTrue); }); + testWidgets('Requesting focus before adding to tree results in a request after adding', (WidgetTester tester) async { + final BuildContext context = await setupWidget(tester); + final FocusScopeNode scope = FocusScopeNode(); + final FocusAttachment scopeAttachment = scope.attach(context); + final FocusNode child = FocusNode(); + child.requestFocus(); + expect(child.hasPrimaryFocus, isFalse); // not attached yet. + + scopeAttachment.reparent(parent: tester.binding.focusManager.rootScope); + await tester.pump(); + expect(scope.focusedChild, isNull); + expect(child.hasPrimaryFocus, isFalse); // not attached yet. + + final FocusAttachment childAttachment = child.attach(context); + expect(child.hasPrimaryFocus, isFalse); // not parented yet. + childAttachment.reparent(parent: scope); + await tester.pump(); + expect(child.hasPrimaryFocus, isTrue); // now attached and parented, so focus finally happened. + }); testWidgets('Autofocus works.', (WidgetTester tester) async { final BuildContext context = await setupWidget(tester); final FocusScopeNode scope = FocusScopeNode(debugLabel: 'Scope'); diff --git a/packages/flutter/test/widgets/focus_scope_test.dart b/packages/flutter/test/widgets/focus_scope_test.dart index afa8f9b16f..d97280ce73 100644 --- a/packages/flutter/test/widgets/focus_scope_test.dart +++ b/packages/flutter/test/widgets/focus_scope_test.dart @@ -1031,6 +1031,44 @@ void main() { await tester.pump(); expect(focusNode.hasPrimaryFocus, isTrue); }); + testWidgets("Won't autofocus a node if one is already focused.", (WidgetTester tester) async { + final FocusNode focusNodeA = FocusNode(debugLabel: 'Test Node A'); + final FocusNode focusNodeB = FocusNode(debugLabel: 'Test Node B'); + await tester.pumpWidget( + Column( + children: [ + Focus( + focusNode: focusNodeA, + autofocus: true, + child: Container(), + ), + ], + ), + ); + + await tester.pump(); + expect(focusNodeA.hasPrimaryFocus, isTrue); + + await tester.pumpWidget( + Column( + children: [ + Focus( + focusNode: focusNodeA, + child: Container(), + ), + Focus( + focusNode: focusNodeB, + autofocus: true, + child: Container(), + ), + ], + ), + ); + + await tester.pump(); + expect(focusNodeB.hasPrimaryFocus, isFalse); + expect(focusNodeA.hasPrimaryFocus, isTrue); + }); }); group(Focus, () { testWidgets('Focus.of stops at the nearest Focus widget.', (WidgetTester tester) async {