Unverified Commit 8557ffa0 authored by Josh Matthews's avatar Josh Matthews Committed by GitHub

Fix PlatformMenuItems with onSelectedIntent are never enabled (#121885)

Fix PlatformMenuItems with onSelectedIntent are never enabled
parent 91dea4d1
...@@ -738,7 +738,8 @@ class PlatformMenuItem with Diagnosticable { ...@@ -738,7 +738,8 @@ class PlatformMenuItem with Diagnosticable {
/// An optional callback that is called when this [PlatformMenuItem] is /// An optional callback that is called when this [PlatformMenuItem] is
/// selected. /// selected.
/// ///
/// If unset, this menu item will be disabled. /// At most one of [onSelected] and [onSelectedIntent] may be set. If neither
/// field is set, this menu item will be disabled.
final VoidCallback? onSelected; final VoidCallback? onSelected;
/// Returns a callback, if any, to be invoked if the platform menu receives a /// Returns a callback, if any, to be invoked if the platform menu receives a
...@@ -760,7 +761,8 @@ class PlatformMenuItem with Diagnosticable { ...@@ -760,7 +761,8 @@ class PlatformMenuItem with Diagnosticable {
/// An optional intent that is invoked when this [PlatformMenuItem] is /// An optional intent that is invoked when this [PlatformMenuItem] is
/// selected. /// selected.
/// ///
/// If unset, this menu item will be disabled. /// At most one of [onSelected] and [onSelectedIntent] may be set. If neither
/// field is set, this menu item will be disabled.
final Intent? onSelectedIntent; final Intent? onSelectedIntent;
/// Returns all descendant [PlatformMenuItem]s of this item. /// Returns all descendant [PlatformMenuItem]s of this item.
...@@ -805,7 +807,7 @@ class PlatformMenuItem with Diagnosticable { ...@@ -805,7 +807,7 @@ class PlatformMenuItem with Diagnosticable {
return <String, Object?>{ return <String, Object?>{
_kIdKey: getId(item), _kIdKey: getId(item),
_kLabelKey: item.label, _kLabelKey: item.label,
_kEnabledKey: item.onSelected != null, _kEnabledKey: item.onSelected != null || item.onSelectedIntent != null,
if (shortcut != null)...shortcut.serializeForMenu().toChannelRepresentation(), if (shortcut != null)...shortcut.serializeForMenu().toChannelRepresentation(),
}; };
} }
......
...@@ -15,12 +15,12 @@ void main() { ...@@ -15,12 +15,12 @@ void main() {
late FakeMenuChannel fakeMenuChannel; late FakeMenuChannel fakeMenuChannel;
late PlatformMenuDelegate originalDelegate; late PlatformMenuDelegate originalDelegate;
late DefaultPlatformMenuDelegate delegate; late DefaultPlatformMenuDelegate delegate;
final List<String> activated = <String>[]; final List<String> selected = <String>[];
final List<String> opened = <String>[]; final List<String> opened = <String>[];
final List<String> closed = <String>[]; final List<String> closed = <String>[];
void onActivate(String item) { void onSelected(String item) {
activated.add(item); selected.add(item);
} }
void onOpen(String item) { void onOpen(String item) {
...@@ -36,7 +36,7 @@ void main() { ...@@ -36,7 +36,7 @@ void main() {
delegate = DefaultPlatformMenuDelegate(channel: fakeMenuChannel); delegate = DefaultPlatformMenuDelegate(channel: fakeMenuChannel);
originalDelegate = WidgetsBinding.instance.platformMenuDelegate; originalDelegate = WidgetsBinding.instance.platformMenuDelegate;
WidgetsBinding.instance.platformMenuDelegate = delegate; WidgetsBinding.instance.platformMenuDelegate = delegate;
activated.clear(); selected.clear();
opened.clear(); opened.clear();
closed.clear(); closed.clear();
}); });
...@@ -46,117 +46,69 @@ void main() { ...@@ -46,117 +46,69 @@ void main() {
}); });
group('PlatformMenuBar', () { group('PlatformMenuBar', () {
testWidgets('basic menu structure is transmitted to platform', (WidgetTester tester) async { group('basic menu structure is transmitted to platform', () {
await tester.pumpWidget( testWidgets('using onSelected', (WidgetTester tester) async {
MaterialApp( await tester.pumpWidget(
home: Material( MaterialApp(
child: PlatformMenuBar( home: Material(
menus: createTestMenus( child: PlatformMenuBar(
onActivate: onActivate, menus: createTestMenus(
onOpen: onOpen, onSelected: onSelected,
onClose: onClose, onOpen: onOpen,
shortcuts: <String, MenuSerializableShortcut>{ onClose: onClose,
subSubMenu10[0]: const SingleActivator(LogicalKeyboardKey.keyA, control: true), shortcuts: <String, MenuSerializableShortcut>{
subSubMenu10[1]: const SingleActivator(LogicalKeyboardKey.keyB, shift: true), subSubMenu10[0]: const SingleActivator(LogicalKeyboardKey.keyA, control: true),
subSubMenu10[2]: const SingleActivator(LogicalKeyboardKey.keyC, alt: true), subSubMenu10[1]: const SingleActivator(LogicalKeyboardKey.keyB, shift: true),
subSubMenu10[3]: const SingleActivator(LogicalKeyboardKey.keyD, meta: true), subSubMenu10[2]: const SingleActivator(LogicalKeyboardKey.keyC, alt: true),
}, subSubMenu10[3]: const SingleActivator(LogicalKeyboardKey.keyD, meta: true),
},
),
child: const Center(child: Text('Body')),
), ),
child: const Center(child: Text('Body')),
), ),
), ),
), );
);
expect(
expect(fakeMenuChannel.outgoingCalls.last.method, equals('Menu.setMenus')); fakeMenuChannel.outgoingCalls.last.method,
expect( equals('Menu.setMenus'),
fakeMenuChannel.outgoingCalls.last.arguments, );
equals(<String, Object?>{ expect(
'0': <Map<String, Object?>>[ fakeMenuChannel.outgoingCalls.last.arguments,
<String, Object?>{ equals(expectedStructure),
'id': 2, );
'label': 'Menu 0', });
'enabled': true, testWidgets('using onSelectedIntent', (WidgetTester tester) async {
'children': <Map<String, Object?>>[ await tester.pumpWidget(
<String, Object?>{ MaterialApp(
'id': 1, home: Material(
'label': 'Sub Menu 00', child: PlatformMenuBar(
'enabled': true, menus: createTestMenus(
}, onSelectedIntent: const DoNothingIntent(),
], onOpen: onOpen,
}, onClose: onClose,
<String, Object?>{ shortcuts: <String, MenuSerializableShortcut>{
'id': 18, subSubMenu10[0]: const SingleActivator(LogicalKeyboardKey.keyA, control: true),
'label': 'Menu 1', subSubMenu10[1]: const SingleActivator(LogicalKeyboardKey.keyB, shift: true),
'enabled': true, subSubMenu10[2]: const SingleActivator(LogicalKeyboardKey.keyC, alt: true),
'children': <Map<String, Object?>>[ subSubMenu10[3]: const SingleActivator(LogicalKeyboardKey.keyD, meta: true),
<String, Object?>{ },
'id': 4, ),
'label': 'Sub Menu 10', child: const Center(child: Text('Body')),
'enabled': true, ),
}, ),
<String, Object?>{'id': 5, 'isDivider': true}, ),
<String, Object?>{ );
'id': 16,
'label': 'Sub Menu 11', expect(
'enabled': true, fakeMenuChannel.outgoingCalls.last.method,
'children': <Map<String, Object?>>[ equals('Menu.setMenus'),
<String, Object?>{ );
'id': 7, expect(
'label': 'Sub Sub Menu 110', fakeMenuChannel.outgoingCalls.last.arguments,
'enabled': true, equals(expectedStructure),
'shortcutTrigger': 97, );
'shortcutModifiers': 8, });
},
<String, Object?>{'id': 8, 'isDivider': true},
<String, Object?>{
'id': 10,
'label': 'Sub Sub Menu 111',
'enabled': true,
'shortcutTrigger': 98,
'shortcutModifiers': 2,
},
<String, Object?>{'id': 11, 'isDivider': true},
<String, Object?>{
'id': 12,
'label': 'Sub Sub Menu 112',
'enabled': true,
'shortcutTrigger': 99,
'shortcutModifiers': 4,
},
<String, Object?>{'id': 13, 'isDivider': true},
<String, Object?>{
'id': 14,
'label': 'Sub Sub Menu 113',
'enabled': true,
'shortcutTrigger': 100,
'shortcutModifiers': 1,
},
],
},
<String, Object?>{
'id': 17,
'label': 'Sub Menu 12',
'enabled': true,
},
],
},
<String, Object?>{
'id': 20,
'label': 'Menu 2',
'enabled': true,
'children': <Map<String, Object?>>[
<String, Object?>{
'id': 19,
'label': 'Sub Menu 20',
'enabled': false,
},
],
},
<String, Object?>{'id': 21, 'label': 'Menu 3', 'enabled': false, 'children': <Map<String, Object?>>[]},
],
}),
);
}); });
testWidgets('asserts when more than one has locked the delegate', (WidgetTester tester) async { testWidgets('asserts when more than one has locked the delegate', (WidgetTester tester) async {
await tester.pumpWidget( await tester.pumpWidget(
...@@ -287,7 +239,8 @@ const List<String> subMenu2 = <String>[ ...@@ -287,7 +239,8 @@ const List<String> subMenu2 = <String>[
]; ];
List<PlatformMenuItem> createTestMenus({ List<PlatformMenuItem> createTestMenus({
void Function(String)? onActivate, void Function(String)? onSelected,
Intent? onSelectedIntent,
void Function(String)? onOpen, void Function(String)? onOpen,
void Function(String)? onClose, void Function(String)? onClose,
Map<String, MenuSerializableShortcut> shortcuts = const <String, MenuSerializableShortcut>{}, Map<String, MenuSerializableShortcut> shortcuts = const <String, MenuSerializableShortcut>{},
...@@ -301,7 +254,8 @@ List<PlatformMenuItem> createTestMenus({ ...@@ -301,7 +254,8 @@ List<PlatformMenuItem> createTestMenus({
menus: <PlatformMenuItem>[ menus: <PlatformMenuItem>[
PlatformMenuItem( PlatformMenuItem(
label: subMenu0[0], label: subMenu0[0],
onSelected: onActivate != null ? () => onActivate(subMenu0[0]) : null, onSelected: onSelected != null ? () => onSelected(subMenu0[0]) : null,
onSelectedIntent: onSelectedIntent,
shortcut: shortcuts[subMenu0[0]], shortcut: shortcuts[subMenu0[0]],
), ),
], ],
...@@ -315,7 +269,8 @@ List<PlatformMenuItem> createTestMenus({ ...@@ -315,7 +269,8 @@ List<PlatformMenuItem> createTestMenus({
members: <PlatformMenuItem>[ members: <PlatformMenuItem>[
PlatformMenuItem( PlatformMenuItem(
label: subMenu1[0], label: subMenu1[0],
onSelected: onActivate != null ? () => onActivate(subMenu1[0]) : null, onSelected: onSelected != null ? () => onSelected(subMenu0[0]) : null,
onSelectedIntent: onSelectedIntent,
shortcut: shortcuts[subMenu1[0]], shortcut: shortcuts[subMenu1[0]],
), ),
], ],
...@@ -329,7 +284,8 @@ List<PlatformMenuItem> createTestMenus({ ...@@ -329,7 +284,8 @@ List<PlatformMenuItem> createTestMenus({
members: <PlatformMenuItem>[ members: <PlatformMenuItem>[
PlatformMenuItem( PlatformMenuItem(
label: subSubMenu10[0], label: subSubMenu10[0],
onSelected: onActivate != null ? () => onActivate(subSubMenu10[0]) : null, onSelected: onSelected != null ? () => onSelected(subSubMenu10[0]) : null,
onSelectedIntent: onSelectedIntent,
shortcut: shortcuts[subSubMenu10[0]], shortcut: shortcuts[subSubMenu10[0]],
), ),
], ],
...@@ -338,21 +294,24 @@ List<PlatformMenuItem> createTestMenus({ ...@@ -338,21 +294,24 @@ List<PlatformMenuItem> createTestMenus({
members: <PlatformMenuItem>[ members: <PlatformMenuItem>[
PlatformMenuItem( PlatformMenuItem(
label: subSubMenu10[1], label: subSubMenu10[1],
onSelected: onActivate != null ? () => onActivate(subSubMenu10[1]) : null, onSelected: onSelected != null ? () => onSelected(subSubMenu10[1]) : null,
onSelectedIntent: onSelectedIntent,
shortcut: shortcuts[subSubMenu10[1]], shortcut: shortcuts[subSubMenu10[1]],
), ),
], ],
), ),
PlatformMenuItem( PlatformMenuItem(
label: subSubMenu10[2], label: subSubMenu10[2],
onSelected: onActivate != null ? () => onActivate(subSubMenu10[2]) : null, onSelected: onSelected != null ? () => onSelected(subSubMenu10[2]) : null,
onSelectedIntent: onSelectedIntent,
shortcut: shortcuts[subSubMenu10[2]], shortcut: shortcuts[subSubMenu10[2]],
), ),
PlatformMenuItemGroup( PlatformMenuItemGroup(
members: <PlatformMenuItem>[ members: <PlatformMenuItem>[
PlatformMenuItem( PlatformMenuItem(
label: subSubMenu10[3], label: subSubMenu10[3],
onSelected: onActivate != null ? () => onActivate(subSubMenu10[3]) : null, onSelected: onSelected != null ? () => onSelected(subSubMenu10[3]) : null,
onSelectedIntent: onSelectedIntent,
shortcut: shortcuts[subSubMenu10[3]], shortcut: shortcuts[subSubMenu10[3]],
), ),
], ],
...@@ -361,7 +320,8 @@ List<PlatformMenuItem> createTestMenus({ ...@@ -361,7 +320,8 @@ List<PlatformMenuItem> createTestMenus({
), ),
PlatformMenuItem( PlatformMenuItem(
label: subMenu1[2], label: subMenu1[2],
onSelected: onActivate != null ? () => onActivate(subMenu1[2]) : null, onSelected: onSelected != null ? () => onSelected(subMenu1[2]) : null,
onSelectedIntent: onSelectedIntent,
shortcut: shortcuts[subMenu1[2]], shortcut: shortcuts[subMenu1[2]],
), ),
], ],
...@@ -389,6 +349,92 @@ List<PlatformMenuItem> createTestMenus({ ...@@ -389,6 +349,92 @@ List<PlatformMenuItem> createTestMenus({
return result; return result;
} }
const Map<String, Object?> expectedStructure = <String, Object?>{
'0': <Map<String, Object?>>[
<String, Object?>{
'id': 2,
'label': 'Menu 0',
'enabled': true,
'children': <Map<String, Object?>>[
<String, Object?>{
'id': 1,
'label': 'Sub Menu 00',
'enabled': true,
},
],
},
<String, Object?>{
'id': 18,
'label': 'Menu 1',
'enabled': true,
'children': <Map<String, Object?>>[
<String, Object?>{
'id': 4,
'label': 'Sub Menu 10',
'enabled': true,
},
<String, Object?>{'id': 5, 'isDivider': true},
<String, Object?>{
'id': 16,
'label': 'Sub Menu 11',
'enabled': true,
'children': <Map<String, Object?>>[
<String, Object?>{
'id': 7,
'label': 'Sub Sub Menu 110',
'enabled': true,
'shortcutTrigger': 97,
'shortcutModifiers': 8,
},
<String, Object?>{'id': 8, 'isDivider': true},
<String, Object?>{
'id': 10,
'label': 'Sub Sub Menu 111',
'enabled': true,
'shortcutTrigger': 98,
'shortcutModifiers': 2,
},
<String, Object?>{'id': 11, 'isDivider': true},
<String, Object?>{
'id': 12,
'label': 'Sub Sub Menu 112',
'enabled': true,
'shortcutTrigger': 99,
'shortcutModifiers': 4,
},
<String, Object?>{'id': 13, 'isDivider': true},
<String, Object?>{
'id': 14,
'label': 'Sub Sub Menu 113',
'enabled': true,
'shortcutTrigger': 100,
'shortcutModifiers': 1,
},
],
},
<String, Object?>{
'id': 17,
'label': 'Sub Menu 12',
'enabled': true,
},
],
},
<String, Object?>{
'id': 20,
'label': 'Menu 2',
'enabled': true,
'children': <Map<String, Object?>>[
<String, Object?>{
'id': 19,
'label': 'Sub Menu 20',
'enabled': false,
},
],
},
<String, Object?>{'id': 21, 'label': 'Menu 3', 'enabled': false, 'children': <Map<String, Object?>>[]},
],
};
class FakeMenuChannel implements MethodChannel { class FakeMenuChannel implements MethodChannel {
FakeMenuChannel(this.outgoing); FakeMenuChannel(this.outgoing);
......
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