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

Change Focus.unfocus to take a disposition for where the focus… (#50831)

When Focus.unfocus is called, the caller usually just thinks about wanting to remove focus from the node, but really, unfocus is a request to automatically pass the focus to another (hopefully useful) node.

This PR removes the focusPrevious flag from unfocus, and replaces it with a disposition enum that indicates where the focus should go from here.

The other value of the UnfocusDisposition enum is UnfocusDisposition.scope.

UnfocusDisposition.previouslyFocusedChild is closest to what focusPrevious used to do: focus the nearest enclosing scope and use its focusedChild field to walk down the tree, finding the leaf focusedChild. This PR modifies it slightly so that it walks up to the nearest focusable enclosing scope before trying to focus the children. This change addresses #48903

A new mode: UnfocusDisposition.scope will focus the nearest focusable enclosing scope of this node without trying to use the FocusScopeNode.focusedChild value to descend to the leaf focused child. This is useful as a default for both text field finalization and for what happens when canRequestFocus is set to false. It allows the scope to stay focused so that nextFocus/previousFocus still work as expected, but removes the focus from primary focus.

In addition to those changes, unfocus called on a FocuScope that wasn't the primary focus used to unfocus the primary focus instead. I removed that behavior, since it was buggy: if the primary focus was inside of a child scope, and you called unfocus on the parent scope, then the child scope could have focused another of its children instead, leaving the scope that you called unfocus on with hasFocus returning true still. If you want to remove the focus from the primary focus instead of the scope, that's easy enough to do: just call primaryFocus.unfocus().

Fixes #48903
parent 444b13b8
......@@ -914,6 +914,8 @@ void main() {
tester.testTextInput.log.clear();
tester.testTextInput.closeConnection();
// A pump is needed to allow the focus change (unfocus) to be resolved.
await tester.pump();
// Widget does not have focus anymore.
expect(state.wantKeepAlive, false);
......
......@@ -1428,10 +1428,10 @@ void main() {
// Check FocusNode with child (focus1). Shouldn't affect children.
await pumpTest(allowFocus1: false);
expect(Focus.of(container1.currentContext).hasFocus, isFalse);
expect(Focus.of(container1.currentContext).hasFocus, isTrue); // focus2 has focus.
Focus.of(focus2.currentContext).requestFocus(); // Try to focus focus1
await tester.pump();
expect(Focus.of(container1.currentContext).hasFocus, isFalse);
expect(Focus.of(container1.currentContext).hasFocus, isTrue); // focus2 still has focus.
Focus.of(container1.currentContext).requestFocus(); // Now try to focus focus2
await tester.pump();
expect(Focus.of(container1.currentContext).hasFocus, isTrue);
......
......@@ -6,9 +6,9 @@ import 'dart:collection';
import 'package:flutter/foundation.dart';
import 'package:flutter/material.dart';
import 'package:mockito/mockito.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:flutter/widgets.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:mockito/mockito.dart';
final List<String> results = <String>[];
......@@ -1242,6 +1242,58 @@ void main() {
// It should refocus page one after pops.
expect(focusNodeOnPageOne.hasFocus, isTrue);
});
testWidgets('focus traversal is correct when popping mutiple pages simultaneously - with focused children', (WidgetTester tester) async {
// Regression test: https://github.com/flutter/flutter/issues/48903
final GlobalKey<NavigatorState> navigatorKey = GlobalKey<NavigatorState>();
await tester.pumpWidget(MaterialApp(
navigatorKey: navigatorKey,
home: const Text('dummy1'),
));
final Element textOnPageOne = tester.element(find.text('dummy1'));
final FocusScopeNode focusNodeOnPageOne = FocusScope.of(textOnPageOne);
expect(focusNodeOnPageOne.hasFocus, isTrue);
// Pushes one page.
navigatorKey.currentState.push<void>(
MaterialPageRoute<void>(
builder: (BuildContext context) => const Material(child: TextField()),
)
);
await tester.pumpAndSettle();
final Element textOnPageTwo = tester.element(find.byType(TextField));
final FocusScopeNode focusNodeOnPageTwo = FocusScope.of(textOnPageTwo);
// The focus should be on second page.
expect(focusNodeOnPageOne.hasFocus, isFalse);
expect(focusNodeOnPageTwo.hasFocus, isTrue);
// Move the focus to another node.
focusNodeOnPageTwo.nextFocus();
await tester.pumpAndSettle();
expect(focusNodeOnPageTwo.hasFocus, isTrue);
expect(focusNodeOnPageTwo.hasPrimaryFocus, isFalse);
// Pushes another page.
navigatorKey.currentState.push<void>(
MaterialPageRoute<void>(
builder: (BuildContext context) => const Text('dummy3'),
)
);
await tester.pumpAndSettle();
final Element textOnPageThree = tester.element(find.text('dummy3'));
final FocusScopeNode focusNodeOnPageThree = FocusScope.of(textOnPageThree);
// The focus should be on third page.
expect(focusNodeOnPageOne.hasFocus, isFalse);
expect(focusNodeOnPageTwo.hasFocus, isFalse);
expect(focusNodeOnPageThree.hasFocus, isTrue);
// Pops two pages simultaneously.
navigatorKey.currentState.popUntil((Route<void> route) => route.isFirst);
await tester.pumpAndSettle();
// It should refocus page one after pops.
expect(focusNodeOnPageOne.hasFocus, isTrue);
});
});
}
......
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