Unverified Commit e3005e69 authored by Greg Spencer's avatar Greg Spencer Committed by GitHub

Fixes Focus and FocusScope's assignment of canRequestFocus. (#46168)

This fixes an issue where lines like this:

    focusNode.canRequestFocus = widget.canRequestFocus ?? focusNode.canRequestFocus;

Were causing the canRequestFocus bit to copy the status of the enclosing scope, since canRequestFocus also looks to the enclosing scope to decide if it can focus.
parent cb0dcbe1
......@@ -2608,6 +2608,9 @@ class SizedOverflowBox extends SingleChildRenderObjectWidget {
/// painting anything, without making the child available for hit testing, and
/// without taking any room in the parent.
///
/// Offstage children are still active: they can receive focus and have keyboard
/// input directed to them.
///
/// Animations continue to run in offstage children, and therefore use battery
/// and CPU time, regardless of whether the animations end up being visible.
///
......@@ -2634,6 +2637,12 @@ class Offstage extends SingleChildRenderObjectWidget {
/// painting anything, without making the child available for hit testing, and
/// without taking any room in the parent.
///
/// Offstage children are still active: they can receive focus and have keyboard
/// input directed to them.
///
/// Animations continue to run in offstage children, and therefore use battery
/// and CPU time, regardless of whether the animations end up being visible.
///
/// If false, the child is included in the tree as normal.
final bool offstage;
......
......@@ -404,9 +404,16 @@ class FocusNode with DiagnosticableTreeMixin, ChangeNotifier {
/// If true, this focus node may request the primary focus.
///
/// Defaults to true. Set to false if you want this node to do nothing when
/// [requestFocus] is called on it. Does not affect the children of this node,
/// and [hasFocus] can still return true if this node is the ancestor of a
/// node with primary focus.
/// [requestFocus] is called on it.
///
/// If set to false on a [FocusScopeNode], will cause all of the children of
/// the scope node to not be focusable.
///
/// If set to false on a [FocusNode], it will not affect the children of the
/// node.
///
/// The [hasFocus] member can still return true if this node is the ancestor
/// of a node with primary focus.
///
/// This is different than [skipTraversal] because [skipTraversal] still
/// allows the node to be focused, just not traversed to via the
......
......@@ -345,8 +345,12 @@ class _FocusState extends State<Focus> {
_internalNode ??= _createNode();
}
_focusAttachment = focusNode.attach(context, onKey: widget.onKey);
focusNode.skipTraversal = widget.skipTraversal ?? focusNode.skipTraversal;
focusNode.canRequestFocus = widget.canRequestFocus ?? focusNode.canRequestFocus;
if (widget.skipTraversal != null) {
focusNode.skipTraversal = widget.skipTraversal;
}
if (widget.canRequestFocus != null) {
focusNode.canRequestFocus = widget.canRequestFocus;
}
_canRequestFocus = focusNode.canRequestFocus;
_hasPrimaryFocus = focusNode.hasPrimaryFocus;
......@@ -410,8 +414,12 @@ class _FocusState extends State<Focus> {
}());
if (oldWidget.focusNode == widget.focusNode) {
focusNode.skipTraversal = widget.skipTraversal ?? focusNode.skipTraversal;
focusNode.canRequestFocus = widget.canRequestFocus ?? focusNode.canRequestFocus;
if (widget.skipTraversal != null) {
focusNode.skipTraversal = widget.skipTraversal;
}
if (widget.canRequestFocus != null) {
focusNode.canRequestFocus = widget.canRequestFocus;
}
} else {
_focusAttachment.detach();
focusNode.removeListener(_handleFocusChanged);
......
......@@ -1336,4 +1336,167 @@ void main() {
expect(key.currentState.focusNode.canRequestFocus, isFalse);
});
testWidgets('canRequestFocus causes descendants of scope to be skipped.', (WidgetTester tester) async {
final GlobalKey scope1 = GlobalKey(debugLabel: 'scope1');
final GlobalKey scope2 = GlobalKey(debugLabel: 'scope2');
final GlobalKey focus1 = GlobalKey(debugLabel: 'focus1');
final GlobalKey focus2 = GlobalKey(debugLabel: 'focus2');
final GlobalKey container1 = GlobalKey(debugLabel: 'container');
Future<void> pumpTest({
bool allowScope1 = true,
bool allowScope2 = true,
bool allowFocus1 = true,
bool allowFocus2 = true,
}) async {
await tester.pumpWidget(
FocusScope(
key: scope1,
canRequestFocus: allowScope1,
child: FocusScope(
key: scope2,
canRequestFocus: allowScope2,
child: Focus(
key: focus1,
canRequestFocus: allowFocus1,
child: Focus(
key: focus2,
canRequestFocus: allowFocus2,
child: Container(
key: container1,
),
),
),
),
),
);
await tester.pump();
}
// Check childless node (focus2).
await pumpTest();
Focus.of(container1.currentContext).requestFocus();
await tester.pump();
expect(Focus.of(container1.currentContext).hasFocus, isTrue);
await pumpTest(allowFocus2: false);
expect(Focus.of(container1.currentContext).hasFocus, isFalse);
Focus.of(container1.currentContext).requestFocus();
await tester.pump();
expect(Focus.of(container1.currentContext).hasFocus, isFalse);
await pumpTest();
Focus.of(container1.currentContext).requestFocus();
await tester.pump();
expect(Focus.of(container1.currentContext).hasFocus, isTrue);
// Check FocusNode with child (focus1). Shouldn't affect children.
await pumpTest(allowFocus1: false);
expect(Focus.of(container1.currentContext).hasFocus, isFalse);
Focus.of(focus2.currentContext).requestFocus(); // Try to focus focus1
await tester.pump();
expect(Focus.of(container1.currentContext).hasFocus, isFalse);
Focus.of(container1.currentContext).requestFocus(); // Now try to focus focus2
await tester.pump();
expect(Focus.of(container1.currentContext).hasFocus, isTrue);
await pumpTest();
// Try again, now that we've set focus1's canRequestFocus to true again.
Focus.of(container1.currentContext).unfocus();
await tester.pump();
expect(Focus.of(container1.currentContext).hasFocus, isFalse);
Focus.of(container1.currentContext).requestFocus();
await tester.pump();
expect(Focus.of(container1.currentContext).hasFocus, isTrue);
// Check FocusScopeNode with only FocusNode children (scope2). Should affect children.
await pumpTest(allowScope2: false);
expect(Focus.of(container1.currentContext).hasFocus, isFalse);
FocusScope.of(focus1.currentContext).requestFocus(); // Try to focus scope2
await tester.pump();
expect(Focus.of(container1.currentContext).hasFocus, isFalse);
Focus.of(focus2.currentContext).requestFocus(); // Try to focus focus1
await tester.pump();
expect(Focus.of(container1.currentContext).hasFocus, isFalse);
Focus.of(container1.currentContext).requestFocus(); // Try to focus focus2
await tester.pump();
expect(Focus.of(container1.currentContext).hasFocus, isFalse);
await pumpTest();
// Try again, now that we've set scope2's canRequestFocus to true again.
Focus.of(container1.currentContext).requestFocus();
await tester.pump();
expect(Focus.of(container1.currentContext).hasFocus, isTrue);
// Check FocusScopeNode with both FocusNode children and FocusScope children (scope1). Should affect children.
await pumpTest(allowScope1: false);
expect(Focus.of(container1.currentContext).hasFocus, isFalse);
FocusScope.of(scope2.currentContext).requestFocus(); // Try to focus scope1
await tester.pump();
expect(Focus.of(container1.currentContext).hasFocus, isFalse);
FocusScope.of(focus1.currentContext).requestFocus(); // Try to focus scope2
await tester.pump();
expect(Focus.of(container1.currentContext).hasFocus, isFalse);
Focus.of(focus2.currentContext).requestFocus(); // Try to focus focus1
await tester.pump();
expect(Focus.of(container1.currentContext).hasFocus, isFalse);
Focus.of(container1.currentContext).requestFocus(); // Try to focus focus2
await tester.pump();
expect(Focus.of(container1.currentContext).hasFocus, isFalse);
await pumpTest();
// Try again, now that we've set scope1's canRequestFocus to true again.
Focus.of(container1.currentContext).requestFocus();
await tester.pump();
expect(Focus.of(container1.currentContext).hasFocus, isTrue);
});
testWidgets('skipTraversal works as expected.', (WidgetTester tester) async {
final FocusScopeNode scope1 = FocusScopeNode(debugLabel: 'scope1');
final FocusScopeNode scope2 = FocusScopeNode(debugLabel: 'scope2');
final FocusNode focus1 = FocusNode(debugLabel: 'focus1');
final FocusNode focus2 = FocusNode(debugLabel: 'focus2');
Future<void> pumpTest({
bool traverseScope1 = false,
bool traverseScope2 = false,
bool traverseFocus1 = false,
bool traverseFocus2 = false,
}) async {
await tester.pumpWidget(
FocusScope(
node: scope1,
skipTraversal: traverseScope1,
child: FocusScope(
node: scope2,
skipTraversal: traverseScope2,
child: Focus(
focusNode: focus1,
skipTraversal: traverseFocus1,
child: Focus(
focusNode: focus2,
skipTraversal: traverseFocus2,
child: Container(),
),
),
),
),
);
await tester.pump();
}
await pumpTest();
expect(scope1.traversalDescendants, equals(<FocusNode>[focus2, focus1, scope2]));
// Check childless node (focus2).
await pumpTest(traverseFocus2: true);
expect(scope1.traversalDescendants, equals(<FocusNode>[focus1, scope2]));
// Check FocusNode with child (focus1). Shouldn't affect children.
await pumpTest(traverseFocus1: true);
expect(scope1.traversalDescendants, equals(<FocusNode>[focus2, scope2]));
// Check FocusScopeNode with only FocusNode children (scope2). Should affect children.
await pumpTest(traverseScope2: true);
expect(scope1.traversalDescendants, equals(<FocusNode>[focus2, focus1]));
// Check FocusScopeNode with both FocusNode children and FocusScope children (scope1). Should affect children.
await pumpTest(traverseScope1: true);
expect(scope1.traversalDescendants, equals(<FocusNode>[focus2, focus1, scope2]));
});
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment