Unverified Commit 36767d01 authored by Greg Spencer's avatar Greg Spencer Committed by GitHub

Make Action.enabled be isEnabled(Intent intent) instead. (#55230)

parent 8ee26efb
...@@ -193,7 +193,7 @@ class UndoIntent extends Intent { ...@@ -193,7 +193,7 @@ class UndoIntent extends Intent {
class UndoAction extends Action<UndoIntent> { class UndoAction extends Action<UndoIntent> {
@override @override
bool get enabled { bool isEnabled(UndoIntent intent) {
final UndoableActionDispatcher manager = Actions.of(primaryFocus?.context ?? FocusDemo.appKey.currentContext, nullOk: true) as UndoableActionDispatcher; final UndoableActionDispatcher manager = Actions.of(primaryFocus?.context ?? FocusDemo.appKey.currentContext, nullOk: true) as UndoableActionDispatcher;
return manager.canUndo; return manager.canUndo;
} }
...@@ -211,7 +211,7 @@ class RedoIntent extends Intent { ...@@ -211,7 +211,7 @@ class RedoIntent extends Intent {
class RedoAction extends Action<RedoIntent> { class RedoAction extends Action<RedoIntent> {
@override @override
bool get enabled { bool isEnabled(RedoIntent intent) {
final UndoableActionDispatcher manager = Actions.of(primaryFocus.context, nullOk: true) as UndoableActionDispatcher; final UndoableActionDispatcher manager = Actions.of(primaryFocus.context, nullOk: true) as UndoableActionDispatcher;
return manager.canRedo; return manager.canRedo;
} }
......
...@@ -82,7 +82,7 @@ abstract class Action<T extends Intent> with Diagnosticable { ...@@ -82,7 +82,7 @@ abstract class Action<T extends Intent> with Diagnosticable {
/// ///
/// If the enabled state changes, overriding subclasses must call /// If the enabled state changes, overriding subclasses must call
/// [notifyActionListeners] to notify any listeners of the change. /// [notifyActionListeners] to notify any listeners of the change.
bool get enabled => true; bool isEnabled(covariant T intent) => true;
/// Called when the action is to be performed. /// Called when the action is to be performed.
/// ///
...@@ -379,7 +379,7 @@ class ActionDispatcher with Diagnosticable { ...@@ -379,7 +379,7 @@ class ActionDispatcher with Diagnosticable {
assert(action != null); assert(action != null);
assert(intent != null); assert(intent != null);
context ??= primaryFocus?.context; context ??= primaryFocus?.context;
if (action.enabled) { if (action.isEnabled(intent)) {
if (action is ContextAction) { if (action is ContextAction) {
return action.invoke(intent, context); return action.invoke(intent, context);
} else { } else {
...@@ -486,7 +486,7 @@ class Actions extends StatefulWidget { ...@@ -486,7 +486,7 @@ class Actions extends StatefulWidget {
/// updated action. /// updated action.
static VoidCallback handler<T extends Intent>(BuildContext context, T intent, {bool nullOk = false}) { static VoidCallback handler<T extends Intent>(BuildContext context, T intent, {bool nullOk = false}) {
final Action<T> action = Actions.find<T>(context, nullOk: nullOk); final Action<T> action = Actions.find<T>(context, nullOk: nullOk);
if (action != null && action.enabled) { if (action != null && action.isEnabled(intent)) {
return () { return () {
Actions.of(context).invokeAction(action, intent, context); Actions.of(context).invokeAction(action, intent, context);
}; };
...@@ -635,12 +635,10 @@ class Actions extends StatefulWidget { ...@@ -635,12 +635,10 @@ class Actions extends StatefulWidget {
} }
class _ActionsState extends State<Actions> { class _ActionsState extends State<Actions> {
// Keeps the last-known enabled state of each action in the action map in // The set of actions that this Actions widget is current listening to.
// order to be able to appropriately notify dependents that the state has Set<Action<Intent>> listenedActions = <Action<Intent>>{};
// changed. // Used to tell the marker to rebuild its dependencies when the state of an
Map<Action<Intent>, bool> enabledState = <Action<Intent>, bool>{}; // action in the map changes.
// Used to tell the marker to rebuild when the enabled state of an action in
// the map changes.
Object rebuildKey = Object(); Object rebuildKey = Object();
@override @override
...@@ -650,42 +648,24 @@ class _ActionsState extends State<Actions> { ...@@ -650,42 +648,24 @@ class _ActionsState extends State<Actions> {
} }
void _handleActionChanged(Action<Intent> action) { void _handleActionChanged(Action<Intent> action) {
assert(enabledState.containsKey(action)); // Generate a new key so that the marker notifies dependents.
final bool actionEnabled = action.enabled; setState(() {
if (enabledState[action] == null || enabledState[action] != actionEnabled) { rebuildKey = Object();
setState(() { });
enabledState[action] = actionEnabled;
// Generate a new key so that the marker notifies dependents.
rebuildKey = Object();
});
}
} }
void _updateActionListeners() { void _updateActionListeners() {
final Map<Action<Intent>, bool> newState = <Action<Intent>, bool>{}; final Set<Action<Intent>> widgetActions = widget.actions.values.toSet();
final Set<Action<Intent>> foundActions = <Action<Intent>>{}; final Set<Action<Intent>> removedActions = listenedActions.difference(widgetActions);
for (final Action<Intent> action in widget.actions.values) { final Set<Action<Intent>> addedActions = widgetActions.difference(listenedActions);
if (enabledState.containsKey(action)) {
// Already subscribed to this action, just copy over the current enabled state. for (final Action<Intent> action in removedActions) {
newState[action] = enabledState[action]; action.removeActionListener(_handleActionChanged);
foundActions.add(action);
} else {
// New action to subscribe to.
// Don't set the new state to action.enabled, since that can cause
// problems when the enabled accessor looks up other widgets (which may
// have already been removed from the tree).
newState[action] = null;
action.addActionListener(_handleActionChanged);
}
} }
// Unregister from any actions in the previous enabledState map that aren't for (final Action<Intent> action in addedActions) {
// going to be transferred to the new one. action.addActionListener(_handleActionChanged);
for (final Action<Intent> action in enabledState.keys) {
if (!foundActions.contains(action)) {
action.removeActionListener(_handleActionChanged);
}
} }
enabledState = newState; listenedActions = widgetActions;
} }
@override @override
...@@ -697,10 +677,10 @@ class _ActionsState extends State<Actions> { ...@@ -697,10 +677,10 @@ class _ActionsState extends State<Actions> {
@override @override
void dispose() { void dispose() {
super.dispose(); super.dispose();
for (final Action<Intent> action in enabledState.keys) { for (final Action<Intent> action in listenedActions) {
action.removeActionListener(_handleActionChanged); action.removeActionListener(_handleActionChanged);
} }
enabledState = null; listenedActions = null;
} }
@override @override
......
...@@ -911,7 +911,7 @@ class ScrollAction extends Action<ScrollIntent> { ...@@ -911,7 +911,7 @@ class ScrollAction extends Action<ScrollIntent> {
static const LocalKey key = ValueKey<Type>(ScrollAction); static const LocalKey key = ValueKey<Type>(ScrollAction);
@override @override
bool get enabled { bool isEnabled(ScrollIntent intent) {
final FocusNode focus = primaryFocus; final FocusNode focus = primaryFocus;
return focus != null && focus.context != null && Scrollable.of(focus.context) != null; return focus != null && focus.context != null && Scrollable.of(focus.context) != null;
} }
......
...@@ -30,6 +30,8 @@ class TestAction extends CallbackAction<TestIntent> { ...@@ -30,6 +30,8 @@ class TestAction extends CallbackAction<TestIntent> {
super(onInvoke: onInvoke); super(onInvoke: onInvoke);
@override @override
bool isEnabled(TestIntent intent) => enabled;
bool get enabled => _enabled; bool get enabled => _enabled;
bool _enabled = true; bool _enabled = true;
set enabled(bool value) { set enabled(bool value) {
...@@ -412,17 +414,17 @@ void main() { ...@@ -412,17 +414,17 @@ void main() {
}, },
); );
bool enabled1 = true; bool enabled1 = true;
action1.addActionListener((Action<Intent> action) => enabled1 = action.enabled); action1.addActionListener((Action<Intent> action) => enabled1 = action.isEnabled(const TestIntent()));
action1.enabled = false; action1.enabled = false;
expect(enabled1, isFalse); expect(enabled1, isFalse);
bool enabled2 = true; bool enabled2 = true;
action2.addActionListener((Action<Intent> action) => enabled2 = action.enabled); action2.addActionListener((Action<Intent> action) => enabled2 = action.isEnabled(const SecondTestIntent()));
action2.enabled = false; action2.enabled = false;
expect(enabled2, isFalse); expect(enabled2, isFalse);
bool enabled3 = true; bool enabled3 = true;
action3.addActionListener((Action<Intent> action) => enabled3 = action.enabled); action3.addActionListener((Action<Intent> action) => enabled3 = action.isEnabled(const ThirdTestIntent()));
action3.enabled = false; action3.enabled = false;
expect(enabled3, isFalse); expect(enabled3, isFalse);
...@@ -466,7 +468,7 @@ void main() { ...@@ -466,7 +468,7 @@ void main() {
SecondTestIntent: action2, SecondTestIntent: action2,
}, },
child: ActionListener( child: ActionListener(
listener: (Action<Intent> action) => enabledChanged = action.enabled, listener: (Action<Intent> action) => enabledChanged = action.isEnabled(const ThirdTestIntent()),
action: action2, action: action2,
child: Actions( child: Actions(
actions: <Type, Action<Intent>>{ actions: <Type, Action<Intent>>{
......
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