Unverified Commit 24e964d8 authored by Greg Spencer's avatar Greg Spencer Committed by GitHub

Delay focus trap unfocus until post-frame (#101847)

parent 37b3feea
......@@ -2152,6 +2152,7 @@ class _RenderFocusTrap extends RenderProxyBoxWithHitTestBehavior {
Expando<BoxHitTestResult> cachedResults = Expando<BoxHitTestResult>();
FocusScopeNode _focusScopeNode;
FocusNode? _previousFocus;
FocusScopeNode get focusScopeNode => _focusScopeNode;
set focusScopeNode(FocusScopeNode value) {
if (focusScopeNode == value)
......@@ -2188,6 +2189,18 @@ class _RenderFocusTrap extends RenderProxyBoxWithHitTestBehavior {
}
}
void _checkForUnfocus() {
if (_previousFocus == null) {
return;
}
// Only continue to unfocus if the previous focus matches the current focus.
// If the focus has changed in the meantime, it was probably intentional.
if (FocusManager.instance.primaryFocus == _previousFocus) {
_previousFocus!.unfocus();
}
_previousFocus = null;
}
@override
void handleEvent(PointerEvent event, HitTestEntry entry) {
assert(debugHandleEvent(event, entry));
......@@ -2219,7 +2232,13 @@ class _RenderFocusTrap extends RenderProxyBoxWithHitTestBehavior {
break;
}
}
if (!hitCurrentFocus)
focusNode.unfocus();
if (!hitCurrentFocus) {
_previousFocus = focusNode;
// Check post-frame to see that the focus hasn't changed before
// unfocusing. This also allows a button tap to capture the previously
// active focus before FocusTrap tries to unfocus it, and avoids a bounce
// through the scope's focus node in between.
SchedulerBinding.instance.scheduleTask<void>(_checkForUnfocus, Priority.touch);
}
}
}
......@@ -5,6 +5,7 @@
import 'dart:collection';
import 'dart:ui';
import 'package:flutter/foundation.dart';
import 'package:flutter/material.dart';
import 'package:flutter/services.dart';
import 'package:flutter_test/flutter_test.dart';
......@@ -1976,6 +1977,143 @@ void main() {
await tester.restoreFrom(restorationData);
expect(find.byType(AlertDialog), findsOneWidget);
}, skip: isBrowser); // https://github.com/flutter/flutter/issues/33615
testWidgets('FocusTrap moves focus to given focus scope when triggered', (WidgetTester tester) async {
final FocusScopeNode focusScope = FocusScopeNode();
final FocusNode focusNode = FocusNode(debugLabel: 'Test');
await tester.pumpWidget(
Directionality(
textDirection: TextDirection.ltr,
child: FocusScope(
node: focusScope,
child: FocusTrap(
focusScopeNode: focusScope,
child: Column(
children: <Widget>[
const Text('Other Widget'),
FocusTrapTestWidget('Focusable', focusNode: focusNode, onTap: () {
focusNode.requestFocus();
}),
],
),
),
),
),
);
await tester.pump();
Future<void> click(Finder finder) async {
final TestGesture gesture = await tester.startGesture(
tester.getCenter(finder),
kind: PointerDeviceKind.mouse,
);
await gesture.up();
await gesture.removePointer();
}
expect(focusScope.hasFocus, isFalse);
expect(focusNode.hasFocus, isFalse);
await click(find.text('Focusable'));
await tester.pump(const Duration(seconds: 1));
expect(focusScope.hasFocus, isTrue);
expect(focusNode.hasPrimaryFocus, isTrue);
await click(find.text('Other Widget'));
// Have to wait out the double click timer.
await tester.pump(const Duration(seconds: 1));
switch (defaultTargetPlatform) {
case TargetPlatform.iOS:
case TargetPlatform.android:
if (kIsWeb) {
// Web is a desktop platform.
expect(focusScope.hasPrimaryFocus, isTrue);
expect(focusNode.hasFocus, isFalse);
} else {
expect(focusScope.hasFocus, isTrue);
expect(focusNode.hasPrimaryFocus, isTrue);
}
break;
case TargetPlatform.fuchsia:
case TargetPlatform.linux:
case TargetPlatform.macOS:
case TargetPlatform.windows:
expect(focusScope.hasPrimaryFocus, isTrue);
expect(focusNode.hasFocus, isFalse);
break;
}
}, variant: TargetPlatformVariant.all());
testWidgets("FocusTrap doesn't unfocus if focus was set to something else before the frame ends", (WidgetTester tester) async {
final FocusScopeNode focusScope = FocusScopeNode();
final FocusNode focusNode = FocusNode(debugLabel: 'Test');
final FocusNode otherFocusNode = FocusNode(debugLabel: 'Other');
FocusNode? previousFocus;
await tester.pumpWidget(
Directionality(
textDirection: TextDirection.ltr,
child: FocusScope(
node: focusScope,
child: FocusTrap(
focusScopeNode: focusScope,
child: Column(
children: <Widget>[
FocusTrapTestWidget(
'Other Widget',
focusNode: otherFocusNode,
onTap: () {
previousFocus = FocusManager.instance.primaryFocus;
otherFocusNode.requestFocus();
},
),
FocusTrapTestWidget(
'Focusable',
focusNode: focusNode,
onTap: () {
focusNode.requestFocus();
},
),
],
),
),
),
),
);
Future<void> click(Finder finder) async {
final TestGesture gesture = await tester.startGesture(
tester.getCenter(finder),
kind: PointerDeviceKind.mouse,
);
await gesture.up();
await gesture.removePointer();
}
await tester.pump();
expect(focusScope.hasFocus, isFalse);
expect(focusNode.hasPrimaryFocus, isFalse);
await click(find.text('Focusable'));
expect(focusScope.hasFocus, isTrue);
expect(focusNode.hasPrimaryFocus, isTrue);
await click(find.text('Other Widget'));
await tester.pump(const Duration(seconds: 1));
// The previous focus as collected by the "Other Widget" should be the
// previous focus, not be unfocused to the scope, since the primary focus
// was set by something other than the FocusTrap (the "Other Widget") during
// the frame.
expect(previousFocus, equals(focusNode));
expect(focusScope.hasFocus, isTrue);
expect(focusNode.hasPrimaryFocus, isFalse);
expect(otherFocusNode.hasPrimaryFocus, isTrue);
}, variant: TargetPlatformVariant.all());
}
double _getOpacity(GlobalKey key, WidgetTester tester) {
......@@ -2232,3 +2370,68 @@ class _RestorableDialogTestWidget extends StatelessWidget {
);
}
}
class FocusTrapTestWidget extends StatefulWidget {
const FocusTrapTestWidget(
this.label, {
super.key,
required this.focusNode,
this.onTap,
this.autofocus = false,
});
final String label;
final FocusNode focusNode;
final VoidCallback? onTap;
final bool autofocus;
@override
State<FocusTrapTestWidget> createState() => _FocusTrapTestWidgetState();
}
class _FocusTrapTestWidgetState extends State<FocusTrapTestWidget> {
Color color = Colors.white;
@override
void initState() {
super.initState();
widget.focusNode.addListener(_handleFocusChange);
}
void _handleFocusChange() {
if (widget.focusNode.hasPrimaryFocus) {
setState(() {
color = Colors.grey.shade500;
});
} else {
setState(() {
color = Colors.white;
});
}
}
@override
void dispose() {
widget.focusNode.removeListener(_handleFocusChange);
widget.focusNode.dispose();
super.dispose();
}
@override
Widget build(BuildContext context) {
return Focus(
autofocus: widget.autofocus,
focusNode: widget.focusNode,
child: GestureDetector(
onTap: () {
widget.onTap?.call();
},
child: Container(
color: color,
alignment: Alignment.center,
child: Text(widget.label, style: const TextStyle(color: Colors.black)),
),
),
);
}
}
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