From e204eb2fca8e8d26dbc3d24d1d919253d73265ac Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Fri, 22 Jan 2021 16:39:04 -0800 Subject: [PATCH] Space and arrow keys in a text field shouldn't scroll (#74454) --- .../flutter/lib/src/cupertino/text_field.dart | 41 ++++++++-------- .../flutter/lib/src/material/text_field.dart | 47 ++++++++++--------- .../lib/src/widgets/editable_text.dart | 15 ++++++ .../test/cupertino/text_field_test.dart | 33 +++++++++++++ .../test/material/text_field_test.dart | 35 ++++++++++++++ 5 files changed, 130 insertions(+), 41 deletions(-) diff --git a/packages/flutter/lib/src/cupertino/text_field.dart b/packages/flutter/lib/src/cupertino/text_field.dart index c890a88b9b..a3adfbc0f0 100644 --- a/packages/flutter/lib/src/cupertino/text_field.dart +++ b/packages/flutter/lib/src/cupertino/text_field.dart @@ -1201,25 +1201,28 @@ class _CupertinoTextFieldState extends State with Restoratio ), ); - return Semantics( - enabled: enabled, - onTap: !enabled || widget.readOnly ? null : () { - if (!controller.selection.isValid) { - controller.selection = TextSelection.collapsed(offset: controller.text.length); - } - _requestKeyboard(); - }, - child: IgnorePointer( - ignoring: !enabled, - child: Container( - decoration: effectiveDecoration, - child: _selectionGestureDetectorBuilder.buildGestureDetector( - behavior: HitTestBehavior.translucent, - child: Align( - alignment: Alignment(-1.0, _textAlignVertical.y), - widthFactor: 1.0, - heightFactor: 1.0, - child: _addTextDependentAttachments(paddedEditable, textStyle, placeholderStyle), + return Shortcuts( + shortcuts: scrollShortcutOverrides, + child: Semantics( + enabled: enabled, + onTap: !enabled || widget.readOnly ? null : () { + if (!controller.selection.isValid) { + controller.selection = TextSelection.collapsed(offset: controller.text.length); + } + _requestKeyboard(); + }, + child: IgnorePointer( + ignoring: !enabled, + child: Container( + decoration: effectiveDecoration, + child: _selectionGestureDetectorBuilder.buildGestureDetector( + behavior: HitTestBehavior.translucent, + child: Align( + alignment: Alignment(-1.0, _textAlignVertical.y), + widthFactor: 1.0, + heightFactor: 1.0, + child: _addTextDependentAttachments(paddedEditable, textStyle, placeholderStyle), + ), ), ), ), diff --git a/packages/flutter/lib/src/material/text_field.dart b/packages/flutter/lib/src/material/text_field.dart index aa60b566c1..aa71322655 100644 --- a/packages/flutter/lib/src/material/text_field.dart +++ b/packages/flutter/lib/src/material/text_field.dart @@ -1290,29 +1290,32 @@ class _TextFieldState extends State with RestorationMixin implements semanticsMaxValueLength = null; } - return MouseRegion( - cursor: effectiveMouseCursor, - onEnter: (PointerEnterEvent event) => _handleHover(true), - onExit: (PointerExitEvent event) => _handleHover(false), - child: IgnorePointer( - ignoring: !_isEnabled, - child: AnimatedBuilder( - animation: controller, // changes the _currentLength - builder: (BuildContext context, Widget? child) { - return Semantics( - maxValueLength: semanticsMaxValueLength, - currentValueLength: _currentLength, - onTap: widget.readOnly ? null : () { - if (!_effectiveController.selection.isValid) - _effectiveController.selection = TextSelection.collapsed(offset: _effectiveController.text.length); - _requestKeyboard(); - }, + return Shortcuts( + shortcuts: scrollShortcutOverrides, + child: MouseRegion( + cursor: effectiveMouseCursor, + onEnter: (PointerEnterEvent event) => _handleHover(true), + onExit: (PointerExitEvent event) => _handleHover(false), + child: IgnorePointer( + ignoring: !_isEnabled, + child: AnimatedBuilder( + animation: controller, // changes the _currentLength + builder: (BuildContext context, Widget? child) { + return Semantics( + maxValueLength: semanticsMaxValueLength, + currentValueLength: _currentLength, + onTap: widget.readOnly ? null : () { + if (!_effectiveController.selection.isValid) + _effectiveController.selection = TextSelection.collapsed(offset: _effectiveController.text.length); + _requestKeyboard(); + }, + child: child, + ); + }, + child: _selectionGestureDetectorBuilder.buildGestureDetector( + behavior: HitTestBehavior.translucent, child: child, - ); - }, - child: _selectionGestureDetectorBuilder.buildGestureDetector( - behavior: HitTestBehavior.translucent, - child: child, + ), ), ), ), diff --git a/packages/flutter/lib/src/widgets/editable_text.dart b/packages/flutter/lib/src/widgets/editable_text.dart index 7e374ebec5..377d072795 100644 --- a/packages/flutter/lib/src/widgets/editable_text.dart +++ b/packages/flutter/lib/src/widgets/editable_text.dart @@ -12,6 +12,7 @@ import 'package:flutter/rendering.dart'; import 'package:flutter/scheduler.dart'; import 'package:flutter/services.dart'; +import 'actions.dart'; import 'autofill.dart'; import 'automatic_keep_alive.dart'; import 'basic.dart'; @@ -26,6 +27,7 @@ import 'media_query.dart'; import 'scroll_controller.dart'; import 'scroll_physics.dart'; import 'scrollable.dart'; +import 'shortcuts.dart'; import 'text.dart'; import 'text_selection.dart'; import 'ticker_provider.dart'; @@ -53,6 +55,19 @@ const Duration _kCursorBlinkWaitForStart = Duration(milliseconds: 150); // is shown in an obscured text field. const int _kObscureShowLatestCharCursorTicks = 3; +/// A map used to disable scrolling shortcuts in text fields. +/// +/// This is a temporary fix for: https://github.com/flutter/flutter/issues/74191 +final Map scrollShortcutOverrides = kIsWeb + ? { + LogicalKeySet(LogicalKeyboardKey.space): DoNothingAndStopPropagationIntent(), + LogicalKeySet(LogicalKeyboardKey.arrowUp): DoNothingAndStopPropagationIntent(), + LogicalKeySet(LogicalKeyboardKey.arrowDown): DoNothingAndStopPropagationIntent(), + LogicalKeySet(LogicalKeyboardKey.arrowLeft): DoNothingAndStopPropagationIntent(), + LogicalKeySet(LogicalKeyboardKey.arrowRight): DoNothingAndStopPropagationIntent(), + } + : {}; + /// A controller for an editable text field. /// /// Whenever the user modifies a text field with an associated diff --git a/packages/flutter/test/cupertino/text_field_test.dart b/packages/flutter/test/cupertino/text_field_test.dart index eac1111f34..838c77c121 100644 --- a/packages/flutter/test/cupertino/text_field_test.dart +++ b/packages/flutter/test/cupertino/text_field_test.dart @@ -4287,6 +4287,39 @@ void main() { expect(focusNode3.hasPrimaryFocus, isTrue); }); + testWidgets('Scrolling shortcuts are disabled in text fields', (WidgetTester tester) async { + bool scrollInvoked = false; + await tester.pumpWidget( + CupertinoApp( + home: Actions( + actions: >{ + ScrollIntent: CallbackAction(onInvoke: (Intent intent) { + scrollInvoked = true; + }), + }, + child: ListView( + children: const [ + Padding(padding: EdgeInsets.symmetric(vertical: 200)), + CupertinoTextField(), + Padding(padding: EdgeInsets.symmetric(vertical: 800)), + ], + ), + ), + ), + ); + await tester.pump(); + expect(scrollInvoked, isFalse); + + // Set focus on the text field. + await tester.tapAt(tester.getTopLeft(find.byType(CupertinoTextField))); + + await tester.sendKeyEvent(LogicalKeyboardKey.space); + expect(scrollInvoked, isFalse); + + await tester.sendKeyEvent(LogicalKeyboardKey.arrowDown); + expect(scrollInvoked, isFalse); + }); + testWidgets('Cupertino text field semantics', (WidgetTester tester) async { await tester.pumpWidget( CupertinoApp( diff --git a/packages/flutter/test/material/text_field_test.dart b/packages/flutter/test/material/text_field_test.dart index 85851893c5..e7c1b9eab6 100644 --- a/packages/flutter/test/material/text_field_test.dart +++ b/packages/flutter/test/material/text_field_test.dart @@ -8754,6 +8754,41 @@ void main() { expect(focusNode3.hasPrimaryFocus, isTrue); }); + testWidgets('Scrolling shortcuts are disabled in text fields', (WidgetTester tester) async { + bool scrollInvoked = false; + await tester.pumpWidget( + MaterialApp( + home: Actions( + actions: >{ + ScrollIntent: CallbackAction(onInvoke: (Intent intent) { + scrollInvoked = true; + }), + }, + child: Material( + child: ListView( + children: const [ + Padding(padding: EdgeInsets.symmetric(vertical: 200)), + TextField(), + Padding(padding: EdgeInsets.symmetric(vertical: 800)), + ], + ), + ), + ), + ), + ); + await tester.pump(); + expect(scrollInvoked, isFalse); + + // Set focus on the text field. + await tester.tapAt(tester.getTopLeft(find.byType(TextField))); + + await tester.sendKeyEvent(LogicalKeyboardKey.space); + expect(scrollInvoked, isFalse); + + await tester.sendKeyEvent(LogicalKeyboardKey.arrowDown); + expect(scrollInvoked, isFalse); + }); + testWidgets("A buildCounter that returns null doesn't affect the size of the TextField", (WidgetTester tester) async { // Regression test for https://github.com/flutter/flutter/issues/44909