Change the way ActionDispatcher is found. (#41245)

This changes the way ActionDispatchers are found by the Actions widget, so that by default it will look for dispatchers of the parent Actions widgets instead of just creating a default ActionDispatcher. This allows overriding of the ActionDispatcher at the top level: before, the custom action dispatcher would only be invoked if explicitly set on all the Actions widgets.

This is not a breaking change because there was a default value to the dispatcher parameter before that performed this function, and not specifying the dispatcher anywhere will still result in a default dispatcher being created.

packages/flutter/lib/src/widgets/actions.dart

@@ -27,6 +27,14 @@ class Intent extends Diagnosticable {
   /// The [key] argument must not be null.
   const Intent(this.key) : assert(key != null);
 
+  /// An intent that can't be mapped to an action.
+  ///
+  /// This Intent is prevented from being mapped to an action in the
+  /// [ActionDispatcher], and as such can be used to indicate that a shortcut
+  /// should not do anything, allowing a shortcut defined at a higher level to
+  /// be disabled at a deeper level in the widget hierarchy.
+  static const Intent doNothing = Intent(ValueKey<Type>(Intent));
+
   /// The key for the action this intent is associated with.
   final LocalKey key;
 
@@ -141,6 +149,8 @@ class ActionDispatcher extends Diagnosticable {
   /// [FocusManager.primaryFocus] if the given `focusNode` is null.
   ///
   /// The `action` and `intent` arguments must not be null.
+  ///
+  /// Returns true if the action was successfully invoked.
   bool invokeAction(Action action, Intent intent, {FocusNode focusNode}) {
     assert(action != null);
     assert(intent != null);
@@ -173,22 +183,52 @@ class Actions extends InheritedWidget {
   /// The [child], [actions], and [dispatcher] arguments must not be null.
   const Actions({
     Key key,
-    this.dispatcher = const ActionDispatcher(),
+    this.dispatcher,
     @required this.actions,
     @required Widget child,
-  })  : assert(dispatcher != null),
-        assert(actions != null),
+  })  : assert(actions != null),
         super(key: key, child: child);
 
   /// The [ActionDispatcher] object that invokes actions.
   ///
   /// This is what is returned from [Actions.of], and used by [Actions.invoke].
+  ///
+  /// If this [dispatcher] is null, then [Actions.of] and [Actions.invoke] will
+  /// look up the tree until they find an Actions widget that has a dispatcher
+  /// set. If not such widget is found, then they will return/use a
+  /// default-constructed [ActionDispatcher].
   final ActionDispatcher dispatcher;
 
   /// A map of [Intent] keys to [ActionFactory] factory methods that defines
   /// which actions this widget knows about.
   final Map<LocalKey, ActionFactory> actions;
 
+  // Finds the nearest valid ActionDispatcher, or creates a new one if it
+  // doesn't find one.
+  static ActionDispatcher _findDispatcher(Element element) {
+    assert(element.widget is Actions);
+    final Actions actions = element.widget;
+    ActionDispatcher dispatcher = actions.dispatcher;
+    if (dispatcher == null) {
+      bool visitAncestorElement(Element visitedElement) {
+        if (visitedElement.widget is! Actions) {
+          // Continue visiting.
+          return true;
+        }
+        final Actions actions = visitedElement.widget;
+        if (actions.dispatcher == null) {
+          // Continue visiting.
+          return true;
+        }
+        dispatcher = actions.dispatcher;
+        // Stop visiting.
+        return false;
+      }
+      element.visitAncestorElements(visitAncestorElement);
+    }
+    return dispatcher ?? const ActionDispatcher();
+  }
+
   /// Returns the [ActionDispatcher] associated with the [Actions] widget that
   /// most tightly encloses the given [BuildContext].
   ///
@@ -200,7 +240,8 @@ class Actions extends InheritedWidget {
   /// The `context` argument must not be null.
   static ActionDispatcher of(BuildContext context, {bool nullOk = false}) {
     assert(context != null);
-    final Actions inherited = context.inheritFromWidgetOfExactType(Actions);
+    final InheritedElement inheritedElement = context.ancestorInheritedElementForWidgetOfExactType(Actions);
+    final Actions inherited = context.inheritFromElement(inheritedElement);
     assert(() {
       if (nullOk) {
         return true;
@@ -217,7 +258,7 @@ class Actions extends InheritedWidget {
       }
       return true;
     }());
-    return inherited?.dispatcher;
+    return inherited?.dispatcher ?? _findDispatcher(inheritedElement);
   }
 
   /// Invokes the action associated with the given [Intent] using the
@@ -233,8 +274,13 @@ class Actions extends InheritedWidget {
   /// `intent` doesn't map to an action in any of the [Actions.actions] maps
   /// that are found.
   ///
+  /// Returns true if an action was successfully invoked.
+  ///
   /// Setting `nullOk` to true means that if no ambient [Actions] widget is
   /// found, then this method will return false instead of throwing.
+  ///
+  /// If the `intent` argument is [Intent.doNothing], then this function will
+  /// return false, without looking for a matching action.
   static bool invoke(
     BuildContext context,
     Intent intent, {
@@ -243,8 +289,13 @@ class Actions extends InheritedWidget {
   }) {
     assert(context != null);
     assert(intent != null);
-    Actions actions;
+    Element actionsElement;
     Action action;
+
+    if (identical(intent, Intent.doNothing)) {
+      return false;
+    }
+
     bool visitAncestorElement(Element element) {
       if (element.widget is! Actions) {
         // Continue visiting.
@@ -252,9 +303,10 @@ class Actions extends InheritedWidget {
       }
       // Below when we invoke the action, we need to use the dispatcher from the
       // Actions widget where we found the action, in case they need to match.
-      actions = element.widget;
+      actionsElement = element;
+      final Actions actions = element.widget;
       action = actions.actions[intent.key]?.call();
-      // Don't continue visiting if we successfully created an action.
+      // Keep looking if we failed to find and create an action.
       return action == null;
     }
 
@@ -263,7 +315,7 @@ class Actions extends InheritedWidget {
       if (nullOk) {
         return true;
       }
-      if (actions == null) {
+      if (actionsElement == null) {
         throw FlutterError('Unable to find a $Actions widget in the context.\n'
             '$Actions.invoke() was called with a context that does not contain an '
             '$Actions widget.\n'
@@ -288,10 +340,10 @@ class Actions extends InheritedWidget {
       // Will only get here if nullOk is true.
       return false;
     }
-    // Invoke the action we found using the dispatcher from the Actions we
-    // found, using the given focus node. Or null, if nullOk is true, and we
-    // didn't find something.
-    return actions?.dispatcher?.invokeAction(action, intent, focusNode: focusNode);
+
+    // Invoke the action we found using the dispatcher from the Actions Element
+    // we found, using the given focus node.
+    return _findDispatcher(actionsElement).invokeAction(action, intent, focusNode: focusNode);
   }
 
   @override

packages/flutter/lib/src/widgets/focus_manager.dart

@@ -1251,6 +1251,7 @@ class FocusManager with DiagnosticableTreeMixin {
     // Walk the current focus from the leaf to the root, calling each one's
     // onKey on the way up, and if one responds that they handled it, stop.
     if (_primaryFocus == null) {
+      assert(_focusDebug('No primary focus for key event, ignored: $event'));
       return;
     }
     Iterable<FocusNode> allNodes(FocusNode node) sync* {
@@ -1260,11 +1261,17 @@ class FocusManager with DiagnosticableTreeMixin {
       }
     }
 
+    bool handled = false;
     for (FocusNode node in allNodes(_primaryFocus)) {
       if (node.onKey != null && node.onKey(node, event)) {
+        assert(_focusDebug('Node $node handled key event $event.'));
+        handled = true;
         break;
       }
     }
+    if (!handled) {
+      assert(_focusDebug('Key event not handled by anyone: $event.'));
+    }
   }
 
   /// The node that currently has the primary focus.

packages/flutter/lib/src/widgets/shortcuts.dart

@@ -345,6 +345,7 @@ class _ShortcutsState extends State<Shortcuts> {
   @override
   Widget build(BuildContext context) {
     return Focus(
+      debugLabel: describeIdentity(widget),
       canRequestFocus: false,
       onKey: _handleOnKey,
       child: _ShortcutsMarker(

packages/flutter/test/widgets/actions_test.dart

@@ -159,7 +159,7 @@ void main() {
       expect(invoked, isTrue);
       expect(invokedIntent, equals(intent));
     });
-    testWidgets('$Actions widget can invoke actions in ancestor dispatcher', (WidgetTester tester) async {
+    testWidgets('$Actions can invoke actions in ancestor dispatcher', (WidgetTester tester) async {
       final GlobalKey containerKey = GlobalKey();
       bool invoked = false;
       const Intent intent = Intent(TestAction.key);
@@ -200,6 +200,46 @@ void main() {
       expect(invokedAction, equals(testAction));
       expect(invokedDispatcher.runtimeType, equals(TestDispatcher1));
     });
+    testWidgets("$Actions can invoke actions in ancestor dispatcher if a lower one isn't specified", (WidgetTester tester) async {
+      final GlobalKey containerKey = GlobalKey();
+      bool invoked = false;
+      const Intent intent = Intent(TestAction.key);
+      FocusNode passedNode;
+      final FocusNode testNode = FocusNode(debugLabel: 'Test Node');
+      final Action testAction = TestAction(
+        onInvoke: (FocusNode node, Intent invocation) {
+          invoked = true;
+          passedNode = node;
+        },
+      );
+
+      await tester.pumpWidget(
+        Actions(
+          dispatcher: TestDispatcher1(postInvoke: collect),
+          actions: <LocalKey, ActionFactory>{
+            TestAction.key: () => testAction,
+          },
+          child: Actions(
+            actions: const <LocalKey, ActionFactory>{},
+            child: Container(key: containerKey),
+          ),
+        ),
+      );
+
+      await tester.pump();
+      final bool result = Actions.invoke(
+        containerKey.currentContext,
+        intent,
+        focusNode: testNode,
+      );
+      expect(passedNode, equals(testNode));
+      expect(invokedNode, equals(testNode));
+      expect(result, isTrue);
+      expect(invoked, isTrue);
+      expect(invokedIntent, equals(intent));
+      expect(invokedAction, equals(testAction));
+      expect(invokedDispatcher.runtimeType, equals(TestDispatcher1));
+    });
     testWidgets('$Actions widget can be found with of', (WidgetTester tester) async {
       final GlobalKey containerKey = GlobalKey();
       final ActionDispatcher testDispatcher = TestDispatcher1(postInvoke: collect);
@@ -214,7 +254,8 @@ void main() {
 
       await tester.pump();
       final ActionDispatcher dispatcher = Actions.of(
-        containerKey.currentContext, nullOk: true,
+        containerKey.currentContext,
+        nullOk: true,
       );
       expect(dispatcher, equals(testDispatcher));
     });
@@ -254,14 +295,18 @@ void main() {
     testWidgets('default $Actions debugFillProperties', (WidgetTester tester) async {
       final DiagnosticPropertiesBuilder builder = DiagnosticPropertiesBuilder();
 
-      Actions(actions: const <LocalKey, ActionFactory>{}, child: Container()).debugFillProperties(builder);
+      Actions(
+        actions: const <LocalKey, ActionFactory>{},
+        dispatcher: const ActionDispatcher(),
+        child: Container(),
+      ).debugFillProperties(builder);
 
       final List<String> description = builder.properties
-        .where((DiagnosticsNode node) {
-          return !node.isFiltered(DiagnosticLevel.info);
-        })
-        .map((DiagnosticsNode node) => node.toString())
-        .toList();
+          .where((DiagnosticsNode node) {
+            return !node.isFiltered(DiagnosticLevel.info);
+          })
+          .map((DiagnosticsNode node) => node.toString())
+          .toList();
 
       expect(description[0], equalsIgnoringHashCodes('dispatcher: ActionDispatcher#00000'));
       expect(description[1], equals('actions: {}'));
@@ -271,6 +316,7 @@ void main() {
 
       Actions(
         key: const ValueKey<String>('foo'),
+        dispatcher: const ActionDispatcher(),
         actions: <LocalKey, ActionFactory>{
           const ValueKey<String>('bar'): () => TestAction(onInvoke: (FocusNode node, Intent intent) {}),
         },
@@ -278,11 +324,11 @@ void main() {
       ).debugFillProperties(builder);
 
       final List<String> description = builder.properties
-        .where((DiagnosticsNode node) {
-          return !node.isFiltered(DiagnosticLevel.info);
-        })
-        .map((DiagnosticsNode node) => node.toString())
-        .toList();
+          .where((DiagnosticsNode node) {
+            return !node.isFiltered(DiagnosticLevel.info);
+          })
+          .map((DiagnosticsNode node) => node.toString())
+          .toList();
 
       expect(description[0], equalsIgnoringHashCodes('dispatcher: ActionDispatcher#00000'));
       expect(description[1], equals('actions: {[<\'bar\'>]: Closure: () => TestAction}'));