From 0351c74aeba1ce0d57fbc4587ef60717ee5a3d85 Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Mon, 14 Dec 2020 18:58:03 -0800 Subject: [PATCH] Make web buttons respond to enter key (#72162) --- .../flutter/lib/src/material/dropdown.dart | 3 + .../flutter/lib/src/material/ink_well.dart | 3 +- packages/flutter/lib/src/widgets/actions.dart | 31 +++++++++- packages/flutter/lib/src/widgets/app.dart | 11 ++-- .../flutter/test/material/dropdown_test.dart | 62 +++++++++++++++++-- .../flutter/test/material/ink_well_test.dart | 33 ++++++++++ 6 files changed, 131 insertions(+), 12 deletions(-) diff --git a/packages/flutter/lib/src/material/dropdown.dart b/packages/flutter/lib/src/material/dropdown.dart index 55c752572ae..fea4feaa0b6 100644 --- a/packages/flutter/lib/src/material/dropdown.dart +++ b/packages/flutter/lib/src/material/dropdown.dart @@ -1113,6 +1113,9 @@ class _DropdownButtonState extends State> with WidgetsBindi ActivateIntent: CallbackAction( onInvoke: (ActivateIntent intent) => _handleTap(), ), + ButtonActivateIntent: CallbackAction( + onInvoke: (ButtonActivateIntent intent) => _handleTap(), + ), }; focusNode!.addListener(_handleFocusChanged); final FocusManager focusManager = WidgetsBinding.instance!.focusManager; diff --git a/packages/flutter/lib/src/material/ink_well.dart b/packages/flutter/lib/src/material/ink_well.dart index b5e60cf820e..e5409c9522a 100644 --- a/packages/flutter/lib/src/material/ink_well.dart +++ b/packages/flutter/lib/src/material/ink_well.dart @@ -734,6 +734,7 @@ class _InkResponseState extends State<_InkResponseStateWidget> final Map<_HighlightType, InkHighlight?> _highlights = <_HighlightType, InkHighlight?>{}; late final Map> _actionMap = >{ ActivateIntent: CallbackAction(onInvoke: _simulateTap), + ButtonActivateIntent: CallbackAction(onInvoke: _simulateTap), }; bool get highlightsExist => _highlights.values.where((InkHighlight? highlight) => highlight != null).isNotEmpty; @@ -756,7 +757,7 @@ class _InkResponseState extends State<_InkResponseStateWidget> } bool get _anyChildInkResponsePressed => _activeChildren.isNotEmpty; - void _simulateTap([ActivateIntent? intent]) { + void _simulateTap([Intent? intent]) { _startSplash(context: context); _handleTap(); } diff --git a/packages/flutter/lib/src/widgets/actions.dart b/packages/flutter/lib/src/widgets/actions.dart index da80e23e75b..21468e5ed13 100644 --- a/packages/flutter/lib/src/widgets/actions.dart +++ b/packages/flutter/lib/src/widgets/actions.dart @@ -1248,12 +1248,41 @@ class DoNothingAction extends Action { void invoke(Intent intent) {} } -/// An intent that activates the currently focused control. +/// An [Intent] that activates the currently focused control. +/// +/// This intent is bound by default to the [LogicalKeyboardKey.space] key on all +/// platforms, and also to the [LogicalKeyboardKey.enter] key on all platforms +/// except the web, where ENTER doesn't toggle selection. On the web, ENTER is +/// bound to [ButtonActivateIntent] instead. +/// +/// See also: +/// +/// * [WidgetsApp.defaultShortcuts], which contains the default shortcuts used +/// in apps. +/// * [WidgetsApp.shortcuts], which defines the shortcuts to use in an +/// application (and defaults to [WidgetsApp.defaultShortcuts]). class ActivateIntent extends Intent { /// Creates a const [ActivateIntent] so subclasses can be const. const ActivateIntent(); } +/// An [Intent] that activates the currently focused button. +/// +/// This intent is bound by default to the [LogicalKeyboardKey.enter] key on the +/// web, where ENTER can be used to activate buttons, but not toggle selection. +/// All other platforms bind [LogicalKeyboardKey.enter] to [ActivateIntent]. +/// +/// See also: +/// +/// * [WidgetsApp.defaultShortcuts], which contains the default shortcuts used +/// in apps. +/// * [WidgetsApp.shortcuts], which defines the shortcuts to use in an +/// application (and defaults to [WidgetsApp.defaultShortcuts]). +class ButtonActivateIntent extends Intent { + /// Creates a const [ButtonActivateIntent] so subclasses can be const. + const ButtonActivateIntent(); +} + /// An action that activates the currently focused control. /// /// This is an abstract class that serves as a base class for actions that diff --git a/packages/flutter/lib/src/widgets/app.dart b/packages/flutter/lib/src/widgets/app.dart index 0bd2d14c938..3cf1fb7e017 100644 --- a/packages/flutter/lib/src/widgets/app.dart +++ b/packages/flutter/lib/src/widgets/app.dart @@ -1028,6 +1028,8 @@ class WidgetsApp extends StatefulWidget { static final Map _defaultWebShortcuts = { // Activation LogicalKeySet(LogicalKeyboardKey.space): const ActivateIntent(), + // On the web, enter activates buttons, but not other controls. + LogicalKeySet(LogicalKeyboardKey.enter): const ButtonActivateIntent(), // Dismissal LogicalKeySet(LogicalKeyboardKey.escape): const DismissIntent(), @@ -1046,7 +1048,7 @@ class WidgetsApp extends StatefulWidget { }; // Default shortcuts for the macOS platform. - static final Map _defaultMacOsShortcuts = { + static final Map _defaultAppleOsShortcuts = { // Activation LogicalKeySet(LogicalKeyboardKey.enter): const ActivateIntent(), LogicalKeySet(LogicalKeyboardKey.space): const ActivateIntent(), @@ -1086,13 +1088,10 @@ class WidgetsApp extends StatefulWidget { case TargetPlatform.linux: case TargetPlatform.windows: return _defaultShortcuts; - case TargetPlatform.macOS: - return _defaultMacOsShortcuts; case TargetPlatform.iOS: - // No keyboard support on iOS yet. - break; + case TargetPlatform.macOS: + return _defaultAppleOsShortcuts; } - return {}; } /// The default value of [WidgetsApp.actions]. diff --git a/packages/flutter/test/material/dropdown_test.dart b/packages/flutter/test/material/dropdown_test.dart index 75decc2c977..91725d269c6 100644 --- a/packages/flutter/test/material/dropdown_test.dart +++ b/packages/flutter/test/material/dropdown_test.dart @@ -2358,7 +2358,7 @@ void main() { expect(find.byKey(buttonKey), isNot(paints ..rrect(rrect: const RRect.fromLTRBXY(0.0, 0.0, 104.0, 48.0, 4.0, 4.0), color: const Color(0xff00ff00)))); }); - testWidgets('DropdownButton is activated with the enter/space key', (WidgetTester tester) async { + testWidgets('DropdownButton is activated with the enter key', (WidgetTester tester) async { final FocusNode focusNode = FocusNode(debugLabel: 'DropdownButton'); String? value = 'one'; @@ -2397,15 +2397,69 @@ void main() { await tester.pump(); // Pump a frame for autofocus to take effect. expect(focusNode.hasPrimaryFocus, isTrue); - // Web doesn't respond to enter, only space. - await tester.sendKeyEvent(kIsWeb ? LogicalKeyboardKey.space : LogicalKeyboardKey.enter); + await tester.sendKeyEvent(LogicalKeyboardKey.enter); await tester.pump(); await tester.pump(const Duration(seconds: 1)); // finish the menu animation expect(value, equals('one')); await tester.sendKeyEvent(LogicalKeyboardKey.tab); // Focus 'two' await tester.pump(); - await tester.sendKeyEvent(LogicalKeyboardKey.enter); // Select 'two', should work on web too. + await tester.sendKeyEvent(LogicalKeyboardKey.enter); // Select 'two'. + await tester.pump(); + + await tester.pump(); + await tester.pump(const Duration(seconds: 1)); // finish the menu animation + + expect(value, equals('two')); + }); + + testWidgets('DropdownButton is activated with the space key', (WidgetTester tester) async { + final FocusNode focusNode = FocusNode(debugLabel: 'DropdownButton'); + String? value = 'one'; + + Widget buildFrame() { + return MaterialApp( + home: Scaffold( + body: Center( + child: StatefulBuilder( + builder: (BuildContext context, StateSetter setState) { + return DropdownButton( + focusNode: focusNode, + autofocus: true, + onChanged: (String? newValue) { + setState(() { + value = newValue; + }); + }, + value: value, + itemHeight: null, + items: menuItems.map>((String item) { + return DropdownMenuItem( + key: ValueKey(item), + value: item, + child: Text(item, key: ValueKey(item + 'Text')), + ); + }).toList(), + ); + }, + ), + ), + ), + ); + } + + await tester.pumpWidget(buildFrame()); + await tester.pump(); // Pump a frame for autofocus to take effect. + expect(focusNode.hasPrimaryFocus, isTrue); + + await tester.sendKeyEvent(LogicalKeyboardKey.space); + await tester.pump(); + await tester.pump(const Duration(seconds: 1)); // finish the menu animation + expect(value, equals('one')); + + await tester.sendKeyEvent(LogicalKeyboardKey.tab); // Focus 'two' + await tester.pump(); + await tester.sendKeyEvent(LogicalKeyboardKey.space); // Select 'two'. await tester.pump(); await tester.pump(); diff --git a/packages/flutter/test/material/ink_well_test.dart b/packages/flutter/test/material/ink_well_test.dart index ff041939faa..9d44d343c7b 100644 --- a/packages/flutter/test/material/ink_well_test.dart +++ b/packages/flutter/test/material/ink_well_test.dart @@ -4,6 +4,7 @@ import 'package:flutter/material.dart'; import 'package:flutter/rendering.dart'; +import 'package:flutter/src/services/keyboard_key.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:flutter/gestures.dart'; @@ -75,6 +76,38 @@ void main() { expect(log, equals(['tap-down', 'tap-cancel'])); }); + testWidgets('InkWell invokes activation actions when expected', (WidgetTester tester) async { + final List log = []; + + await tester.pumpWidget(Directionality( + textDirection: TextDirection.ltr, + child: Shortcuts( + shortcuts: { + LogicalKeySet(LogicalKeyboardKey.space): const ActivateIntent(), + LogicalKeySet(LogicalKeyboardKey.enter): const ButtonActivateIntent(), + }, + child: Material( + child: Center( + child: InkWell( + autofocus: true, + onTap: () { + log.add('tap'); + }, + ), + ), + ), + ), + )); + + await tester.sendKeyEvent(LogicalKeyboardKey.space); + await tester.pump(); + expect(log, equals(['tap'])); + log.clear(); + await tester.sendKeyEvent(LogicalKeyboardKey.enter); + await tester.pump(); + expect(log, equals(['tap'])); + }); + testWidgets('long-press and tap on disabled should not throw', (WidgetTester tester) async { await tester.pumpWidget(const Material( child: Directionality(