From 01c98fa95e42a88d396ba32223403f128ea81b8f Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Tue, 18 May 2021 20:19:03 -0700 Subject: [PATCH] Character activator (#81807) --- .../flutter/lib/src/widgets/shortcuts.dart | 174 +++++++++++++++--- .../flutter/test/widgets/shortcuts_test.dart | 113 +++++++++++- packages/flutter_test/lib/src/controller.dart | 4 +- .../lib/src/event_simulation.dart | 52 +++--- 4 files changed, 289 insertions(+), 54 deletions(-) diff --git a/packages/flutter/lib/src/widgets/shortcuts.dart b/packages/flutter/lib/src/widgets/shortcuts.dart index 39b39d1c899..60f11add3b6 100644 --- a/packages/flutter/lib/src/widgets/shortcuts.dart +++ b/packages/flutter/lib/src/widgets/shortcuts.dart @@ -160,6 +160,8 @@ class KeySet { /// /// * [SingleActivator], an implementation that represents a single key combined /// with modifiers (control, shift, alt, meta). +/// * [CharacterActivator], an implementation that represents key combinations +/// that result in the specified character, such as question mark. /// * [LogicalKeySet], an implementation that requires one or more /// [LogicalKeyboardKey]s to be pressed at the same time. Prefer /// [SingleActivator] when possible. @@ -179,7 +181,13 @@ abstract class ShortcutActivator { /// [Intent]s are stored in a [Map] and indexed by trigger keys. Subclasses /// should make sure that the return value of this method does not change /// throughout the lifespan of this object. - Iterable get triggers; + /// + /// This method might also return null, which means this activator declares + /// all keys as the trigger key. All activators whose [triggers] returns null + /// will be tested with [accepts] on every event. Since this becomes a + /// linear search, and having too many might impact performance, it is + /// preferred to return non-null [triggers] whenever possible. + Iterable? get triggers; /// Whether the triggering `event` and the keyboard `state` at the time of the /// event meet required conditions, providing that the event is a triggering @@ -194,6 +202,9 @@ abstract class ShortcutActivator { /// this is only used to query whether [RawKeyboard.keysPressed] contains /// a key. /// + /// Since [ShortcutActivator] accepts all event types, subclasses might want + /// to check the event type in [accepts]. + /// /// See also: /// /// * [LogicalKeyboardKey.collapseSynonyms], which helps deciding whether a @@ -323,9 +334,6 @@ class LogicalKeySet extends KeySet with Diagnosticable LogicalKeyboardKey.meta: [LogicalKeyboardKey.metaLeft, LogicalKeyboardKey.metaRight], }; - /// Returns a description of the key set that is short and readable. - /// - /// Intended to be used in debug mode for logging purposes. @override String debugDescribeKeys() { final List sortedKeys = keys.toList()..sort( @@ -387,7 +395,7 @@ class ShortcutMapProperty extends DiagnosticsProperty{ +/// CharacterActivator('?'): HelpMenuIntent(), +/// }, +/// child: Actions( +/// actions: >{ +/// HelpMenuIntent: CallbackAction( +/// onInvoke: (HelpMenuIntent intent) { +/// ScaffoldMessenger.of(context).showSnackBar( +/// const SnackBar(content: Text('Keep calm and carry on!')), +/// ); +/// return null; +/// }, +/// ), +/// }, +/// child: Focus( +/// autofocus: true, +/// child: Column( +/// children: const [ +/// Text('Press question mark for help'), +/// ], +/// ), +/// ), +/// ), +/// ); +/// } +/// ``` +/// {@end-tool} +/// +/// See also: +/// +/// * [SingleActivator], an activator that represents a single key combined +/// with modifiers, such as `Ctrl+C`. +class CharacterActivator with Diagnosticable implements ShortcutActivator { + /// Create a [CharacterActivator] from the triggering character. + const CharacterActivator(this.character); + + /// The character of the triggering event. + /// + /// This is typically a single-character string, such as '?' or 'œ', although + /// [CharacterActivator] doesn't check the length of [character] or whether it + /// can be matched by any key combination at all. It is case-sensitive, since + /// the [character] is directly compared by `==` to the character reported by + /// the platform. + /// + /// See also: + /// + /// * [RawKeyEvent.character], the character of a key event. + final String character; + + @override + Iterable? get triggers => null; + + @override + bool accepts(RawKeyEvent event, RawKeyboard state) { + return event is RawKeyDownEvent + && event.character == character; + } + + @override + String debugDescribeKeys() { + String result = ''; + assert(() { + result = "'$character'"; + return true; + }()); + return result; + } + + @override + void debugFillProperties(DiagnosticPropertiesBuilder properties) { + super.debugFillProperties(properties); + properties.add(StringProperty('character', character)); + } +} + class _ActivatorIntentPair with Diagnosticable { const _ActivatorIntentPair(this.activator, this.intent); final ShortcutActivator activator; @@ -639,20 +756,22 @@ class ShortcutManager extends ChangeNotifier with Diagnosticable { } } - static Map> _indexShortcuts(Map source) { - final Map> result = >{}; + static Map> _indexShortcuts(Map source) { + final Map> result = >{}; source.forEach((ShortcutActivator activator, Intent intent) { - for (final LogicalKeyboardKey trigger in activator.triggers) { + // This intermediate variable is necessary to comply with Dart analyzer. + final Iterable? nullableTriggers = activator.triggers; + for (final LogicalKeyboardKey? trigger in nullableTriggers ?? [null]) { result.putIfAbsent(trigger, () => <_ActivatorIntentPair>[]) .add(_ActivatorIntentPair(activator, intent)); } }); return result; } - Map> get _indexedShortcuts { + Map> get _indexedShortcuts { return _indexedShortcutsCache ??= _indexShortcuts(_shortcuts); } - Map>? _indexedShortcutsCache; + Map>? _indexedShortcutsCache; /// Returns the [Intent], if any, that matches the current set of pressed /// keys. @@ -662,9 +781,12 @@ class ShortcutManager extends ChangeNotifier with Diagnosticable { /// Defaults to a set derived from [RawKeyboard.keysPressed] if `keysPressed` /// is not supplied. Intent? _find(RawKeyEvent event, RawKeyboard state) { - final List<_ActivatorIntentPair>? candidates = _indexedShortcuts[event.logicalKey]; - if (candidates == null) - return null; + final List<_ActivatorIntentPair>? candidatesByKey = _indexedShortcuts[event.logicalKey]; + final List<_ActivatorIntentPair>? candidatesByNull = _indexedShortcuts[null]; + final List<_ActivatorIntentPair> candidates = <_ActivatorIntentPair>[ + if (candidatesByKey != null) ...candidatesByKey, + if (candidatesByNull != null) ...candidatesByNull, + ]; for (final _ActivatorIntentPair activatorIntent in candidates) { if (activatorIntent.activator.accepts(event, state)) { return activatorIntent.intent; diff --git a/packages/flutter/test/widgets/shortcuts_test.dart b/packages/flutter/test/widgets/shortcuts_test.dart index bf8204bca16..50cf54abe93 100644 --- a/packages/flutter/test/widgets/shortcuts_test.dart +++ b/packages/flutter/test/widgets/shortcuts_test.dart @@ -9,7 +9,7 @@ import 'package:flutter_test/flutter_test.dart'; typedef PostInvokeCallback = void Function({Action action, Intent intent, BuildContext? context, ActionDispatcher dispatcher}); -class TestAction extends CallbackAction { +class TestAction extends CallbackAction { TestAction({ required OnInvokeCallback onInvoke, }) : assert(onInvoke != null), @@ -31,10 +31,47 @@ class TestDispatcher extends ActionDispatcher { } } +/// An activator that accepts down events that has [key] as the logical key. +/// +/// This class is used only to tests. It is intentionally designed poorly by +/// returning null in [triggers], and checks [key] in [accepts]. +class DumbLogicalActivator extends ShortcutActivator { + const DumbLogicalActivator(this.key); + + final LogicalKeyboardKey key; + + @override + Iterable? get triggers => null; + + @override + bool accepts(RawKeyEvent event, RawKeyboard state) { + return event is RawKeyDownEvent + && event.logicalKey == key; + } + + /// Returns a short and readable description of the key combination. + /// + /// Intended to be used in debug mode for logging purposes. In release mode, + /// [debugDescribeKeys] returns an empty string. + @override + String debugDescribeKeys() { + String result = ''; + assert(() { + result = key.keyLabel; + return true; + }()); + return result; + } +} + class TestIntent extends Intent { const TestIntent(); } +class TestIntent2 extends Intent { + const TestIntent2(); +} + class TestShortcutManager extends ShortcutManager { TestShortcutManager(this.keys); @@ -49,7 +86,13 @@ class TestShortcutManager extends ShortcutManager { } } -Widget activatorTester(ShortcutActivator activator, ValueSetter onInvoke) { +Widget activatorTester( + ShortcutActivator activator, + ValueSetter onInvoke, [ + ShortcutActivator? activator2, + ValueSetter? onInvoke2, +]) { + final bool hasSecond = activator2 != null && onInvoke2 != null; return Actions( key: GlobalKey(), actions: >{ @@ -57,10 +100,16 @@ Widget activatorTester(ShortcutActivator activator, ValueSetter onInvoke onInvoke(intent); return true; }), + if (hasSecond) + TestIntent2: TestAction(onInvoke: (Intent intent) { + onInvoke2(intent); + }), }, child: Shortcuts( shortcuts: { activator: const TestIntent(), + if (hasSecond) + activator2: const TestIntent2(), }, child: const Focus( autofocus: true, @@ -967,5 +1016,65 @@ void main() { expect(value, isTrue); expect(controller.position.pixels, 0.0); }); + + testWidgets('Shortcuts support activators that returns null in triggers', (WidgetTester tester) async { + int invoked = 0; + await tester.pumpWidget(activatorTester( + const DumbLogicalActivator(LogicalKeyboardKey.keyC), + (Intent intent) { invoked += 1; }, + const SingleActivator(LogicalKeyboardKey.keyC, control: true), + (Intent intent) { invoked += 10; }, + )); + await tester.pump(); + + // Press KeyC: Accepted by DumbLogicalActivator + await tester.sendKeyDownEvent(LogicalKeyboardKey.keyC); + expect(invoked, 1); + await tester.sendKeyUpEvent(LogicalKeyboardKey.keyC); + expect(invoked, 1); + invoked = 0; + + // Press ControlLeft + KeyC: Accepted by SingleActivator + await tester.sendKeyDownEvent(LogicalKeyboardKey.controlLeft); + expect(invoked, 0); + await tester.sendKeyDownEvent(LogicalKeyboardKey.keyC); + expect(invoked, 10); + await tester.sendKeyUpEvent(LogicalKeyboardKey.keyC); + await tester.sendKeyUpEvent(LogicalKeyboardKey.controlLeft); + expect(invoked, 10); + invoked = 0; + + // Press ControlLeft + ShiftLeft + KeyC: Accepted by DumbLogicalActivator + await tester.sendKeyDownEvent(LogicalKeyboardKey.shiftLeft); + await tester.sendKeyDownEvent(LogicalKeyboardKey.controlLeft); + expect(invoked, 0); + await tester.sendKeyDownEvent(LogicalKeyboardKey.keyC); + expect(invoked, 1); + await tester.sendKeyUpEvent(LogicalKeyboardKey.keyC); + await tester.sendKeyUpEvent(LogicalKeyboardKey.controlLeft); + await tester.sendKeyUpEvent(LogicalKeyboardKey.shiftLeft); + expect(invoked, 1); + invoked = 0; + }); + }); + + group('CharacterActivator', () { + testWidgets('is triggered on events with correct character', (WidgetTester tester) async { + int invoked = 0; + await tester.pumpWidget(activatorTester( + const CharacterActivator('?'), + (Intent intent) { invoked += 1; }, + )); + await tester.pump(); + + // Press KeyC: Accepted by DumbLogicalActivator + await tester.sendKeyDownEvent(LogicalKeyboardKey.shiftLeft); + await tester.sendKeyDownEvent(LogicalKeyboardKey.slash, character: '?'); + expect(invoked, 1); + await tester.sendKeyUpEvent(LogicalKeyboardKey.slash); + await tester.sendKeyUpEvent(LogicalKeyboardKey.shiftLeft); + expect(invoked, 1); + invoked = 0; + }); }); } diff --git a/packages/flutter_test/lib/src/controller.dart b/packages/flutter_test/lib/src/controller.dart index 4482b1b36b8..5627f70cc9f 100644 --- a/packages/flutter_test/lib/src/controller.dart +++ b/packages/flutter_test/lib/src/controller.dart @@ -1021,10 +1021,10 @@ abstract class WidgetController { /// /// - [sendKeyUpEvent] to simulate the corresponding key up event. /// - [sendKeyEvent] to simulate both the key up and key down in the same call. - Future sendKeyDownEvent(LogicalKeyboardKey key, { String platform = _defaultPlatform }) async { + Future sendKeyDownEvent(LogicalKeyboardKey key, { String? character, String platform = _defaultPlatform }) async { assert(platform != null); // Internally wrapped in async guard. - return simulateKeyDownEvent(key, platform: platform); + return simulateKeyDownEvent(key, character: character, platform: platform); } /// Simulates sending a physical key up event through the system channel. diff --git a/packages/flutter_test/lib/src/event_simulation.dart b/packages/flutter_test/lib/src/event_simulation.dart index 121c20d17cf..ce9504b60c8 100644 --- a/packages/flutter_test/lib/src/event_simulation.dart +++ b/packages/flutter_test/lib/src/event_simulation.dart @@ -194,6 +194,7 @@ class KeyEventSimulator { required String platform, bool isDown = true, PhysicalKeyboardKey? physicalKey, + String? character, }) { assert(_osIsSupported(platform), 'Platform $platform not supported for key simulation'); @@ -211,27 +212,31 @@ class KeyEventSimulator { 'keymap': platform, }; - if (kIsWeb) { + final String resultCharacter = character ?? _keyLabel(key); + void assignWeb() { result['code'] = _getWebKeyCode(key); - result['key'] = _keyLabel(key); + result['key'] = resultCharacter; result['metaState'] = _getWebModifierFlags(key, isDown); + } + if (kIsWeb) { + assignWeb(); return result; } switch (platform) { case 'android': result['keyCode'] = keyCode; - if (_keyLabel(key).isNotEmpty) { - result['codePoint'] = _keyLabel(key).codeUnitAt(0); - result['character'] = _keyLabel(key); + if (resultCharacter.isNotEmpty) { + result['codePoint'] = resultCharacter.codeUnitAt(0); + result['character'] = resultCharacter; } result['scanCode'] = scanCode; result['metaState'] = _getAndroidModifierFlags(key, isDown); break; case 'fuchsia': result['hidUsage'] = physicalKey.usbHidUsage; - if (_keyLabel(key).isNotEmpty) { - result['codePoint'] = _keyLabel(key).codeUnitAt(0); + if (resultCharacter.isNotEmpty) { + result['codePoint'] = resultCharacter.codeUnitAt(0); } result['modifiers'] = _getFuchsiaModifierFlags(key, isDown); break; @@ -240,34 +245,33 @@ class KeyEventSimulator { result['keyCode'] = keyCode; result['scanCode'] = scanCode; result['modifiers'] = _getGlfwModifierFlags(key, isDown); - result['unicodeScalarValues'] = _keyLabel(key).isNotEmpty ? _keyLabel(key).codeUnitAt(0) : 0; + result['unicodeScalarValues'] = resultCharacter.isNotEmpty ? resultCharacter.codeUnitAt(0) : 0; break; case 'macos': result['keyCode'] = scanCode; - if (_keyLabel(key).isNotEmpty) { - result['characters'] = _keyLabel(key); - result['charactersIgnoringModifiers'] = _keyLabel(key); + if (resultCharacter.isNotEmpty) { + result['characters'] = resultCharacter; + result['charactersIgnoringModifiers'] = resultCharacter; } result['modifiers'] = _getMacOsModifierFlags(key, isDown); break; case 'ios': result['keyCode'] = scanCode; - result['characters'] = _keyLabel(key); - result['charactersIgnoringModifiers'] = _keyLabel(key); + result['characters'] = resultCharacter; + result['charactersIgnoringModifiers'] = resultCharacter; result['modifiers'] = _getIOSModifierFlags(key, isDown); break; - case 'web': - result['code'] = _getWebKeyCode(key); - result['key'] = _keyLabel(key); - result['metaState'] = _getWebModifierFlags(key, isDown); - break; case 'windows': result['keyCode'] = keyCode; result['scanCode'] = scanCode; - if (_keyLabel(key).isNotEmpty) { - result['characterCodePoint'] = _keyLabel(key).codeUnitAt(0); + if (resultCharacter.isNotEmpty) { + result['characterCodePoint'] = resultCharacter.codeUnitAt(0); } result['modifiers'] = _getWindowsModifierFlags(key, isDown); + break; + case 'web': + assignWeb(); + break; } return result; } @@ -631,12 +635,12 @@ class KeyEventSimulator { /// See also: /// /// - [simulateKeyUpEvent] to simulate the corresponding key up event. - static Future simulateKeyDownEvent(LogicalKeyboardKey key, {String? platform, PhysicalKeyboardKey? physicalKey}) async { + static Future simulateKeyDownEvent(LogicalKeyboardKey key, {String? platform, PhysicalKeyboardKey? physicalKey, String? character}) async { return TestAsyncUtils.guard(() async { platform ??= Platform.operatingSystem; assert(_osIsSupported(platform!), 'Platform $platform not supported for key simulation'); - final Map data = getKeyData(key, platform: platform!, isDown: true, physicalKey: physicalKey); + final Map data = getKeyData(key, platform: platform!, isDown: true, physicalKey: physicalKey, character: character); bool result = false; await ServicesBinding.instance!.defaultBinaryMessenger.handlePlatformMessage( SystemChannels.keyEvent.name, @@ -715,8 +719,8 @@ class KeyEventSimulator { /// See also: /// /// - [simulateKeyUpEvent] to simulate the corresponding key up event. -Future simulateKeyDownEvent(LogicalKeyboardKey key, {String? platform, PhysicalKeyboardKey? physicalKey}) { - return KeyEventSimulator.simulateKeyDownEvent(key, platform: platform, physicalKey: physicalKey); +Future simulateKeyDownEvent(LogicalKeyboardKey key, {String? platform, PhysicalKeyboardKey? physicalKey, String? character}) { + return KeyEventSimulator.simulateKeyDownEvent(key, platform: platform, physicalKey: physicalKey, character: character); } /// Simulates sending a hardware key up event through the system channel.