Unverified Commit 3afdd08a authored by Greg Spencer's avatar Greg Spencer Committed by GitHub

This disables the up/down arrow focus navigation in text fields in a different way. (#42790)

In #42533, I disabled the up/down arrows for focus navigation in text fields, but we thought of a better way to do it, so this is that better way.

This change reverts the other change, and instead it tests the context of the node in the action to see if it's an EditableText node. If so, then it doesn't do the navigation action.
parent e007720a
...@@ -599,17 +599,6 @@ class _CupertinoTextFieldState extends State<CupertinoTextField> with AutomaticK ...@@ -599,17 +599,6 @@ class _CupertinoTextFieldState extends State<CupertinoTextField> with AutomaticK
bool get selectionEnabled => widget.selectionEnabled; bool get selectionEnabled => widget.selectionEnabled;
// End of API for TextSelectionGestureDetectorBuilderDelegate. // End of API for TextSelectionGestureDetectorBuilderDelegate.
// Disables all directional focus actions inside of a text field, since up and
// down shouldn't go to another field, even in a single line text field. We
// remap the keys rather than the actions, since someone might want to invoke
// a directional navigation action from another key binding.
final Map<LogicalKeySet, Intent> _disabledNavigationKeys = <LogicalKeySet, Intent>{
LogicalKeySet(LogicalKeyboardKey.arrowUp): const Intent(DoNothingAction.key),
LogicalKeySet(LogicalKeyboardKey.arrowDown): const Intent(DoNothingAction.key),
LogicalKeySet(LogicalKeyboardKey.arrowLeft): const Intent(DoNothingAction.key),
LogicalKeySet(LogicalKeyboardKey.arrowRight): const Intent(DoNothingAction.key),
};
@override @override
void initState() { void initState() {
super.initState(); super.initState();
...@@ -870,8 +859,6 @@ class _CupertinoTextFieldState extends State<CupertinoTextField> with AutomaticK ...@@ -870,8 +859,6 @@ class _CupertinoTextFieldState extends State<CupertinoTextField> with AutomaticK
final Widget paddedEditable = Padding( final Widget paddedEditable = Padding(
padding: widget.padding, padding: widget.padding,
child: RepaintBoundary( child: RepaintBoundary(
child: Shortcuts(
shortcuts: _disabledNavigationKeys,
child: EditableText( child: EditableText(
key: editableTextKey, key: editableTextKey,
controller: controller, controller: controller,
...@@ -916,7 +903,6 @@ class _CupertinoTextFieldState extends State<CupertinoTextField> with AutomaticK ...@@ -916,7 +903,6 @@ class _CupertinoTextFieldState extends State<CupertinoTextField> with AutomaticK
enableInteractiveSelection: widget.enableInteractiveSelection, enableInteractiveSelection: widget.enableInteractiveSelection,
), ),
), ),
),
); );
return Semantics( return Semantics(
......
...@@ -705,17 +705,6 @@ class _TextFieldState extends State<TextField> implements TextSelectionGestureDe ...@@ -705,17 +705,6 @@ class _TextFieldState extends State<TextField> implements TextSelectionGestureDe
bool _isHovering = false; bool _isHovering = false;
// Disables all directional focus actions inside of a text field, since up and
// down shouldn't go to another field, even in a single line text field. We
// remap the keys rather than the actions, since someone might want to invoke
// a directional navigation action from another key binding.
final Map<LogicalKeySet, Intent> _disabledNavigationKeys = <LogicalKeySet, Intent>{
LogicalKeySet(LogicalKeyboardKey.arrowUp): const Intent(DoNothingAction.key),
LogicalKeySet(LogicalKeyboardKey.arrowDown): const Intent(DoNothingAction.key),
LogicalKeySet(LogicalKeyboardKey.arrowLeft): const Intent(DoNothingAction.key),
LogicalKeySet(LogicalKeyboardKey.arrowRight): const Intent(DoNothingAction.key),
};
bool get needsCounter => widget.maxLength != null bool get needsCounter => widget.maxLength != null
&& widget.decoration != null && widget.decoration != null
&& widget.decoration.counterText == null; && widget.decoration.counterText == null;
...@@ -947,8 +936,6 @@ class _TextFieldState extends State<TextField> implements TextSelectionGestureDe ...@@ -947,8 +936,6 @@ class _TextFieldState extends State<TextField> implements TextSelectionGestureDe
} }
Widget child = RepaintBoundary( Widget child = RepaintBoundary(
child: Shortcuts(
shortcuts: _disabledNavigationKeys,
child: EditableText( child: EditableText(
key: editableTextKey, key: editableTextKey,
readOnly: widget.readOnly, readOnly: widget.readOnly,
...@@ -993,7 +980,6 @@ class _TextFieldState extends State<TextField> implements TextSelectionGestureDe ...@@ -993,7 +980,6 @@ class _TextFieldState extends State<TextField> implements TextSelectionGestureDe
scrollController: widget.scrollController, scrollController: widget.scrollController,
scrollPhysics: widget.scrollPhysics, scrollPhysics: widget.scrollPhysics,
), ),
),
); );
if (widget.decoration != null) { if (widget.decoration != null) {
......
...@@ -10,6 +10,7 @@ import 'package:flutter/painting.dart'; ...@@ -10,6 +10,7 @@ import 'package:flutter/painting.dart';
import 'actions.dart'; import 'actions.dart';
import 'basic.dart'; import 'basic.dart';
import 'binding.dart'; import 'binding.dart';
import 'editable_text.dart';
import 'focus_manager.dart'; import 'focus_manager.dart';
import 'framework.dart'; import 'framework.dart';
...@@ -801,7 +802,7 @@ class _RequestFocusActionBase extends Action { ...@@ -801,7 +802,7 @@ class _RequestFocusActionBase extends Action {
FocusNode _previousFocus; FocusNode _previousFocus;
@override @override
void invoke(FocusNode node, Intent tag) { void invoke(FocusNode node, Intent intent) {
_previousFocus = WidgetsBinding.instance.focusManager.primaryFocus; _previousFocus = WidgetsBinding.instance.focusManager.primaryFocus;
node.requestFocus(); node.requestFocus();
} }
...@@ -842,8 +843,8 @@ class RequestFocusAction extends _RequestFocusActionBase { ...@@ -842,8 +843,8 @@ class RequestFocusAction extends _RequestFocusActionBase {
static const LocalKey key = ValueKey<Type>(RequestFocusAction); static const LocalKey key = ValueKey<Type>(RequestFocusAction);
@override @override
void invoke(FocusNode node, Intent tag) { void invoke(FocusNode node, Intent intent) {
super.invoke(node, tag); super.invoke(node, intent);
node.requestFocus(); node.requestFocus();
} }
} }
...@@ -861,8 +862,8 @@ class NextFocusAction extends _RequestFocusActionBase { ...@@ -861,8 +862,8 @@ class NextFocusAction extends _RequestFocusActionBase {
static const LocalKey key = ValueKey<Type>(NextFocusAction); static const LocalKey key = ValueKey<Type>(NextFocusAction);
@override @override
void invoke(FocusNode node, Intent tag) { void invoke(FocusNode node, Intent intent) {
super.invoke(node, tag); super.invoke(node, intent);
node.nextFocus(); node.nextFocus();
} }
} }
...@@ -881,8 +882,8 @@ class PreviousFocusAction extends _RequestFocusActionBase { ...@@ -881,8 +882,8 @@ class PreviousFocusAction extends _RequestFocusActionBase {
static const LocalKey key = ValueKey<Type>(PreviousFocusAction); static const LocalKey key = ValueKey<Type>(PreviousFocusAction);
@override @override
void invoke(FocusNode node, Intent tag) { void invoke(FocusNode node, Intent intent) {
super.invoke(node, tag); super.invoke(node, intent);
node.previousFocus(); node.previousFocus();
} }
} }
...@@ -897,11 +898,19 @@ class PreviousFocusAction extends _RequestFocusActionBase { ...@@ -897,11 +898,19 @@ class PreviousFocusAction extends _RequestFocusActionBase {
class DirectionalFocusIntent extends Intent { class DirectionalFocusIntent extends Intent {
/// Creates a [DirectionalFocusIntent] with a fixed [key], and the given /// Creates a [DirectionalFocusIntent] with a fixed [key], and the given
/// [direction]. /// [direction].
const DirectionalFocusIntent(this.direction) : super(DirectionalFocusAction.key); const DirectionalFocusIntent(this.direction, {this.ignoreTextFields = true})
: assert(ignoreTextFields != null), super(DirectionalFocusAction.key);
/// The direction in which to look for the next focusable node when the /// The direction in which to look for the next focusable node when the
/// associated [DirectionalFocusAction] is invoked. /// associated [DirectionalFocusAction] is invoked.
final TraversalDirection direction; final TraversalDirection direction;
/// If true, then directional focus actions that occur within a text field
/// will not happen when the focus node which received the key is a text
/// field.
///
/// Defaults to true.
final bool ignoreTextFields;
} }
/// An [Action] that moves the focus to the focusable node in the given /// An [Action] that moves the focus to the focusable node in the given
...@@ -922,9 +931,10 @@ class DirectionalFocusAction extends _RequestFocusActionBase { ...@@ -922,9 +931,10 @@ class DirectionalFocusAction extends _RequestFocusActionBase {
TraversalDirection direction; TraversalDirection direction;
@override @override
void invoke(FocusNode node, DirectionalFocusIntent tag) { void invoke(FocusNode node, DirectionalFocusIntent intent) {
super.invoke(node, tag); super.invoke(node, intent);
final DirectionalFocusIntent args = tag; if (!intent.ignoreTextFields || node.context.widget is! EditableText) {
node.focusInDirection(args.direction); node.focusInDirection(intent.direction);
}
} }
} }
// Copyright 2019 The Chromium Authors. All rights reserved. // Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
import 'dart:ui';
import 'package:flutter/painting.dart'; import 'package:flutter/painting.dart';
import 'package:flutter/services.dart'; import 'package:flutter/services.dart';
...@@ -967,17 +968,13 @@ void main() { ...@@ -967,17 +968,13 @@ void main() {
), ),
); );
// Initial focus happens.
expect(Focus.of(upperLeftKey.currentContext).hasPrimaryFocus, isTrue); expect(Focus.of(upperLeftKey.currentContext).hasPrimaryFocus, isTrue);
await tester.sendKeyEvent(LogicalKeyboardKey.tab); await tester.sendKeyEvent(LogicalKeyboardKey.tab);
expect(Focus.of(upperRightKey.currentContext).hasPrimaryFocus, isTrue); expect(Focus.of(upperRightKey.currentContext).hasPrimaryFocus, isTrue);
// Initial focus happens.
await tester.sendKeyEvent(LogicalKeyboardKey.tab); await tester.sendKeyEvent(LogicalKeyboardKey.tab);
expect(Focus.of(lowerLeftKey.currentContext).hasPrimaryFocus, isTrue); expect(Focus.of(lowerLeftKey.currentContext).hasPrimaryFocus, isTrue);
// Initial focus happens.
await tester.sendKeyEvent(LogicalKeyboardKey.tab); await tester.sendKeyEvent(LogicalKeyboardKey.tab);
expect(Focus.of(lowerRightKey.currentContext).hasPrimaryFocus, isTrue); expect(Focus.of(lowerRightKey.currentContext).hasPrimaryFocus, isTrue);
// Initial focus happens.
await tester.sendKeyEvent(LogicalKeyboardKey.tab); await tester.sendKeyEvent(LogicalKeyboardKey.tab);
expect(Focus.of(upperLeftKey.currentContext).hasPrimaryFocus, isTrue); expect(Focus.of(upperLeftKey.currentContext).hasPrimaryFocus, isTrue);
...@@ -985,17 +982,14 @@ void main() { ...@@ -985,17 +982,14 @@ void main() {
await tester.sendKeyEvent(LogicalKeyboardKey.tab); await tester.sendKeyEvent(LogicalKeyboardKey.tab);
await tester.sendKeyUpEvent(LogicalKeyboardKey.shift); await tester.sendKeyUpEvent(LogicalKeyboardKey.shift);
expect(Focus.of(lowerRightKey.currentContext).hasPrimaryFocus, isTrue); expect(Focus.of(lowerRightKey.currentContext).hasPrimaryFocus, isTrue);
// Initial focus happens.
await tester.sendKeyDownEvent(LogicalKeyboardKey.shift); await tester.sendKeyDownEvent(LogicalKeyboardKey.shift);
await tester.sendKeyEvent(LogicalKeyboardKey.tab); await tester.sendKeyEvent(LogicalKeyboardKey.tab);
await tester.sendKeyUpEvent(LogicalKeyboardKey.shift); await tester.sendKeyUpEvent(LogicalKeyboardKey.shift);
expect(Focus.of(lowerLeftKey.currentContext).hasPrimaryFocus, isTrue); expect(Focus.of(lowerLeftKey.currentContext).hasPrimaryFocus, isTrue);
// Initial focus happens.
await tester.sendKeyDownEvent(LogicalKeyboardKey.shift); await tester.sendKeyDownEvent(LogicalKeyboardKey.shift);
await tester.sendKeyEvent(LogicalKeyboardKey.tab); await tester.sendKeyEvent(LogicalKeyboardKey.tab);
await tester.sendKeyUpEvent(LogicalKeyboardKey.shift); await tester.sendKeyUpEvent(LogicalKeyboardKey.shift);
expect(Focus.of(upperRightKey.currentContext).hasPrimaryFocus, isTrue); expect(Focus.of(upperRightKey.currentContext).hasPrimaryFocus, isTrue);
// Initial focus happens.
await tester.sendKeyDownEvent(LogicalKeyboardKey.shift); await tester.sendKeyDownEvent(LogicalKeyboardKey.shift);
await tester.sendKeyEvent(LogicalKeyboardKey.tab); await tester.sendKeyEvent(LogicalKeyboardKey.tab);
await tester.sendKeyUpEvent(LogicalKeyboardKey.shift); await tester.sendKeyUpEvent(LogicalKeyboardKey.shift);
...@@ -1004,16 +998,135 @@ void main() { ...@@ -1004,16 +998,135 @@ void main() {
// Traverse in a direction // Traverse in a direction
await tester.sendKeyEvent(LogicalKeyboardKey.arrowRight); await tester.sendKeyEvent(LogicalKeyboardKey.arrowRight);
expect(Focus.of(upperRightKey.currentContext).hasPrimaryFocus, isTrue); expect(Focus.of(upperRightKey.currentContext).hasPrimaryFocus, isTrue);
// Initial focus happens.
await tester.sendKeyEvent(LogicalKeyboardKey.arrowDown); await tester.sendKeyEvent(LogicalKeyboardKey.arrowDown);
expect(Focus.of(lowerRightKey.currentContext).hasPrimaryFocus, isTrue); expect(Focus.of(lowerRightKey.currentContext).hasPrimaryFocus, isTrue);
// Initial focus happens.
await tester.sendKeyEvent(LogicalKeyboardKey.arrowLeft); await tester.sendKeyEvent(LogicalKeyboardKey.arrowLeft);
expect(Focus.of(lowerLeftKey.currentContext).hasPrimaryFocus, isTrue); expect(Focus.of(lowerLeftKey.currentContext).hasPrimaryFocus, isTrue);
// Initial focus happens.
await tester.sendKeyEvent(LogicalKeyboardKey.arrowUp); await tester.sendKeyEvent(LogicalKeyboardKey.arrowUp);
expect(Focus.of(upperLeftKey.currentContext).hasPrimaryFocus, isTrue); expect(Focus.of(upperLeftKey.currentContext).hasPrimaryFocus, isTrue);
}); });
testWidgets('Arrow focus traversal actions can be re-enabled for text fields.', (WidgetTester tester) async {
final GlobalKey upperLeftKey = GlobalKey(debugLabel: 'upperLeftKey');
final GlobalKey upperRightKey = GlobalKey(debugLabel: 'upperRightKey');
final GlobalKey lowerLeftKey = GlobalKey(debugLabel: 'lowerLeftKey');
final GlobalKey lowerRightKey = GlobalKey(debugLabel: 'lowerRightKey');
final TextEditingController controller1 = TextEditingController();
final TextEditingController controller2 = TextEditingController();
final TextEditingController controller3 = TextEditingController();
final TextEditingController controller4 = TextEditingController();
final FocusNode focusNodeUpperLeft = FocusNode(debugLabel: 'upperLeft');
final FocusNode focusNodeUpperRight = FocusNode(debugLabel: 'upperRight');
final FocusNode focusNodeLowerLeft = FocusNode(debugLabel: 'lowerLeft');
final FocusNode focusNodeLowerRight = FocusNode(debugLabel: 'lowerRight');
Widget generateTestWidgets(bool ignoreTextFields) {
final Map<LogicalKeySet, Intent> shortcuts = <LogicalKeySet, Intent>{
LogicalKeySet(LogicalKeyboardKey.arrowLeft): DirectionalFocusIntent(TraversalDirection.left, ignoreTextFields: ignoreTextFields),
LogicalKeySet(LogicalKeyboardKey.arrowRight): DirectionalFocusIntent(TraversalDirection.right, ignoreTextFields: ignoreTextFields),
LogicalKeySet(LogicalKeyboardKey.arrowDown): DirectionalFocusIntent(TraversalDirection.down, ignoreTextFields: ignoreTextFields),
LogicalKeySet(LogicalKeyboardKey.arrowUp): DirectionalFocusIntent(TraversalDirection.up, ignoreTextFields: ignoreTextFields),
};
return MaterialApp(
home: Shortcuts(
shortcuts: shortcuts,
child: FocusScope(
debugLabel: 'scope',
child: Column(
children: <Widget>[
Row(
children: <Widget>[
Container(
width: 100,
height: 100,
child: EditableText(
autofocus: true,
key: upperLeftKey,
controller: controller1,
focusNode: focusNodeUpperLeft,
cursorColor: const Color(0xffffffff),
backgroundCursorColor: const Color(0xff808080),
style: const TextStyle(),
),
),
Container(
width: 100,
height: 100,
child: EditableText(
key: upperRightKey,
controller: controller2,
focusNode: focusNodeUpperRight,
cursorColor: const Color(0xffffffff),
backgroundCursorColor: const Color(0xff808080),
style: const TextStyle(),
),
),
],
),
Row(
children: <Widget>[
Container(
width: 100,
height: 100,
child: EditableText(
key: lowerLeftKey,
controller: controller3,
focusNode: focusNodeLowerLeft,
cursorColor: const Color(0xffffffff),
backgroundCursorColor: const Color(0xff808080),
style: const TextStyle(),
),
),
Container(
width: 100,
height: 100,
child: EditableText(
key: lowerRightKey,
controller: controller4,
focusNode: focusNodeLowerRight,
cursorColor: const Color(0xffffffff),
backgroundCursorColor: const Color(0xff808080),
style: const TextStyle(),
),
),
],
),
],
),
),
),
);
}
await tester.pumpWidget(generateTestWidgets(false));
expect(focusNodeUpperLeft.hasPrimaryFocus, isTrue);
await tester.sendKeyEvent(LogicalKeyboardKey.arrowRight);
expect(focusNodeUpperRight.hasPrimaryFocus, isTrue);
await tester.sendKeyEvent(LogicalKeyboardKey.arrowDown);
expect(focusNodeLowerRight.hasPrimaryFocus, isTrue);
await tester.sendKeyEvent(LogicalKeyboardKey.arrowLeft);
expect(focusNodeLowerLeft.hasPrimaryFocus, isTrue);
await tester.sendKeyEvent(LogicalKeyboardKey.arrowUp);
expect(focusNodeUpperLeft.hasPrimaryFocus, isTrue);
await tester.pumpWidget(generateTestWidgets(true));
expect(focusNodeUpperLeft.hasPrimaryFocus, isTrue);
await tester.sendKeyEvent(LogicalKeyboardKey.arrowRight);
expect(focusNodeUpperRight.hasPrimaryFocus, isFalse);
expect(focusNodeUpperLeft.hasPrimaryFocus, isTrue);
await tester.sendKeyEvent(LogicalKeyboardKey.arrowDown);
expect(focusNodeLowerRight.hasPrimaryFocus, isFalse);
expect(focusNodeUpperLeft.hasPrimaryFocus, isTrue);
await tester.sendKeyEvent(LogicalKeyboardKey.arrowLeft);
expect(focusNodeLowerLeft.hasPrimaryFocus, isFalse);
expect(focusNodeUpperLeft.hasPrimaryFocus, isTrue);
await tester.sendKeyEvent(LogicalKeyboardKey.arrowUp);
expect(focusNodeUpperLeft.hasPrimaryFocus, 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