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

Fix `ExcludeFocus` so it won't refocus a sibling of the focused node. (#61756)

This changes FocusNode.descendantsAreFocusable so that it doesn't allow the enclosing scope to re-focus a node that is a descendant of the node with descendantsAreFocusable set to false.

Because of the order in which the internal state for descendantsAreFocusable was being set, setting it to false was causing a sibling node to be focused when descendantsAreFocusable of the parent was set to false, even though it shouldn't have been focusable, because the enclosing scope would search for a candidate to be focused before the internal state was set to false.

Instead of looping over the children and telling them all to unfocus (and select the previously focused node), this unfocuses the node that has descendantsAreFocusable set to false, with the disposition UnfocusDisposition.previouslyFocusedChild, so that its enclosing scope will look for a previously focused child that isn't part of the subtree being excluded.

This affects how the ExcludeFocus widget behaves when turning on exclude.
parent ddb8e6e3
......@@ -515,6 +515,9 @@ class FocusNode with DiagnosticableTreeMixin, ChangeNotifier {
///
/// Does not affect the value of [canRequestFocus] on the descendants.
///
/// If a descendant node loses focus when this value is changed, the focus
/// will move to the scope enclosing this node.
///
/// See also:
///
/// * [ExcludeFocus], a widget that uses this property to conditionally
......@@ -531,12 +534,12 @@ class FocusNode with DiagnosticableTreeMixin, ChangeNotifier {
if (value == _descendantsAreFocusable) {
return;
}
// Set _descendantsAreFocusable before unfocusing, so the scope won't try
// and focus any of the children here again if it is false.
_descendantsAreFocusable = value;
if (!value && hasFocus) {
for (final FocusNode child in children) {
child.unfocus(disposition: UnfocusDisposition.previouslyFocusedChild);
}
unfocus(disposition: UnfocusDisposition.previouslyFocusedChild);
}
_descendantsAreFocusable = value;
_manager?._markPropertiesChanged(this);
}
......
......@@ -77,7 +77,7 @@ class TestFocusState extends State<TestFocus> {
}
void main() {
group(FocusScope, () {
group('FocusScope', () {
testWidgets('Can focus', (WidgetTester tester) async {
final GlobalKey<TestFocusState> key = GlobalKey();
......@@ -1078,7 +1078,7 @@ void main() {
});
});
group(Focus, () {
group('Focus', () {
testWidgets('Focus.of stops at the nearest Focus widget.', (WidgetTester tester) async {
final GlobalKey key1 = GlobalKey(debugLabel: '1');
final GlobalKey key2 = GlobalKey(debugLabel: '2');
......@@ -1596,7 +1596,7 @@ void main() {
expect(semantics, hasSemantics(expectedSemantics));
});
});
group(ExcludeFocus, () {
group('ExcludeFocus', () {
testWidgets("Descendants of ExcludeFocus aren't focusable.", (WidgetTester tester) async {
final GlobalKey key1 = GlobalKey(debugLabel: '1');
final GlobalKey key2 = GlobalKey(debugLabel: '2');
......@@ -1635,6 +1635,76 @@ void main() {
expect(containerNode.hasFocus, isFalse);
expect(unfocusableNode.hasFocus, isFalse);
});
// Regression test for https://github.com/flutter/flutter/issues/61700
testWidgets("ExcludeFocus doesn't transfer focus to another descendant.", (WidgetTester tester) async {
final FocusNode parentFocusNode = FocusNode(debugLabel: 'group');
final FocusNode focusNode1 = FocusNode(debugLabel: 'node 1');
final FocusNode focusNode2 = FocusNode(debugLabel: 'node 2');
await tester.pumpWidget(
ExcludeFocus(
excluding: false,
child: Focus(
focusNode: parentFocusNode,
child: Column(
children: <Widget>[
Focus(
autofocus: true,
focusNode: focusNode1,
child: Container(),
),
Focus(
focusNode: focusNode2,
child: Container(),
),
],
),
),
),
);
await tester.pump();
expect(parentFocusNode.hasFocus, isTrue);
expect(focusNode1.hasPrimaryFocus, isTrue);
expect(focusNode2.hasFocus, isFalse);
// Move focus to the second node to create some focus history for the scope.
focusNode2.requestFocus();
await tester.pump();
expect(parentFocusNode.hasFocus, isTrue);
expect(focusNode1.hasFocus, isFalse);
expect(focusNode2.hasPrimaryFocus, isTrue);
// Now turn off the focus for the subtree.
await tester.pumpWidget(
ExcludeFocus(
excluding: true,
child: Focus(
focusNode: parentFocusNode,
child: Column(
children: <Widget>[
Focus(
autofocus: true,
focusNode: focusNode1,
child: Container(),
),
Focus(
focusNode: focusNode2,
child: Container(),
),
],
),
),
),
);
await tester.pump();
expect(focusNode1.hasFocus, isFalse);
expect(focusNode2.hasFocus, isFalse);
expect(parentFocusNode.hasFocus, isFalse);
expect(parentFocusNode.enclosingScope.hasPrimaryFocus, isTrue);
});
testWidgets("ExcludeFocus doesn't introduce a Semantics node", (WidgetTester tester) async {
final SemanticsTester semantics = SemanticsTester(tester);
await tester.pumpWidget(ExcludeFocus(child: Container()));
......
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