From 7cdcfb2a88d22c0a72060f654b06430ca8845573 Mon Sep 17 00:00:00 2001 From: chunhtai <47866232+chunhtai@users.noreply.github.com> Date: Thu, 5 May 2022 15:39:12 -0700 Subject: [PATCH] DefaultTextEditingShortcuts should use meta-based shortcut for iOS (#103077) --- .../default_text_editing_shortcuts.dart | 23 +---- packages/flutter/test/widgets/app_test.dart | 89 +++++++++++++++++++ .../widgets/editable_text_shortcuts_test.dart | 30 +++---- .../test/widgets/editable_text_test.dart | 44 ++++----- 4 files changed, 130 insertions(+), 56 deletions(-) diff --git a/packages/flutter/lib/src/widgets/default_text_editing_shortcuts.dart b/packages/flutter/lib/src/widgets/default_text_editing_shortcuts.dart index 1bcf7d63b9..5b0ba186ed 100644 --- a/packages/flutter/lib/src/widgets/default_text_editing_shortcuts.dart +++ b/packages/flutter/lib/src/widgets/default_text_editing_shortcuts.dart @@ -231,25 +231,6 @@ class DefaultTextEditingShortcuts extends Shortcuts { static final Map _fuchsiaShortcuts = _androidShortcuts; - // The following key combinations have no effect on text editing on this - // platform: - // * End - // * Home - // * Meta + X - // * Meta + C - // * Meta + V - // * Meta + A - // * Meta + shift? + Z - // * Meta + shift? + arrow down - // * Meta + shift? + arrow left - // * Meta + shift? + arrow right - // * Meta + shift? + arrow up - // * Shift + end - // * Shift + home - // * Meta + shift? + delete - // * Meta + shift? + backspace - static final Map _iOSShortcuts = _commonShortcuts; - static final Map _linuxShortcuts = { ..._commonShortcuts, const SingleActivator(LogicalKeyboardKey.home): const ExtendSelectionToLineBreakIntent(forward: false, collapseSelection: true), @@ -342,6 +323,10 @@ class DefaultTextEditingShortcuts extends Shortcuts { // * Control + shift? + Z }; + // There is no complete documentation of iOS shortcuts. Use mac shortcuts for + // now. + static final Map _iOSShortcuts = _macShortcuts; + // The following key combinations have no effect on text editing on this // platform: // * Meta + X diff --git a/packages/flutter/test/widgets/app_test.dart b/packages/flutter/test/widgets/app_test.dart index d0a7404bb2..7bae0cd641 100644 --- a/packages/flutter/test/widgets/app_test.dart +++ b/packages/flutter/test/widgets/app_test.dart @@ -643,11 +643,100 @@ void main() { ); expect(MediaQuery.of(capturedContext), isNotNull); }); + + testWidgets('WidgetsApp provides meta based shortcuts for iOS and macOS', (WidgetTester tester) async { + final FocusNode focusNode = FocusNode(); + final SelectAllSpy selectAllSpy = SelectAllSpy(); + final CopySpy copySpy = CopySpy(); + final PasteSpy pasteSpy = PasteSpy(); + final Map> actions = >{ + // Copy Paste + SelectAllTextIntent: selectAllSpy, + CopySelectionTextIntent: copySpy, + PasteTextIntent: pasteSpy, + }; + await tester.pumpWidget( + WidgetsApp( + builder: (BuildContext context, Widget? child) { + return Actions( + actions: actions, + child: Focus( + focusNode: focusNode, + child: const Placeholder(), + ), + ); + }, + color: const Color(0xFF123456), + ), + ); + focusNode.requestFocus(); + await tester.pump(); + expect(selectAllSpy.invoked, isFalse); + expect(copySpy.invoked, isFalse); + expect(pasteSpy.invoked, isFalse); + + // Select all. + await tester.sendKeyDownEvent(LogicalKeyboardKey.metaLeft); + await tester.sendKeyDownEvent(LogicalKeyboardKey.keyA); + await tester.sendKeyUpEvent(LogicalKeyboardKey.keyA); + await tester.sendKeyUpEvent(LogicalKeyboardKey.metaLeft); + await tester.pump(); + + expect(selectAllSpy.invoked, isTrue); + expect(copySpy.invoked, isFalse); + expect(pasteSpy.invoked, isFalse); + + // Copy. + await tester.sendKeyDownEvent(LogicalKeyboardKey.metaLeft); + await tester.sendKeyDownEvent(LogicalKeyboardKey.keyC); + await tester.sendKeyUpEvent(LogicalKeyboardKey.keyC); + await tester.sendKeyUpEvent(LogicalKeyboardKey.metaLeft); + await tester.pump(); + + expect(selectAllSpy.invoked, isTrue); + expect(copySpy.invoked, isTrue); + expect(pasteSpy.invoked, isFalse); + + // Paste. + await tester.sendKeyDownEvent(LogicalKeyboardKey.metaLeft); + await tester.sendKeyDownEvent(LogicalKeyboardKey.keyV); + await tester.sendKeyUpEvent(LogicalKeyboardKey.keyV); + await tester.sendKeyUpEvent(LogicalKeyboardKey.metaLeft); + await tester.pump(); + + expect(selectAllSpy.invoked, isTrue); + expect(copySpy.invoked, isTrue); + expect(pasteSpy.invoked, isTrue); + }, variant: const TargetPlatformVariant({ TargetPlatform.iOS, TargetPlatform.macOS }), skip: kIsWeb); // [intended] Web uses a different set of shortcuts. } typedef SimpleRouterDelegateBuilder = Widget Function(BuildContext, RouteInformation); typedef SimpleNavigatorRouterDelegatePopPage = bool Function(Route route, T result, SimpleNavigatorRouterDelegate delegate); +class SelectAllSpy extends Action { + bool invoked = false; + @override + void invoke(SelectAllTextIntent intent) { + invoked = true; + } +} + +class CopySpy extends Action { + bool invoked = false; + @override + void invoke(CopySelectionTextIntent intent) { + invoked = true; + } +} + +class PasteSpy extends Action { + bool invoked = false; + @override + void invoke(PasteTextIntent intent) { + invoked = true; + } +} + class SimpleRouteInformationParser extends RouteInformationParser { SimpleRouteInformationParser(); diff --git a/packages/flutter/test/widgets/editable_text_shortcuts_test.dart b/packages/flutter/test/widgets/editable_text_shortcuts_test.dart index e0f23a78dc..4c600067f5 100644 --- a/packages/flutter/test/widgets/editable_text_shortcuts_test.dart +++ b/packages/flutter/test/widgets/editable_text_shortcuts_test.dart @@ -124,7 +124,7 @@ void main() { group('Common text editing shortcuts: ', () { - final TargetPlatformVariant allExceptMacOS = TargetPlatformVariant(TargetPlatform.values.toSet()..remove(TargetPlatform.macOS)); + final TargetPlatformVariant allExceptApple = TargetPlatformVariant(TargetPlatform.values.toSet()..removeAll([TargetPlatform.macOS, TargetPlatform.iOS])); group('backspace', () { const LogicalKeyboardKey trigger = LogicalKeyboardKey.backspace; @@ -491,8 +491,8 @@ void main() { group('word modifier + backspace', () { const LogicalKeyboardKey trigger = LogicalKeyboardKey.backspace; SingleActivator wordModifierBackspace() { - final bool isMacOS = defaultTargetPlatform == TargetPlatform.macOS; - return SingleActivator(trigger, control: !isMacOS, alt: isMacOS); + final bool isApple = defaultTargetPlatform == TargetPlatform.macOS || defaultTargetPlatform == TargetPlatform.iOS; + return SingleActivator(trigger, control: !isApple, alt: isApple); } testWidgets('WordModifier-backspace', (WidgetTester tester) async { @@ -631,8 +631,8 @@ void main() { group('word modifier + delete', () { const LogicalKeyboardKey trigger = LogicalKeyboardKey.delete; SingleActivator wordModifierDelete() { - final bool isMacOS = defaultTargetPlatform == TargetPlatform.macOS; - return SingleActivator(trigger, control: !isMacOS, alt: isMacOS); + final bool isApple = defaultTargetPlatform == TargetPlatform.macOS || defaultTargetPlatform == TargetPlatform.iOS; + return SingleActivator(trigger, control: !isApple, alt: isApple); } testWidgets('WordModifier-delete', (WidgetTester tester) async { @@ -764,8 +764,8 @@ void main() { group('line modifier + backspace', () { const LogicalKeyboardKey trigger = LogicalKeyboardKey.backspace; SingleActivator lineModifierBackspace() { - final bool isMacOS = defaultTargetPlatform == TargetPlatform.macOS; - return SingleActivator(trigger, meta: isMacOS, alt: !isMacOS); + final bool isApple = defaultTargetPlatform == TargetPlatform.macOS || defaultTargetPlatform == TargetPlatform.iOS; + return SingleActivator(trigger, meta: isApple, alt: !isApple); } testWidgets('alt-backspace', (WidgetTester tester) async { @@ -945,8 +945,8 @@ void main() { group('line modifier + delete', () { const LogicalKeyboardKey trigger = LogicalKeyboardKey.delete; SingleActivator lineModifierDelete() { - final bool isMacOS = defaultTargetPlatform == TargetPlatform.macOS; - return SingleActivator(trigger, meta: isMacOS, alt: !isMacOS); + final bool isApple = defaultTargetPlatform == TargetPlatform.macOS || defaultTargetPlatform == TargetPlatform.iOS; + return SingleActivator(trigger, meta: isApple, alt: !isApple); } testWidgets('alt-delete', (WidgetTester tester) async { @@ -1167,7 +1167,7 @@ void main() { expect(controller.selection, const TextSelection.collapsed( offset: 4, )); - }, variant: allExceptMacOS); + }, variant: allExceptApple); testWidgets('line modifier + arrow key movement', (WidgetTester tester) async { controller.text = testText; @@ -1181,7 +1181,7 @@ void main() { expect(controller.selection, const TextSelection.collapsed( offset: 20, )); - }, variant: allExceptMacOS); + }, variant: allExceptApple); }); group('right', () { @@ -1230,7 +1230,7 @@ void main() { expect(controller.selection, const TextSelection.collapsed( offset: 10, )); - }, variant: allExceptMacOS); + }, variant: allExceptApple); testWidgets('line modifier + arrow key movement', (WidgetTester tester) async { controller.text = testText; @@ -1245,7 +1245,7 @@ void main() { offset: 35, // Before the newline character. affinity: TextAffinity.upstream, )); - }, variant: allExceptMacOS); + }, variant: allExceptApple); }); group('With initial non-collapsed selection', () { @@ -1352,7 +1352,7 @@ void main() { expect(controller.selection, const TextSelection.collapsed( offset: 28, // After "good". )); - }, variant: allExceptMacOS); + }, variant: allExceptApple); testWidgets('line modifier + arrow key movement', (WidgetTester tester) async { controller.text = testText; @@ -1406,7 +1406,7 @@ void main() { offset: 35, // After "people". affinity: TextAffinity.upstream, )); - }, variant: allExceptMacOS); + }, variant: allExceptApple); }); group('vertical movement', () { diff --git a/packages/flutter/test/widgets/editable_text_test.dart b/packages/flutter/test/widgets/editable_text_test.dart index 9fbbb9e219..83d6727f88 100644 --- a/packages/flutter/test/widgets/editable_text_test.dart +++ b/packages/flutter/test/widgets/editable_text_test.dart @@ -5204,19 +5204,19 @@ void main() { } if (shortcutModifier) { await tester.sendKeyDownEvent( - platform == 'macos' ? LogicalKeyboardKey.metaLeft : LogicalKeyboardKey.controlLeft, + platform == 'macos' || platform == 'ios' ? LogicalKeyboardKey.metaLeft : LogicalKeyboardKey.controlLeft, platform: platform, ); } if (wordModifier) { await tester.sendKeyDownEvent( - platform == 'macos' ? LogicalKeyboardKey.altLeft : LogicalKeyboardKey.controlLeft, + platform == 'macos' || platform == 'ios' ? LogicalKeyboardKey.altLeft : LogicalKeyboardKey.controlLeft, platform: platform, ); } if (lineModifier) { await tester.sendKeyDownEvent( - platform == 'macos' ? LogicalKeyboardKey.metaLeft : LogicalKeyboardKey.altLeft, + platform == 'macos' || platform == 'ios' ? LogicalKeyboardKey.metaLeft : LogicalKeyboardKey.altLeft, platform: platform, ); } @@ -5226,19 +5226,19 @@ void main() { } if (lineModifier) { await tester.sendKeyUpEvent( - platform == 'macos' ? LogicalKeyboardKey.metaLeft : LogicalKeyboardKey.altLeft, + platform == 'macos' || platform == 'ios' ? LogicalKeyboardKey.metaLeft : LogicalKeyboardKey.altLeft, platform: platform, ); } if (wordModifier) { await tester.sendKeyUpEvent( - platform == 'macos' ? LogicalKeyboardKey.altLeft : LogicalKeyboardKey.controlLeft, + platform == 'macos' || platform == 'ios' ? LogicalKeyboardKey.altLeft : LogicalKeyboardKey.controlLeft, platform: platform, ); } if (shortcutModifier) { await tester.sendKeyUpEvent( - platform == 'macos' ? LogicalKeyboardKey.metaLeft : LogicalKeyboardKey.controlLeft, + platform == 'macos' || platform == 'ios' ? LogicalKeyboardKey.metaLeft : LogicalKeyboardKey.controlLeft, platform: platform, ); } @@ -5547,7 +5547,6 @@ void main() { switch (defaultTargetPlatform) { // These platforms extend by line. - case TargetPlatform.iOS: case TargetPlatform.android: case TargetPlatform.fuchsia: case TargetPlatform.linux: @@ -5565,7 +5564,8 @@ void main() { ); break; - // Mac expands by line. + // Mac and iOS expand by line. + case TargetPlatform.iOS: case TargetPlatform.macOS: expect( selection, @@ -6738,7 +6738,6 @@ void main() { switch (defaultTargetPlatform) { // These platforms don't handle shift + home/end at all. case TargetPlatform.android: - case TargetPlatform.iOS: case TargetPlatform.fuchsia: expect( selectionAfterHome, @@ -6811,7 +6810,8 @@ void main() { ); break; - // Mac goes to the start/end of the document. + // Mac and iOS go to the start/end of the document. + case TargetPlatform.iOS: case TargetPlatform.macOS: expect( selectionAfterHome, @@ -7088,7 +7088,6 @@ void main() { switch (defaultTargetPlatform) { // These platforms don't move the selection with shift + home/end at all. case TargetPlatform.android: - case TargetPlatform.iOS: case TargetPlatform.fuchsia: expect( selection, @@ -7101,7 +7100,8 @@ void main() { ); break; - // Mac selects to the start of the document. + // Mac and iOS select to the start of the document. + case TargetPlatform.iOS: case TargetPlatform.macOS: expect( selection, @@ -7145,7 +7145,6 @@ void main() { switch (defaultTargetPlatform) { // These platforms don't move the selection with home/end at all still. case TargetPlatform.android: - case TargetPlatform.iOS: case TargetPlatform.fuchsia: expect( selection, @@ -7158,7 +7157,8 @@ void main() { ); break; - // Mac selects to the start of the document. + // Mac and iOS select to the start of the document. + case TargetPlatform.iOS: case TargetPlatform.macOS: expect( selection, @@ -7280,7 +7280,6 @@ void main() { switch (defaultTargetPlatform) { // These platforms don't move the selection with home/end at all. case TargetPlatform.android: - case TargetPlatform.iOS: case TargetPlatform.fuchsia: expect( selection, @@ -7293,7 +7292,8 @@ void main() { ); break; - // Mac selects to the end of the document. + // Mac and iOS select to the end of the document. + case TargetPlatform.iOS: case TargetPlatform.macOS: expect( selection, @@ -7337,7 +7337,6 @@ void main() { switch (defaultTargetPlatform) { // These platforms don't move the selection with home/end at all still. case TargetPlatform.android: - case TargetPlatform.iOS: case TargetPlatform.fuchsia: expect( selection, @@ -7350,7 +7349,8 @@ void main() { ); break; - // Mac stays at the end of the document. + // Mac and iOS stay at the end of the document. + case TargetPlatform.iOS: case TargetPlatform.macOS: expect( selection, @@ -10213,7 +10213,7 @@ void main() { wordModifier: true, targetPlatform: defaultTargetPlatform, ); - if (defaultTargetPlatform == TargetPlatform.macOS) { + if (defaultTargetPlatform == TargetPlatform.macOS || defaultTargetPlatform == TargetPlatform.iOS) { // word wo|rd word expect(controller.selection.isCollapsed, true); expect(controller.selection.baseOffset, 7); @@ -10264,7 +10264,7 @@ void main() { wordModifier: true, targetPlatform: defaultTargetPlatform, ); - if (defaultTargetPlatform == TargetPlatform.macOS) { + if (defaultTargetPlatform == TargetPlatform.macOS || defaultTargetPlatform == TargetPlatform.iOS) { // word wo|rd word expect(controller.selection.isCollapsed, true); expect(controller.selection.baseOffset, 7); @@ -10353,7 +10353,6 @@ void main() { expect(controller.selection.isCollapsed, false); switch (defaultTargetPlatform) { // These platforms extend by line. - case TargetPlatform.iOS: case TargetPlatform.android: case TargetPlatform.fuchsia: case TargetPlatform.linux: @@ -10362,7 +10361,8 @@ void main() { expect(controller.selection.extentOffset, 15); break; - // Mac expands by line. + // Mac and iOS expand by line. + case TargetPlatform.iOS: case TargetPlatform.macOS: expect(controller.selection.baseOffset, 15); expect(controller.selection.extentOffset, 24);