Skip to content

Commit eb3e2f5

Browse files
gspencergooggspencergoog
authored
Address previous comments, fix Intent.doNothing. (#41417)
This addresses comments in the original PR (#41245) that introduced Intent.doNothing, adds tests, and fixes an issue with it.
1 parent b07056b commit eb3e2f5

File tree

5 files changed

+120
-67
lines changed

5 files changed

+120
-67
lines changed

‎dev/manual_tests/lib/actions.dart‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -235,9 +235,9 @@ abstract class UndoableAction extends Action {
235235

236236
@override
237237
@mustCallSuper
238-
void invoke(FocusNode node, Intent tag) {
238+
void invoke(FocusNode node, Intent intent) {
239239
invocationNode = node;
240-
invocationIntent = tag;
240+
invocationIntent = intent;
241241
}
242242

243243
@override
@@ -253,8 +253,8 @@ class SetFocusActionBase extends UndoableAction {
253253
FocusNode _previousFocus;
254254

255255
@override
256-
void invoke(FocusNode node, Intent tag) {
257-
super.invoke(node, tag);
256+
void invoke(FocusNode node, Intent intent) {
257+
super.invoke(node, intent);
258258
_previousFocus = WidgetsBinding.instance.focusManager.primaryFocus;
259259
node.requestFocus();
260260
}
@@ -292,8 +292,8 @@ class SetFocusAction extends SetFocusActionBase {
292292
static const LocalKey key = ValueKey<Type>(SetFocusAction);
293293

294294
@override
295-
void invoke(FocusNode node, Intent tag) {
296-
super.invoke(node, tag);
295+
void invoke(FocusNode node, Intent intent) {
296+
super.invoke(node, intent);
297297
node.requestFocus();
298298
}
299299
}
@@ -305,8 +305,8 @@ class NextFocusAction extends SetFocusActionBase {
305305
static const LocalKey key = ValueKey<Type>(NextFocusAction);
306306

307307
@override
308-
void invoke(FocusNode node, Intent tag) {
309-
super.invoke(node, tag);
308+
void invoke(FocusNode node, Intent intent) {
309+
super.invoke(node, intent);
310310
node.nextFocus();
311311
}
312312
}
@@ -317,8 +317,8 @@ class PreviousFocusAction extends SetFocusActionBase {
317317
static const LocalKey key = ValueKey<Type>(PreviousFocusAction);
318318

319319
@override
320-
void invoke(FocusNode node, Intent tag) {
321-
super.invoke(node, tag);
320+
void invoke(FocusNode node, Intent intent) {
321+
super.invoke(node, intent);
322322
node.previousFocus();
323323
}
324324
}
@@ -337,9 +337,9 @@ class DirectionalFocusAction extends SetFocusActionBase {
337337
TraversalDirection direction;
338338

339339
@override
340-
void invoke(FocusNode node, DirectionalFocusIntent tag) {
341-
super.invoke(node, tag);
342-
final DirectionalFocusIntent args = tag;
340+
void invoke(FocusNode node, DirectionalFocusIntent intent) {
341+
super.invoke(node, intent);
342+
final DirectionalFocusIntent args = intent;
343343
node.focusInDirection(args.direction);
344344
}
345345
}

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

Lines changed: 33 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,10 @@ class Intent extends Diagnosticable {
2929

3030
/// An intent that can't be mapped to an action.
3131
///
32-
/// This Intent is prevented from being mapped to an action in the
33-
/// [ActionDispatcher], and as such can be used to indicate that a shortcut
34-
/// should not do anything, allowing a shortcut defined at a higher level to
35-
/// be disabled at a deeper level in the widget hierarchy.
36-
static const Intent doNothing = Intent(ValueKey<Type>(Intent));
32+
/// This Intent is mapped to an action in the [WidgetsApp] that does nothing,
33+
/// so that it can be bound to a key in a [Shortcuts] widget in order to
34+
/// disable a key binding made above it in the hierarchy.
35+
static const Intent doNothing = DoNothingIntent();
3736

3837
/// The key for the action this intent is associated with.
3938
final LocalKey key;
@@ -95,7 +94,7 @@ abstract class Action extends Diagnosticable {
9594
/// needed in the action, use [ActionDispatcher.invokeFocusedAction] instead.
9695
@protected
9796
@mustCallSuper
98-
void invoke(FocusNode node, covariant Intent tag);
97+
void invoke(FocusNode node, covariant Intent intent);
9998

10099
@override
101100
void debugFillProperties(DiagnosticPropertiesBuilder properties) {
@@ -134,7 +133,7 @@ class CallbackAction extends Action {
134133
final OnInvokeCallback onInvoke;
135134

136135
@override
137-
void invoke(FocusNode node, Intent tag) => onInvoke.call(node, tag);
136+
void invoke(FocusNode node, Intent intent) => onInvoke.call(node, intent);
138137
}
139138

140139
/// An action manager that simply invokes the actions given to it.
@@ -224,6 +223,7 @@ class Actions extends InheritedWidget {
224223
// Stop visiting.
225224
return false;
226225
}
226+
227227
element.visitAncestorElements(visitAncestorElement);
228228
}
229229
return dispatcher ?? const ActionDispatcher();
@@ -278,9 +278,6 @@ class Actions extends InheritedWidget {
278278
///
279279
/// Setting `nullOk` to true means that if no ambient [Actions] widget is
280280
/// found, then this method will return false instead of throwing.
281-
///
282-
/// If the `intent` argument is [Intent.doNothing], then this function will
283-
/// return false, without looking for a matching action.
284281
static bool invoke(
285282
BuildContext context,
286283
Intent intent, {
@@ -292,10 +289,6 @@ class Actions extends InheritedWidget {
292289
Element actionsElement;
293290
Action action;
294291

295-
if (identical(intent, Intent.doNothing)) {
296-
return false;
297-
}
298-
299292
bool visitAncestorElement(Element element) {
300293
if (element.widget is! Actions) {
301294
// Continue visiting.
@@ -358,3 +351,29 @@ class Actions extends InheritedWidget {
358351
properties.add(DiagnosticsProperty<Map<LocalKey, ActionFactory>>('actions', actions));
359352
}
360353
}
354+
355+
/// An [Action], that, as the name implies, does nothing.
356+
///
357+
/// This action is bound to the [Intent.doNothing] intent inside of
358+
/// [WidgetsApp.build] so that a [Shortcuts] widget can bind a key to it to
359+
/// override another shortcut binding defined above it in the hierarchy.
360+
class DoNothingAction extends Action {
361+
/// Const constructor for [DoNothingAction].
362+
const DoNothingAction() : super(key);
363+
364+
/// The Key used for the [DoNothingIntent] intent, and registered at the top
365+
/// level actions in [WidgetsApp.build].
366+
static const LocalKey key = ValueKey<Type>(DoNothingAction);
367+
368+
@override
369+
void invoke(FocusNode node, Intent intent) { }
370+
}
371+
372+
/// An [Intent] that can be used to disable [Shortcuts] key bindings defined
373+
/// above a widget in the hierarchy.
374+
///
375+
/// The [Intent.doNothing] intent is of this type.
376+
class DoNothingIntent extends Intent {
377+
/// Const constructor for [DoNothingIntent].
378+
const DoNothingIntent() : super(DoNothingAction.key);
379+
}

‎packages/flutter/lib/src/widgets/app.dart‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import 'dart:collection' show HashMap;
88
import 'package:flutter/foundation.dart';
99
import 'package:flutter/rendering.dart';
1010

11+
import 'actions.dart';
1112
import 'banner.dart';
1213
import 'basic.dart';
1314
import 'binding.dart';
@@ -1194,14 +1195,19 @@ class _WidgetsAppState extends State<WidgetsApp> implements WidgetsBindingObserv
11941195

11951196
assert(_debugCheckLocalizations(appLocale));
11961197

1197-
return DefaultFocusTraversal(
1198-
policy: ReadingOrderTraversalPolicy(),
1199-
child: MediaQuery(
1200-
data: MediaQueryData.fromWindow(WidgetsBinding.instance.window),
1201-
child: Localizations(
1202-
locale: appLocale,
1203-
delegates: _localizationsDelegates.toList(),
1204-
child: title,
1198+
return Actions(
1199+
actions: <LocalKey, ActionFactory>{
1200+
DoNothingAction.key: () => const DoNothingAction(),
1201+
},
1202+
child: DefaultFocusTraversal(
1203+
policy: ReadingOrderTraversalPolicy(),
1204+
child: MediaQuery(
1205+
data: MediaQueryData.fromWindow(WidgetsBinding.instance.window),
1206+
child: Localizations(
1207+
locale: appLocale,
1208+
delegates: _localizationsDelegates.toList(),
1209+
child: title,
1210+
),
12051211
),
12061212
),
12071213
);

‎packages/flutter/test/widgets/actions_test.dart‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -302,11 +302,11 @@ void main() {
302302
).debugFillProperties(builder);
303303

304304
final List<String> description = builder.properties
305-
.where((DiagnosticsNode node) {
306-
return !node.isFiltered(DiagnosticLevel.info);
307-
})
308-
.map((DiagnosticsNode node) => node.toString())
309-
.toList();
305+
.where((DiagnosticsNode node) {
306+
return !node.isFiltered(DiagnosticLevel.info);
307+
})
308+
.map((DiagnosticsNode node) => node.toString())
309+
.toList();
310310

311311
expect(description[0], equalsIgnoringHashCodes('dispatcher: ActionDispatcher#00000'));
312312
expect(description[1], equals('actions: {}'));

‎packages/flutter/test/widgets/shortcuts_test.dart‎

Lines changed: 55 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -37,22 +37,6 @@ class TestIntent extends Intent {
3737
const TestIntent() : super(TestAction.key);
3838
}
3939

40-
class DoNothingAction extends Action {
41-
const DoNothingAction({
42-
@required OnInvokeCallback onInvoke,
43-
}) : assert(onInvoke != null),
44-
super(key);
45-
46-
static const LocalKey key = ValueKey<Type>(DoNothingAction);
47-
48-
@override
49-
void invoke(FocusNode node, Intent invocation) {}
50-
}
51-
52-
class DoNothingIntent extends Intent {
53-
const DoNothingIntent() : super(DoNothingAction.key);
54-
}
55-
5640
class TestShortcutManager extends ShortcutManager {
5741
TestShortcutManager(this.keys);
5842

@@ -143,8 +127,8 @@ void main() {
143127
LogicalKeyboardKey.keyD,
144128
LogicalKeyboardKey.keyC,
145129
LogicalKeyboardKey.keyB,
146-
LogicalKeyboardKey.keyA,}
147-
);
130+
LogicalKeyboardKey.keyA,
131+
});
148132
final Map<LogicalKeySet, String> map = <LogicalKeySet, String>{set1: 'one'};
149133
expect(set2 == set3, isTrue);
150134
expect(set2 == set4, isTrue);
@@ -192,10 +176,12 @@ void main() {
192176
await tester.pumpWidget(
193177
Actions(
194178
actions: <LocalKey, ActionFactory>{
195-
TestAction.key: () => TestAction(onInvoke: (FocusNode node, Intent intent) {
196-
invoked = true;
197-
return true;
198-
}),
179+
TestAction.key: () => TestAction(
180+
onInvoke: (FocusNode node, Intent intent) {
181+
invoked = true;
182+
return true;
183+
},
184+
),
199185
},
200186
child: Shortcuts(
201187
manager: testManager,
@@ -228,14 +214,16 @@ void main() {
228214
},
229215
child: Actions(
230216
actions: <LocalKey, ActionFactory>{
231-
TestAction.key: () => TestAction(onInvoke: (FocusNode node, Intent intent) {
232-
invoked = true;
233-
return true;
234-
}),
217+
TestAction.key: () => TestAction(
218+
onInvoke: (FocusNode node, Intent intent) {
219+
invoked = true;
220+
return true;
221+
},
222+
),
235223
},
236224
child: Shortcuts(
237225
shortcuts: <LogicalKeySet, Intent>{
238-
LogicalKeySet(LogicalKeyboardKey.keyA): const DoNothingIntent(),
226+
LogicalKeySet(LogicalKeyboardKey.keyA): Intent.doNothing,
239227
},
240228
child: Focus(
241229
autofocus: true,
@@ -251,5 +239,45 @@ void main() {
251239
expect(invoked, isTrue);
252240
expect(pressedKeys, equals(<LogicalKeyboardKey>[LogicalKeyboardKey.shiftLeft]));
253241
});
242+
testWidgets('$Shortcuts can disable a shortcut with Intent.doNothing', (WidgetTester tester) async {
243+
final GlobalKey containerKey = GlobalKey();
244+
final List<LogicalKeyboardKey> pressedKeys = <LogicalKeyboardKey>[];
245+
final TestShortcutManager testManager = TestShortcutManager(pressedKeys);
246+
bool invoked = false;
247+
await tester.pumpWidget(
248+
MaterialApp(
249+
home: Shortcuts(
250+
manager: testManager,
251+
shortcuts: <LogicalKeySet, Intent>{
252+
LogicalKeySet(LogicalKeyboardKey.shift): const TestIntent(),
253+
},
254+
child: Actions(
255+
actions: <LocalKey, ActionFactory>{
256+
TestAction.key: () => TestAction(
257+
onInvoke: (FocusNode node, Intent intent) {
258+
invoked = true;
259+
return true;
260+
},
261+
),
262+
},
263+
child: Shortcuts(
264+
shortcuts: <LogicalKeySet, Intent>{
265+
LogicalKeySet(LogicalKeyboardKey.shift): Intent.doNothing,
266+
},
267+
child: Focus(
268+
autofocus: true,
269+
child: Container(key: containerKey, width: 100, height: 100),
270+
),
271+
),
272+
),
273+
),
274+
),
275+
);
276+
await tester.pump();
277+
expect(Shortcuts.of(containerKey.currentContext), isNotNull);
278+
await tester.sendKeyDownEvent(LogicalKeyboardKey.shiftLeft);
279+
expect(invoked, isFalse);
280+
expect(pressedKeys, isEmpty);
281+
});
254282
});
255283
}

0 commit comments

Comments
 (0)