From 3c3e5fe3717fc333145a2b355cb3c7ce6bce5a3b Mon Sep 17 00:00:00 2001 From: Casey Hillers Date: Thu, 26 May 2022 09:01:07 -0700 Subject: [PATCH] Revert "Removing Shorcuts.of and Shortctus.maybeOf (#104215)" This reverts commit da24f105bd31ff2086c3f33105a0f2053bd14760. --- packages/flutter/lib/src/widgets/actions.dart | 4 - .../flutter/lib/src/widgets/shortcuts.dart | 121 +++++---- .../flutter/test/widgets/shortcuts_test.dart | 236 ++++++++---------- 3 files changed, 166 insertions(+), 195 deletions(-) diff --git a/packages/flutter/lib/src/widgets/actions.dart b/packages/flutter/lib/src/widgets/actions.dart index 3309ec9ba45a..98fef5086158 100644 --- a/packages/flutter/lib/src/widgets/actions.dart +++ b/packages/flutter/lib/src/widgets/actions.dart @@ -97,10 +97,6 @@ typedef ActionListenerCallback = void Function(Action action); /// developers to change that if they add an ancestor [Actions] widget that maps /// [SelectAllTextIntent] to a different [Action]. /// -/// See the article on [Using Actions and -/// Shortcuts](https://docs.flutter.dev/development/ui/advanced/actions_and_shortcuts) -/// for a detailed explanation. -/// /// See also: /// /// * [Shortcuts], which is a widget that contains a key map, in which it looks diff --git a/packages/flutter/lib/src/widgets/shortcuts.dart b/packages/flutter/lib/src/widgets/shortcuts.dart index a5d84f4a37dd..60394ca64f8c 100644 --- a/packages/flutter/lib/src/widgets/shortcuts.dart +++ b/packages/flutter/lib/src/widgets/shortcuts.dart @@ -637,15 +637,13 @@ class _ActivatorIntentPair with Diagnosticable { } } -/// A manager of keyboard shortcut bindings used by [Shortcuts] to handle key -/// events. +/// A manager of keyboard shortcut bindings. +/// +/// A `ShortcutManager` is obtained by calling [Shortcuts.of] on the context of +/// the widget that you want to find a manager for. /// /// The manager may be listened to (with [addListener]/[removeListener]) for /// change notifications when the shortcuts change. -/// -/// Typically, a [Shortcuts] widget supplies its own manager, but in uncommon -/// cases where overriding the usual shortcut manager behavior is desired, a -/// subclassed [ShortcutManager] may be supplied. class ShortcutManager with Diagnosticable, ChangeNotifier { /// Constructs a [ShortcutManager]. ShortcutManager({ @@ -776,10 +774,6 @@ class ShortcutManager with Diagnosticable, ChangeNotifier { /// when invoking an [Action] via a keyboard key combination that maps to an /// [Intent]. /// -/// See the article on [Using Actions and -/// Shortcuts](https://docs.flutter.dev/development/ui/advanced/actions_and_shortcuts) -/// for a detailed explanation. -/// /// {@tool dartpad} /// Here, we will use the [Shortcuts] and [Actions] widgets to add and subtract /// from a counter. When the child widget has keyboard focus, and a user presses @@ -817,61 +811,35 @@ class ShortcutManager with Diagnosticable, ChangeNotifier { /// * [Action], a class for defining an invocation of a user action. /// * [CallbackAction], a class for creating an action from a callback. class Shortcuts extends StatefulWidget { - /// Creates a const [Shortcuts] widget that owns the map of shortcuts and - /// creates its own manager. - /// - /// When using this constructor, [manager] will return null. + /// Creates a const [Shortcuts] widget. /// /// The [child] and [shortcuts] arguments are required. - /// - /// See also: - /// - /// * [Shortcuts.manager], a constructor that uses a [ShortcutManager] to - /// manage the shortcuts list instead. const Shortcuts({ super.key, - required Map shortcuts, - required this.child, - this.debugLabel, - }) : _shortcuts = shortcuts, - manager = null, - assert(shortcuts != null), - assert(child != null); - - /// Creates a const [Shortcuts] widget that uses the [manager] to - /// manage the map of shortcuts. - /// - /// If this constructor is used, [shortcuts] will return the contents of - /// [ShortcutManager.shortcuts]. - /// - /// The [child] and [manager] arguments are required. - const Shortcuts.manager({ - super.key, - required ShortcutManager this.manager, + this.manager, + required this.shortcuts, required this.child, this.debugLabel, - }) : _shortcuts = const {}, - assert(manager != null), + }) : assert(shortcuts != null), assert(child != null); /// The [ShortcutManager] that will manage the mapping between key /// combinations and [Action]s. /// - /// If this widget was created with [Shortcuts.manager], then - /// [ShortcutManager.shortcuts] will be used as the source for shortcuts. If - /// the unnamed constructor is used, this manager will be null, and a - /// default-constructed `ShortcutsManager` will be used. + /// If not specified, uses a default-constructed [ShortcutManager]. + /// + /// This manager will be given new [shortcuts] to manage whenever the + /// [shortcuts] change materially. final ShortcutManager? manager; /// {@template flutter.widgets.shortcuts.shortcuts} - /// The map of shortcuts that describes the mapping between a key sequence - /// defined by a [ShortcutActivator] and the [Intent] that will be emitted - /// when that key sequence is pressed. + /// The map of shortcuts that the [ShortcutManager] will be given to manage. + /// + /// For performance reasons, it is recommended that a pre-built map is passed + /// in here (e.g. a final variable from your widget class) instead of defining + /// it inline in the build function. /// {@endtemplate} - Map get shortcuts { - return manager == null ? _shortcuts : manager!.shortcuts; - } - final Map _shortcuts; + final Map shortcuts; /// The child widget for this [Shortcuts] widget. /// @@ -887,6 +855,52 @@ class Shortcuts extends StatefulWidget { /// unnecessarily with large default shortcut maps. final String? debugLabel; + /// Returns the [ShortcutManager] that most tightly encloses the given + /// [BuildContext]. + /// + /// If no [Shortcuts] widget encloses the context given, will assert in debug + /// mode and throw an exception in release mode. + /// + /// See also: + /// + /// * [maybeOf], which is similar to this function, but will return null if + /// it doesn't find a [Shortcuts] ancestor. + static ShortcutManager of(BuildContext context) { + assert(context != null); + final _ShortcutsMarker? inherited = context.dependOnInheritedWidgetOfExactType<_ShortcutsMarker>(); + assert(() { + if (inherited == null) { + throw FlutterError( + 'Unable to find a $Shortcuts widget in the context.\n' + '$Shortcuts.of() was called with a context that does not contain a ' + '$Shortcuts widget.\n' + 'No $Shortcuts ancestor could be found starting from the context that was ' + 'passed to $Shortcuts.of().\n' + 'The context used was:\n' + ' $context', + ); + } + return true; + }()); + return inherited!.manager; + } + + /// Returns the [ShortcutManager] that most tightly encloses the given + /// [BuildContext]. + /// + /// If no [Shortcuts] widget encloses the context given, will return null. + /// + /// See also: + /// + /// * [of], which is similar to this function, but returns a non-nullable + /// result, and will throw an exception if it doesn't find a [Shortcuts] + /// ancestor. + static ShortcutManager? maybeOf(BuildContext context) { + assert(context != null); + final _ShortcutsMarker? inherited = context.dependOnInheritedWidgetOfExactType<_ShortcutsMarker>(); + return inherited?.manager; + } + @override State createState() => _ShortcutsState(); @@ -913,8 +927,8 @@ class _ShortcutsState extends State { super.initState(); if (widget.manager == null) { _internalManager = ShortcutManager(); - _internalManager!.shortcuts = widget.shortcuts; } + manager.shortcuts = widget.shortcuts; } @override @@ -928,7 +942,7 @@ class _ShortcutsState extends State { _internalManager ??= ShortcutManager(); } } - _internalManager?.shortcuts = widget.shortcuts; + manager.shortcuts = widget.shortcuts; } KeyEventResult _handleOnKey(FocusNode node, RawKeyEvent event) { @@ -1318,7 +1332,8 @@ class _ShortcutRegistrarState extends State { @override Widget build(BuildContext context) { - return Shortcuts.manager( + return Shortcuts( + shortcuts: registry.shortcuts, manager: manager, child: _ShortcutRegistrarMarker( registry: registry, diff --git a/packages/flutter/test/widgets/shortcuts_test.dart b/packages/flutter/test/widgets/shortcuts_test.dart index 4d63ae48c692..8e9ba8a82e51 100644 --- a/packages/flutter/test/widgets/shortcuts_test.dart +++ b/packages/flutter/test/widgets/shortcuts_test.dart @@ -532,81 +532,69 @@ void main() { group(Shortcuts, () { testWidgets('Default constructed Shortcuts has empty shortcuts', (WidgetTester tester) async { - const Shortcuts shortcuts = Shortcuts(shortcuts: {}, child: SizedBox()); - await tester.pumpWidget(shortcuts); - expect(shortcuts.shortcuts, isNotNull); - expect(shortcuts.shortcuts, isEmpty); - }); - testWidgets('Default constructed Shortcuts.manager has empty shortcuts', (WidgetTester tester) async { final ShortcutManager manager = ShortcutManager(); expect(manager.shortcuts, isNotNull); expect(manager.shortcuts, isEmpty); - final Shortcuts shortcuts = Shortcuts.manager(manager: manager, child: const SizedBox()); + const Shortcuts shortcuts = Shortcuts(shortcuts: {}, child: SizedBox()); await tester.pumpWidget(shortcuts); expect(shortcuts.shortcuts, isNotNull); expect(shortcuts.shortcuts, isEmpty); }); - testWidgets('Shortcuts.manager passes on shortcuts', (WidgetTester tester) async { - final Map testShortcuts = { - LogicalKeySet(LogicalKeyboardKey.shift): const TestIntent(), - }; - final ShortcutManager manager = ShortcutManager(shortcuts: testShortcuts); - expect(manager.shortcuts, isNotNull); - expect(manager.shortcuts, equals(testShortcuts)); - final Shortcuts shortcuts = Shortcuts.manager(manager: manager, child: const SizedBox()); - await tester.pumpWidget(shortcuts); - expect(shortcuts.shortcuts, isNotNull); - expect(shortcuts.shortcuts, equals(testShortcuts)); - }); - testWidgets('ShortcutManager handles shortcuts', (WidgetTester tester) async { + testWidgets('Shortcuts.of and maybeOf find shortcuts', (WidgetTester tester) async { final GlobalKey containerKey = GlobalKey(); final List pressedKeys = []; - final TestShortcutManager testManager = TestShortcutManager( - pressedKeys, - shortcuts: { - LogicalKeySet(LogicalKeyboardKey.shift): const TestIntent(), - }, - ); - bool invoked = false; + final TestShortcutManager testManager = TestShortcutManager(pressedKeys); await tester.pumpWidget( - Actions( - actions: >{ - TestIntent: TestAction( - onInvoke: (Intent intent) { - invoked = true; - return true; - }, - ), + Shortcuts( + manager: testManager, + shortcuts: { + LogicalKeySet(LogicalKeyboardKey.shift): const TestIntent(), }, - child: Shortcuts.manager( - manager: testManager, - child: Focus( - autofocus: true, - child: SizedBox(key: containerKey, width: 100, height: 100), - ), + child: Focus( + autofocus: true, + child: SizedBox(key: containerKey, width: 100, height: 100), ), ), ); await tester.pump(); - await tester.sendKeyDownEvent(LogicalKeyboardKey.shiftLeft); - expect(invoked, isTrue); - expect(pressedKeys, equals([LogicalKeyboardKey.shiftLeft])); + expect(Shortcuts.maybeOf(containerKey.currentContext!), isNotNull); + expect(Shortcuts.maybeOf(containerKey.currentContext!), equals(testManager)); + expect(Shortcuts.of(containerKey.currentContext!), equals(testManager)); }); - testWidgets('Shortcuts.manager lets manager handle shortcuts', (WidgetTester tester) async { + testWidgets('Shortcuts.of and maybeOf work correctly without shortcuts', (WidgetTester tester) async { final GlobalKey containerKey = GlobalKey(); - final List pressedKeys = []; - bool shortcutsSet = false; - void onShortcutsSet() { - shortcutsSet = true; + await tester.pumpWidget(Container(key: containerKey)); + expect(Shortcuts.maybeOf(containerKey.currentContext!), isNull); + late FlutterError error; + try { + Shortcuts.of(containerKey.currentContext!); + } on FlutterError catch (e) { + error = e; + } finally { + expect(error, isNotNull); + expect(error.diagnostics.length, 5); + expect(error.diagnostics[2].level, DiagnosticLevel.info); + expect( + error.diagnostics[2].toStringDeep(), + 'No Shortcuts ancestor could be found starting from the context\n' + 'that was passed to Shortcuts.of().\n', + ); + expect(error.toStringDeep(), equalsIgnoringHashCodes( + 'FlutterError\n' + ' Unable to find a Shortcuts widget in the context.\n' + ' Shortcuts.of() was called with a context that does not contain a\n' + ' Shortcuts widget.\n' + ' No Shortcuts ancestor could be found starting from the context\n' + ' that was passed to Shortcuts.of().\n' + ' The context used was:\n' + ' Container-[GlobalKey#00000]\n', + )); } - final TestShortcutManager testManager = TestShortcutManager( - pressedKeys, - onShortcutsSet: onShortcutsSet, - shortcuts: { - LogicalKeySet(LogicalKeyboardKey.shift): const TestIntent(), - }, - ); - shortcutsSet = false; + }); + testWidgets('ShortcutManager handles shortcuts', (WidgetTester tester) async { + final GlobalKey containerKey = GlobalKey(); + final List pressedKeys = []; + final TestShortcutManager testManager = TestShortcutManager(pressedKeys); bool invoked = false; await tester.pumpWidget( Actions( @@ -618,8 +606,11 @@ void main() { }, ), }, - child: Shortcuts.manager( + child: Shortcuts( manager: testManager, + shortcuts: { + LogicalKeySet(LogicalKeyboardKey.shift): const TestIntent(), + }, child: Focus( autofocus: true, child: SizedBox(key: containerKey, width: 100, height: 100), @@ -628,20 +619,15 @@ void main() { ), ); await tester.pump(); + expect(Shortcuts.of(containerKey.currentContext!), isNotNull); await tester.sendKeyDownEvent(LogicalKeyboardKey.shiftLeft); expect(invoked, isTrue); - expect(shortcutsSet, isFalse); expect(pressedKeys, equals([LogicalKeyboardKey.shiftLeft])); }); - testWidgets('ShortcutManager ignores key presses with no primary focus', (WidgetTester tester) async { + testWidgets('ShortcutManager ignores keypresses with no primary focus', (WidgetTester tester) async { final GlobalKey containerKey = GlobalKey(); final List pressedKeys = []; - final TestShortcutManager testManager = TestShortcutManager( - pressedKeys, - shortcuts: { - LogicalKeySet(LogicalKeyboardKey.shift): const TestIntent(), - }, - ); + final TestShortcutManager testManager = TestShortcutManager(pressedKeys); bool invoked = false; await tester.pumpWidget( Actions( @@ -653,14 +639,18 @@ void main() { }, ), }, - child: Shortcuts.manager( + child: Shortcuts( manager: testManager, + shortcuts: { + LogicalKeySet(LogicalKeyboardKey.shift): const TestIntent(), + }, child: SizedBox(key: containerKey, width: 100, height: 100), ), ), ); await tester.pump(); expect(primaryFocus, isNull); + expect(Shortcuts.of(containerKey.currentContext!), isNotNull); await tester.sendKeyDownEvent(LogicalKeyboardKey.shiftLeft); expect(invoked, isFalse); expect(pressedKeys, isEmpty); @@ -668,16 +658,14 @@ void main() { testWidgets("Shortcuts passes to the next Shortcuts widget if it doesn't map the key", (WidgetTester tester) async { final GlobalKey containerKey = GlobalKey(); final List pressedKeys = []; - final TestShortcutManager testManager = TestShortcutManager( - pressedKeys, - shortcuts: { - LogicalKeySet(LogicalKeyboardKey.shift): const TestIntent(), - }, - ); + final TestShortcutManager testManager = TestShortcutManager(pressedKeys); bool invoked = false; await tester.pumpWidget( - Shortcuts.manager( + Shortcuts( manager: testManager, + shortcuts: { + LogicalKeySet(LogicalKeyboardKey.shift): const TestIntent(), + }, child: Actions( actions: >{ TestIntent: TestAction( @@ -700,6 +688,7 @@ void main() { ), ); await tester.pump(); + expect(Shortcuts.of(containerKey.currentContext!), isNotNull); await tester.sendKeyDownEvent(LogicalKeyboardKey.shiftLeft); expect(invoked, isTrue); expect(pressedKeys, equals([LogicalKeyboardKey.shiftLeft])); @@ -707,17 +696,15 @@ void main() { testWidgets('Shortcuts can disable a shortcut with Intent.doNothing', (WidgetTester tester) async { final GlobalKey containerKey = GlobalKey(); final List pressedKeys = []; - final TestShortcutManager testManager = TestShortcutManager( - pressedKeys, - shortcuts: { - LogicalKeySet(LogicalKeyboardKey.shift): const TestIntent(), - }, - ); + final TestShortcutManager testManager = TestShortcutManager(pressedKeys); bool invoked = false; await tester.pumpWidget( MaterialApp( - home: Shortcuts.manager( + home: Shortcuts( manager: testManager, + shortcuts: { + LogicalKeySet(LogicalKeyboardKey.shift): const TestIntent(), + }, child: Actions( actions: >{ TestIntent: TestAction( @@ -741,6 +728,7 @@ void main() { ), ); await tester.pump(); + expect(Shortcuts.of(containerKey.currentContext!), isNotNull); await tester.sendKeyDownEvent(LogicalKeyboardKey.shiftLeft); expect(invoked, isFalse); expect(pressedKeys, isEmpty); @@ -748,23 +736,22 @@ void main() { testWidgets("Shortcuts that aren't bound to an action don't absorb keys meant for text fields", (WidgetTester tester) async { final GlobalKey textFieldKey = GlobalKey(); final List pressedKeys = []; - final TestShortcutManager testManager = TestShortcutManager( - pressedKeys, - shortcuts: { - LogicalKeySet(LogicalKeyboardKey.keyA): const TestIntent(), - }, - ); + final TestShortcutManager testManager = TestShortcutManager(pressedKeys); await tester.pumpWidget( MaterialApp( home: Material( - child: Shortcuts.manager( + child: Shortcuts( manager: testManager, + shortcuts: { + LogicalKeySet(LogicalKeyboardKey.keyA): const TestIntent(), + }, child: TextField(key: textFieldKey, autofocus: true), ), ), ), ); await tester.pump(); + expect(Shortcuts.of(textFieldKey.currentContext!), isNotNull); final bool handled = await tester.sendKeyEvent(LogicalKeyboardKey.keyA); expect(handled, isFalse); expect(pressedKeys, equals([LogicalKeyboardKey.keyA])); @@ -772,18 +759,16 @@ void main() { testWidgets('Shortcuts that are bound to an action do override text fields', (WidgetTester tester) async { final GlobalKey textFieldKey = GlobalKey(); final List pressedKeys = []; - final TestShortcutManager testManager = TestShortcutManager( - pressedKeys, - shortcuts: { - LogicalKeySet(LogicalKeyboardKey.keyA): const TestIntent(), - }, - ); + final TestShortcutManager testManager = TestShortcutManager(pressedKeys); bool invoked = false; await tester.pumpWidget( MaterialApp( home: Material( - child: Shortcuts.manager( + child: Shortcuts( manager: testManager, + shortcuts: { + LogicalKeySet(LogicalKeyboardKey.keyA): const TestIntent(), + }, child: Actions( actions: >{ TestIntent: TestAction( @@ -800,6 +785,7 @@ void main() { ), ); await tester.pump(); + expect(Shortcuts.of(textFieldKey.currentContext!), isNotNull); final bool result = await tester.sendKeyEvent(LogicalKeyboardKey.keyA); expect(result, isTrue); expect(pressedKeys, equals([LogicalKeyboardKey.keyA])); @@ -808,18 +794,16 @@ void main() { testWidgets('Shortcuts can override intents that apply to text fields', (WidgetTester tester) async { final GlobalKey textFieldKey = GlobalKey(); final List pressedKeys = []; - final TestShortcutManager testManager = TestShortcutManager( - pressedKeys, - shortcuts: { - LogicalKeySet(LogicalKeyboardKey.keyA): const TestIntent(), - }, - ); + final TestShortcutManager testManager = TestShortcutManager(pressedKeys); bool invoked = false; await tester.pumpWidget( MaterialApp( home: Material( - child: Shortcuts.manager( + child: Shortcuts( manager: testManager, + shortcuts: { + LogicalKeySet(LogicalKeyboardKey.keyA): const TestIntent(), + }, child: Actions( actions: >{ TestIntent: TestAction( @@ -841,6 +825,7 @@ void main() { ), ); await tester.pump(); + expect(Shortcuts.of(textFieldKey.currentContext!), isNotNull); final bool result = await tester.sendKeyEvent(LogicalKeyboardKey.keyA); expect(result, isFalse); expect(invoked, isFalse); @@ -848,18 +833,16 @@ void main() { testWidgets('Shortcuts can override intents that apply to text fields with DoNothingAndStopPropagationIntent', (WidgetTester tester) async { final GlobalKey textFieldKey = GlobalKey(); final List pressedKeys = []; - final TestShortcutManager testManager = TestShortcutManager( - pressedKeys, - shortcuts: { - LogicalKeySet(LogicalKeyboardKey.keyA): const TestIntent(), - }, - ); + final TestShortcutManager testManager = TestShortcutManager(pressedKeys); bool invoked = false; await tester.pumpWidget( MaterialApp( home: Material( - child: Shortcuts.manager( + child: Shortcuts( manager: testManager, + shortcuts: { + LogicalKeySet(LogicalKeyboardKey.keyA): const TestIntent(), + }, child: Actions( actions: >{ TestIntent: TestAction( @@ -881,6 +864,7 @@ void main() { ), ); await tester.pump(); + expect(Shortcuts.of(textFieldKey.currentContext!), isNotNull); final bool result = await tester.sendKeyEvent(LogicalKeyboardKey.keyA); expect(result, isFalse); expect(invoked, isFalse); @@ -935,10 +919,11 @@ void main() { expect(description.length, equals(1)); expect(description[0], equals('shortcuts: ')); }); - test('Shortcuts diagnostics work when manager not specified.', () { + test('Shortcuts diagnostics work when manager specified.', () { final DiagnosticPropertiesBuilder builder = DiagnosticPropertiesBuilder(); Shortcuts( + manager: ShortcutManager(), shortcuts: { LogicalKeySet( LogicalKeyboardKey.keyA, @@ -952,35 +937,11 @@ void main() { return !node.isFiltered(DiagnosticLevel.info); }).map((DiagnosticsNode node) => node.toString()).toList(); - expect(description.length, equals(1)); - expect(description[0], equalsIgnoringHashCodes('shortcuts: {{Key A + Key B}: ActivateIntent#00000}')); - }); - test('Shortcuts diagnostics work when manager specified.', () { - final DiagnosticPropertiesBuilder builder = DiagnosticPropertiesBuilder(); - final List pressedKeys = []; - final TestShortcutManager testManager = TestShortcutManager( - pressedKeys, - shortcuts: { - LogicalKeySet( - LogicalKeyboardKey.keyA, - LogicalKeyboardKey.keyB, - ): const ActivateIntent(), - }, - ); - - Shortcuts.manager( - manager: testManager, - child: const SizedBox(), - ).debugFillProperties(builder); - - final List description = builder.properties.where((DiagnosticsNode node) { - return !node.isFiltered(DiagnosticLevel.info); - }).map((DiagnosticsNode node) => node.toString()).toList(); - expect(description.length, equals(2)); - expect(description[0], equalsIgnoringHashCodes('manager: TestShortcutManager#00000(shortcuts: {LogicalKeySet#00000(keys: Key A + Key B): ActivateIntent#00000})')); + expect(description[0], equalsIgnoringHashCodes('manager: ShortcutManager#00000(shortcuts: {})')); expect(description[1], equalsIgnoringHashCodes('shortcuts: {{Key A + Key B}: ActivateIntent#00000}')); }); + testWidgets('Shortcuts support multiple intents', (WidgetTester tester) async { tester.binding.focusManager.highlightStrategy = FocusHighlightStrategy.alwaysTraditional; bool? value = true; @@ -1826,10 +1787,9 @@ class TestIntent2 extends Intent { } class TestShortcutManager extends ShortcutManager { - TestShortcutManager(this.keys, { super.shortcuts, this.onShortcutsSet }); + TestShortcutManager(this.keys); List keys; - VoidCallback? onShortcutsSet; @override KeyEventResult handleKeypress(BuildContext context, RawKeyEvent event) {