Skip to content

Commit

Permalink
Revert "Address previous comments, fix Intent.doNothing. (flutter#41417…
Browse files Browse the repository at this point in the history 
…)" (flutter#41449)

This reverts commit eb3e2f5 because of a suspected analyzer bug.  Reverting to investigate.
  • Loading branch information
@gspencergoog
gspencergoog committed Sep 27, 2019
1 parent 4e820ce commit 957d839
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 120 deletions.
26 changes: 13 additions & 13 deletions dev/manual_tests/lib/actions.dart
View file
Expand Up @@ -235,9 +235,9 @@ abstract class UndoableAction extends Action {

@override
@mustCallSuper
void invoke(FocusNode node, Intent intent) {
void invoke(FocusNode node, Intent tag) {
invocationNode = node;
invocationIntent = intent;
invocationIntent = tag;
}

@override
Expand All @@ -253,8 +253,8 @@ class SetFocusActionBase extends UndoableAction {
FocusNode _previousFocus;

@override
void invoke(FocusNode node, Intent intent) {
super.invoke(node, intent);
void invoke(FocusNode node, Intent tag) {
super.invoke(node, tag);
_previousFocus = WidgetsBinding.instance.focusManager.primaryFocus;
node.requestFocus();
}
Expand Down Expand Up @@ -292,8 +292,8 @@ class SetFocusAction extends SetFocusActionBase {
static const LocalKey key = ValueKey<Type>(SetFocusAction);

@override
void invoke(FocusNode node, Intent intent) {
super.invoke(node, intent);
void invoke(FocusNode node, Intent tag) {
super.invoke(node, tag);
node.requestFocus();
}
}
Expand All @@ -305,8 +305,8 @@ class NextFocusAction extends SetFocusActionBase {
static const LocalKey key = ValueKey<Type>(NextFocusAction);

@override
void invoke(FocusNode node, Intent intent) {
super.invoke(node, intent);
void invoke(FocusNode node, Intent tag) {
super.invoke(node, tag);
node.nextFocus();
}
}
Expand All @@ -317,8 +317,8 @@ class PreviousFocusAction extends SetFocusActionBase {
static const LocalKey key = ValueKey<Type>(PreviousFocusAction);

@override
void invoke(FocusNode node, Intent intent) {
super.invoke(node, intent);
void invoke(FocusNode node, Intent tag) {
super.invoke(node, tag);
node.previousFocus();
}
}
Expand All @@ -337,9 +337,9 @@ class DirectionalFocusAction extends SetFocusActionBase {
TraversalDirection direction;

@override
void invoke(FocusNode node, DirectionalFocusIntent intent) {
super.invoke(node, intent);
final DirectionalFocusIntent args = intent;
void invoke(FocusNode node, DirectionalFocusIntent tag) {
super.invoke(node, tag);
final DirectionalFocusIntent args = tag;
node.focusInDirection(args.direction);
}
}
Expand Down
47 changes: 14 additions & 33 deletions packages/flutter/lib/src/widgets/actions.dart
View file
Expand Up @@ -29,10 +29,11 @@ class Intent extends Diagnosticable {

/// An intent that can't be mapped to an action.
///
/// This Intent is mapped to an action in the [WidgetsApp] that does nothing,
/// so that it can be bound to a key in a [Shortcuts] widget in order to
/// disable a key binding made above it in the hierarchy.
static const Intent doNothing = DoNothingIntent();
/// 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;
Expand Down Expand Up @@ -94,7 +95,7 @@ abstract class Action extends Diagnosticable {
/// needed in the action, use [ActionDispatcher.invokeFocusedAction] instead.
@protected
@mustCallSuper
void invoke(FocusNode node, covariant Intent intent);
void invoke(FocusNode node, covariant Intent tag);

@override
void debugFillProperties(DiagnosticPropertiesBuilder properties) {
Expand Down Expand Up @@ -133,7 +134,7 @@ class CallbackAction extends Action {
final OnInvokeCallback onInvoke;

@override
void invoke(FocusNode node, Intent intent) => onInvoke.call(node, intent);
void invoke(FocusNode node, Intent tag) => onInvoke.call(node, tag);
}

/// An action manager that simply invokes the actions given to it.
Expand Down Expand Up @@ -223,7 +224,6 @@ class Actions extends InheritedWidget {
// Stop visiting.
return false;
}

element.visitAncestorElements(visitAncestorElement);
}
return dispatcher ?? const ActionDispatcher();
Expand Down Expand Up @@ -278,6 +278,9 @@ class Actions extends InheritedWidget {
///
/// 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, {
Expand All @@ -289,6 +292,10 @@ class Actions extends InheritedWidget {
Element actionsElement;
Action action;

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

bool visitAncestorElement(Element element) {
if (element.widget is! Actions) {
// Continue visiting.
Expand Down Expand Up @@ -351,29 +358,3 @@ class Actions extends InheritedWidget {
properties.add(DiagnosticsProperty<Map<LocalKey, ActionFactory>>('actions', actions));
}
}

/// An [Action], that, as the name implies, does nothing.
///
/// This action is bound to the [Intent.doNothing] intent inside of
/// [WidgetsApp.build] so that a [Shortcuts] widget can bind a key to it to
/// override another shortcut binding defined above it in the hierarchy.
class DoNothingAction extends Action {
/// Const constructor for [DoNothingAction].
const DoNothingAction() : super(key);

/// The Key used for the [DoNothingIntent] intent, and registered at the top
/// level actions in [WidgetsApp.build].
static const LocalKey key = ValueKey<Type>(DoNothingAction);

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

/// An [Intent] that can be used to disable [Shortcuts] key bindings defined
/// above a widget in the hierarchy.
///
/// The [Intent.doNothing] intent is of this type.
class DoNothingIntent extends Intent {
/// Const constructor for [DoNothingIntent].
const DoNothingIntent() : super(DoNothingAction.key);
}
22 changes: 8 additions & 14 deletions packages/flutter/lib/src/widgets/app.dart
View file
Expand Up @@ -8,7 +8,6 @@ import 'dart:collection' show HashMap;
import 'package:flutter/foundation.dart';
import 'package:flutter/rendering.dart';

import 'actions.dart';
import 'banner.dart';
import 'basic.dart';
import 'binding.dart';
Expand Down Expand Up @@ -1195,19 +1194,14 @@ class _WidgetsAppState extends State<WidgetsApp> implements WidgetsBindingObserv

assert(_debugCheckLocalizations(appLocale));

return Actions(
actions: <LocalKey, ActionFactory>{
DoNothingAction.key: () => const DoNothingAction(),
},
child: DefaultFocusTraversal(
policy: ReadingOrderTraversalPolicy(),
child: MediaQuery(
data: MediaQueryData.fromWindow(WidgetsBinding.instance.window),
child: Localizations(
locale: appLocale,
delegates: _localizationsDelegates.toList(),
child: title,
),
return DefaultFocusTraversal(
policy: ReadingOrderTraversalPolicy(),
child: MediaQuery(
data: MediaQueryData.fromWindow(WidgetsBinding.instance.window),
child: Localizations(
locale: appLocale,
delegates: _localizationsDelegates.toList(),
child: title,
),
),
);
Expand Down
10 changes: 5 additions & 5 deletions packages/flutter/test/widgets/actions_test.dart
View file
Expand Up @@ -302,11 +302,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
82 changes: 27 additions & 55 deletions packages/flutter/test/widgets/shortcuts_test.dart
View file
Expand Up @@ -37,6 +37,22 @@ 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 @@ -127,8 +143,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 @@ -176,12 +192,10 @@ 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 @@ -214,16 +228,14 @@ 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): Intent.doNothing,
LogicalKeySet(LogicalKeyboardKey.keyA): const DoNothingIntent(),
},
child: Focus(
autofocus: true,
Expand All @@ -239,45 +251,5 @@ 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(
MaterialApp(
home: 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);
});
});
}

0 comments on commit 957d839

Please sign in to comment.