Unverified Commit 4f586fa3 authored by Greg Spencer's avatar Greg Spencer Committed by GitHub

Remove nullOk parameter from Shortcuts.of, Actions.find, and Actions.handler (#68921)

This removes the nullOk parameter from Shortcuts.of, Actions.find and Actions.handler and created Shortcuts.maybeOf and Actions.maybeFind. Shortcuts.of and Actions.find now return non-nullable values, and the maybe versions return a nullable value. I didn't create a non-nullable version of Actions.handler, since it needs to be able to return null if an action is not enabled, but I did remove the nullOk parameter, effectively setting it to true permanently, since setting it to false doesn't make much sense if the function can still return null when the action is not enabled.
parent 70d253b9
......@@ -502,7 +502,7 @@ class Actions extends StatefulWidget {
/// Returns a [VoidCallback] handler that invokes the bound action for the
/// given `intent` if the action is enabled, and returns null if the action is
/// not enabled.
/// not enabled, or no matching action is found.
///
/// This is intended to be used in widgets which have something similar to an
/// `onTap` handler, which takes a `VoidCallback`, and can be set to the
......@@ -511,8 +511,8 @@ class Actions extends StatefulWidget {
/// Creates a dependency on the [Actions] widget that maps the bound action so
/// that if the actions change, the context will be rebuilt and find the
/// updated action.
static VoidCallback? handler<T extends Intent>(BuildContext context, T intent, {bool nullOk = false}) {
final Action<T>? action = Actions.find<T>(context, nullOk: nullOk);
static VoidCallback? handler<T extends Intent>(BuildContext context, T intent) {
final Action<T>? action = Actions.maybeFind<T>(context);
if (action != null && action.isEnabled(intent)) {
return () {
// Could be that the action was enabled when the closure was created,
......@@ -534,7 +534,52 @@ class Actions extends StatefulWidget {
/// The optional `intent` argument supplies the type of the intent to look for
/// if the concrete type of the intent sought isn't available. If not
/// supplied, then `T` is used.
static Action<T>? find<T extends Intent>(BuildContext context, {bool nullOk = false, T? intent}) {
///
/// If no [Actions] widget surrounds the given context, this function will
/// assert in debug mode, and throw an exception in release mode.
///
/// See also:
///
/// * [maybeFind], which is similar to this function, but will return null if
/// no [Actions] ancestor is found.
static Action<T> find<T extends Intent>(BuildContext context, { T? intent }) {
final Action<T>? action = maybeFind(context, intent: intent);
assert(() {
if (action == null) {
final Type type = intent?.runtimeType ?? T;
throw FlutterError('Unable to find an action for a $type in an $Actions widget '
'in the given context.\n'
"$Actions.find() was called on a context that doesn't contain an "
'$Actions widget with a mapping for the given intent type.\n'
'The context used was:\n'
' $context\n'
'The intent type requested was:\n'
' $type');
}
return true;
}());
return action!;
}
/// Finds the [Action] bound to the given intent type `T` in the given `context`.
///
/// Creates a dependency on the [Actions] widget that maps the bound action so
/// that if the actions change, the context will be rebuilt and find the
/// updated action.
///
/// The optional `intent` argument supplies the type of the intent to look for
/// if the concrete type of the intent sought isn't available. If not
/// supplied, then `T` is used.
///
/// If no [Actions] widget surrounds the given context, this function will
/// return null.
///
/// See also:
///
/// * [find], which is similar to this function, but will throw if
/// no [Actions] ancestor is found.
static Action<T>? maybeFind<T extends Intent>(BuildContext context, { T? intent }) {
Action<T>? action;
// Specialize the type if a runtime example instance of the intent is given.
......@@ -558,22 +603,6 @@ class Actions extends StatefulWidget {
return false;
});
assert(() {
if (nullOk) {
return true;
}
if (action == null) {
throw FlutterError('Unable to find an action for a $type in an $Actions widget '
'in the given context.\n'
"$Actions.find() was called on a context that doesn't contain an "
'$Actions widget with a mapping for the given intent type.\n'
'The context used was:\n'
' $context\n'
'The intent type requested was:\n'
' $type');
}
return true;
}());
return action;
}
......
......@@ -360,10 +360,9 @@ class ShortcutManager extends ChangeNotifier with Diagnosticable {
if (matchedIntent != null) {
final BuildContext primaryContext = primaryFocus!.context!;
assert (primaryContext != null);
final Action<Intent>? action = Actions.find<Intent>(
final Action<Intent>? action = Actions.maybeFind<Intent>(
primaryContext,
intent: matchedIntent,
nullOk: true,
);
if (action != null && action.isEnabled(matchedIntent)) {
Actions.of(primaryContext).invokeAction(action, matchedIntent, primaryContext);
......@@ -442,13 +441,18 @@ class Shortcuts extends StatefulWidget {
/// [BuildContext].
///
/// The [context] argument must not be null.
static ShortcutManager? of(BuildContext context, {bool nullOk = false}) {
///
/// 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 (nullOk) {
return true;
}
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 '
......@@ -460,6 +464,24 @@ class Shortcuts extends StatefulWidget {
}
return true;
}());
return inherited!.manager;
}
/// Returns the [ActionDispatcher] that most tightly encloses the given
/// [BuildContext].
///
/// The [context] argument must not be null.
///
/// 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;
}
......
......@@ -293,7 +293,7 @@ void main() {
await tester.pump();
expect(Actions.find<TestIntent>(containerKey.currentContext!), equals(testAction));
expect(() => Actions.find<DoNothingIntent>(containerKey.currentContext!), throwsAssertionError);
expect(Actions.find<DoNothingIntent>(containerKey.currentContext!, nullOk: true), isNull);
expect(Actions.maybeFind<DoNothingIntent>(containerKey.currentContext!), isNull);
await tester.pumpWidget(
Actions(
......@@ -313,7 +313,7 @@ void main() {
await tester.pump();
expect(Actions.find<TestIntent>(containerKey.currentContext!), equals(testAction));
expect(() => Actions.find<DoNothingIntent>(containerKey.currentContext!), throwsAssertionError);
expect(Actions.find<DoNothingIntent>(containerKey.currentContext!, nullOk: true), isNull);
expect(Actions.maybeFind<DoNothingIntent>(containerKey.currentContext!), isNull);
});
testWidgets('FocusableActionDetector keeps track of focus and hover even when disabled.', (WidgetTester tester) async {
FocusManager.instance.highlightStrategy = FocusHighlightStrategy.alwaysTraditional;
......
......@@ -211,6 +211,57 @@ void main() {
expect(shortcuts.shortcuts, isNotNull);
expect(shortcuts.shortcuts, isEmpty);
});
testWidgets('Shortcuts.of and maybeOf find shortcuts', (WidgetTester tester) async {
final GlobalKey containerKey = GlobalKey();
final List<LogicalKeyboardKey> pressedKeys = <LogicalKeyboardKey>[];
final TestShortcutManager testManager = TestShortcutManager(pressedKeys);
await tester.pumpWidget(
Shortcuts(
manager: testManager,
shortcuts: <LogicalKeySet, Intent>{
LogicalKeySet(LogicalKeyboardKey.shift): const TestIntent(),
},
child: Focus(
autofocus: true,
child: Container(key: containerKey, width: 100, height: 100),
),
),
);
await tester.pump();
expect(Shortcuts.maybeOf(containerKey.currentContext!), isNotNull);
expect(Shortcuts.maybeOf(containerKey.currentContext!), equals(testManager));
expect(Shortcuts.of(containerKey.currentContext!), equals(testManager));
});
testWidgets('Shortcuts.of and maybeOf work correctly without shortcuts', (WidgetTester tester) async {
final GlobalKey containerKey = GlobalKey();
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',
));
}
});
testWidgets('ShortcutManager handles shortcuts', (WidgetTester tester) async {
final GlobalKey containerKey = GlobalKey();
final List<LogicalKeyboardKey> pressedKeys = <LogicalKeyboardKey>[];
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment