Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Address previous comments, fix Intent.doNothing. #41417

Merged
merged 4 commits into from Sep 27, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 10 additions & 2 deletions packages/flutter/lib/src/widgets/actions.dart
Expand Up @@ -151,9 +151,17 @@ class ActionDispatcher extends Diagnosticable {
/// The `action` and `intent` arguments must not be null.
///
/// Returns true if the action was successfully invoked.
///
/// If the `intent` argument is [Intent.doNothing], then this function will
/// return true, without invoking the action.
bool invokeAction(Action action, Intent intent, {FocusNode focusNode}) {
assert(action != null);
assert(intent != null);

if (identical(intent, Intent.doNothing)) {
return true;
}

focusNode ??= WidgetsBinding.instance.focusManager.primaryFocus;
if (action != null && intent.isEnabled(focusNode.context)) {
action.invoke(focusNode, intent);
Expand Down Expand Up @@ -280,7 +288,7 @@ class Actions extends InheritedWidget {
/// 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.
/// return true, without looking for a matching action.
static bool invoke(
BuildContext context,
Intent intent, {
Expand All @@ -293,7 +301,7 @@ class Actions extends InheritedWidget {
Action action;

if (identical(intent, Intent.doNothing)) {
return false;
return true;
}

bool visitAncestorElement(Element element) {
Expand Down
26 changes: 21 additions & 5 deletions packages/flutter/test/widgets/actions_test.dart
Expand Up @@ -71,6 +71,22 @@ void main() {
expect(result, isTrue);
expect(invoked, isTrue);
});
test('$ActionDispatcher does nothing if Intent.doNothing is the intent.', () {
bool invoked = false;
const ActionDispatcher dispatcher = ActionDispatcher();
final FocusNode testNode = FocusNode(debugLabel: 'Test Node');
final bool result = dispatcher.invokeAction(
TestAction(
onInvoke: (FocusNode node, Intent invocation) {
invoked = true;
},
),
Intent.doNothing,
focusNode: testNode,
);
expect(result, isTrue);
expect(invoked, isFalse);
});
});
group(Actions, () {
Intent invokedIntent;
Expand Down Expand Up @@ -302,11 +318,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: {}'));
Expand Down
80 changes: 53 additions & 27 deletions packages/flutter/test/widgets/shortcuts_test.dart
Expand Up @@ -37,22 +37,6 @@ class TestIntent extends Intent {
const TestIntent() : super(TestAction.key);
}

class DoNothingAction extends Action {
const DoNothingAction({
@required OnInvokeCallback onInvoke,
}) : assert(onInvoke != null),
super(key);

static const LocalKey key = ValueKey<Type>(DoNothingAction);

@override
void invoke(FocusNode node, Intent invocation) {}
}

class DoNothingIntent extends Intent {
const DoNothingIntent() : super(DoNothingAction.key);
}

class TestShortcutManager extends ShortcutManager {
TestShortcutManager(this.keys);

Expand Down Expand Up @@ -143,8 +127,8 @@ void main() {
LogicalKeyboardKey.keyD,
LogicalKeyboardKey.keyC,
LogicalKeyboardKey.keyB,
LogicalKeyboardKey.keyA,}
);
LogicalKeyboardKey.keyA,
});
final Map<LogicalKeySet, String> map = <LogicalKeySet, String>{set1: 'one'};
expect(set2 == set3, isTrue);
expect(set2 == set4, isTrue);
Expand Down Expand Up @@ -192,10 +176,12 @@ void main() {
await tester.pumpWidget(
Actions(
actions: <LocalKey, ActionFactory>{
TestAction.key: () => TestAction(onInvoke: (FocusNode node, Intent intent) {
invoked = true;
return true;
}),
TestAction.key: () => TestAction(
onInvoke: (FocusNode node, Intent intent) {
invoked = true;
return true;
},
),
},
child: Shortcuts(
manager: testManager,
Expand Down Expand Up @@ -228,14 +214,16 @@ void main() {
},
child: Actions(
actions: <LocalKey, ActionFactory>{
TestAction.key: () => TestAction(onInvoke: (FocusNode node, Intent intent) {
invoked = true;
return true;
}),
TestAction.key: () => TestAction(
onInvoke: (FocusNode node, Intent intent) {
invoked = true;
return true;
},
),
},
child: Shortcuts(
shortcuts: <LogicalKeySet, Intent>{
LogicalKeySet(LogicalKeyboardKey.keyA): const DoNothingIntent(),
LogicalKeySet(LogicalKeyboardKey.keyA): Intent.doNothing,
},
child: Focus(
autofocus: true,
Expand All @@ -251,5 +239,43 @@ void main() {
expect(invoked, isTrue);
expect(pressedKeys, equals(<LogicalKeyboardKey>[LogicalKeyboardKey.shiftLeft]));
});
testWidgets('$Shortcuts can disable a shortcut with Intent.doNothing', (WidgetTester tester) async {
final GlobalKey containerKey = GlobalKey();
final List<LogicalKeyboardKey> pressedKeys = <LogicalKeyboardKey>[];
final TestShortcutManager testManager = TestShortcutManager(pressedKeys);
bool invoked = false;
await tester.pumpWidget(
Shortcuts(
manager: testManager,
shortcuts: <LogicalKeySet, Intent>{
LogicalKeySet(LogicalKeyboardKey.shift): const TestIntent(),
},
child: Actions(
actions: <LocalKey, ActionFactory>{
TestAction.key: () => TestAction(
onInvoke: (FocusNode node, Intent intent) {
invoked = true;
return true;
},
),
},
child: Shortcuts(
shortcuts: <LogicalKeySet, Intent>{
LogicalKeySet(LogicalKeyboardKey.shift): Intent.doNothing,
},
child: Focus(
autofocus: true,
child: Container(key: containerKey, width: 100, height: 100),
),
),
),
),
);
await tester.pump();
expect(Shortcuts.of(containerKey.currentContext), isNotNull);
await tester.sendKeyDownEvent(LogicalKeyboardKey.shiftLeft);
expect(invoked, isFalse);
expect(pressedKeys, isEmpty);
});
});
}