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.

Sign up for GitHub

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

Jump to bottom

PlatformMenuBar changes to bring it into line with upcoming MenuBar implementation #104565

Merged
merged 1 commit into from
May 25, 2022
Merged

PlatformMenuBar changes to bring it into line with upcoming MenuBar implementation #104565

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion examples/api/lib/material/platform_menu_bar/platform_menu_bar.0.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ class _MyMenuBarAppState extends State<MyMenuBarApp> {
],
),
],
body: Center(
child: Center(
child: Text(_showMessage
? _message
: 'This space intentionally left blank.\n'
Expand Down
165 changes: 143 additions & 22 deletions packages/flutter/lib/src/widgets/platform_menu_bar.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ import 'dart:async';
import 'package:flutter/foundation.dart';
import 'package:flutter/services.dart';

import 'actions.dart';
import 'basic.dart';
import 'binding.dart';
import 'focus_manager.dart';
import 'framework.dart';
import 'shortcuts.dart';

Expand Down Expand Up @@ -44,6 +47,7 @@ class ShortcutSerialization {
/// This is used by a [CharacterActivator] to serialize itself.
ShortcutSerialization.character(String character)
: _internal = <String, Object?>{_kShortcutCharacter: character},
_character = character,
assert(character.length == 1);

/// Creates a [ShortcutSerialization] representing a specific
Expand All @@ -70,6 +74,11 @@ class ShortcutSerialization {
trigger != LogicalKeyboardKey.metaRight,
'Specifying a modifier key as a trigger is not allowed. '
'Use provided boolean parameters instead.'),
_trigger = trigger,
_control = control,
_shift = shift,
_alt = alt,
_meta = meta,
_internal = <String, Object?>{
_kShortcutTrigger: trigger.keyId,
_kShortcutModifiers: (control ? _shortcutModifierControl : 0) |
Expand All @@ -80,6 +89,35 @@ class ShortcutSerialization {

final Map<String, Object?> _internal;

/// The keyboard key that triggers this shortcut, if any.
LogicalKeyboardKey? get trigger => _trigger;
LogicalKeyboardKey? _trigger;

/// The character that triggers this shortcut, if any.
String? get character => _character;
String? _character;

/// If this shortcut has a [trigger], this indicates whether or not the
/// control modifier needs to be down or not.
bool? get control => _control;
bool? _control;

/// If this shortcut has a [trigger], this indicates whether or not the
/// shift modifier needs to be down or not.
bool? get shift => _shift;
bool? _shift;

/// If this shortcut has a [trigger], this indicates whether or not the
/// alt modifier needs to be down or not.
bool? get alt => _alt;
bool? _alt;

/// If this shortcut has a [trigger], this indicates whether or not the meta
/// (also known as the Windows or Command key) modifier needs to be down or
/// not.
bool? get meta => _meta;
bool? _meta;

/// The bit mask for the [LogicalKeyboardKey.meta] key (or it's left/right
/// equivalents) being down.
static const int _shortcutModifierMeta = 1 << 0;
Expand Down Expand Up @@ -124,10 +162,13 @@ mixin MenuSerializableShortcut {
/// An abstract class for describing cascading menu hierarchies that are part of
/// a [PlatformMenuBar].
///
/// This type is used by [PlatformMenuDelegate.setMenus] to accept the menu
/// This type is also used by [PlatformMenuDelegate.setMenus] to accept the menu
/// hierarchy to be sent to the platform, and by [PlatformMenuBar] to define the
/// menu hierarchy.
///
/// This class is abstract, and so can't be used directly. Typically subclasses
/// like [PlatformMenuItem] are used.
///
/// See also:
///
/// * [PlatformMenuBar], a widget that renders menu items using platform APIs
Expand All @@ -141,8 +182,8 @@ abstract class MenuItem with Diagnosticable {
///
/// The `delegate` is the [PlatformMenuDelegate] that is requesting the
/// serialization. The `index` is the position of this menu item in the list
/// of children of the [PlatformMenu] it belongs to, and `count` is the number
/// of children in the [PlatformMenu] it belongs to.
/// of [menus] of the [PlatformMenu] it belongs to, and `count` is the number
/// of [menus] in the [PlatformMenu] it belongs to.
///
/// The `getId` parameter is a [MenuItemSerializableIdGenerator] function that
/// generates a unique ID for each menu item, which is to be returned in the
Expand All @@ -152,6 +193,17 @@ abstract class MenuItem with Diagnosticable {
required MenuItemSerializableIdGenerator getId,
});

/// The optional shortcut that selects this [MenuItem].
///
/// This shortcut is only enabled when [onSelected] is set.
MenuSerializableShortcut? get shortcut => null;

/// Returns any child [MenuItem]s of this item.
///
/// Returns an empty list if this type of menu item doesn't have
/// children.
List<MenuItem> get menus => const <MenuItem>[];

/// Returns all descendant [MenuItem]s of this item.
///
/// Returns an empty list if this type of menu item doesn't have
Expand All @@ -163,9 +215,27 @@ abstract class MenuItem with Diagnosticable {
///
/// Only items that do not have submenus will have this callback invoked.
///
/// Only one of [onSelected] or [onSelectedIntent] may be specified.
///
/// If neither [onSelected] nor [onSelectedIntent] are specified, then this
/// menu item is considered to be disabled.
///
/// The default implementation returns null.
VoidCallback? get onSelected => null;

/// Returns an intent, if any, to be invoked if the platform receives a
/// "Menu.selectedCallback" method call from the platform for this item.
///
/// Only items that do not have submenus will have this intent invoked.
///
/// Only one of [onSelected] or [onSelectedIntent] may be specified.
///
/// If neither [onSelected] nor [onSelectedIntent] are specified, then this
/// menu item is considered to be disabled.
///
/// The default implementation returns null.
Intent? get onSelectedIntent => null;

/// Returns a callback, if any, to be invoked if the platform menu receives a
/// "Menu.opened" method call from the platform for this item.
///
Expand All @@ -181,6 +251,12 @@ abstract class MenuItem with Diagnosticable {
///
/// The default implementation returns null.
VoidCallback? get onClose => null;

/// Returns the list of group members if this menu item is a "grouping" menu
/// item, such as [PlatformMenuItemGroup].
///
/// Defaults to an empty list.
List<MenuItem> get members => const <MenuItem>[];
}

/// An abstract delegate class that can be used to set
Expand Down Expand Up @@ -383,7 +459,12 @@ class DefaultPlatformMenuDelegate extends PlatformMenuDelegate {
}
final MenuItem item = _idMap[id]!;
if (call.method == _kMenuSelectedCallbackMethod) {
assert(item.onSelected == null || item.onSelectedIntent == null,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we assert this earlier closer to the creation point of the MenuItem? Currently, you'd only see the assert if you actually selected the item in the menu

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see below that this is also asserted earlier. Never mind this comment then.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I did both because they're in separate classes. It's probably overkill.

'Only one of MenuItem.onSelected or MenuItem.onSelectedIntent may be specified');
item.onSelected?.call();
if (item.onSelectedIntent != null) {
Actions.maybeInvoke(FocusManager.instance.primaryFocus!.context!, item.onSelectedIntent!);
}
} else if (call.method == _kMenuItemOpenedMethod) {
item.onOpen?.call();
} else if (call.method == _kMenuItemClosedMethod) {
Expand All @@ -407,7 +488,7 @@ class DefaultPlatformMenuDelegate extends PlatformMenuDelegate {
/// the platform menu bar.
///
/// As far as Flutter is concerned, this widget has no visual representation,
/// and intercepts no events: it just returns the [body] from its build
/// and intercepts no events: it just returns the [child] from its build
/// function. This is because all of the rendering, shortcuts, and event
/// handling for the menu is handled by the plugin on the host platform.
///
Expand All @@ -429,18 +510,32 @@ class DefaultPlatformMenuDelegate extends PlatformMenuDelegate {
class PlatformMenuBar extends StatefulWidget with DiagnosticableTreeMixin {
/// Creates a const [PlatformMenuBar].
///
/// The [body] and [menus] attributes are required.
/// The [child] and [menus] attributes are required.
const PlatformMenuBar({
super.key,
required this.body,
required this.menus,
});
this.child,
@Deprecated(
'Use the child attribute instead. '
'This feature was deprecated after v3.1.0-0.0.pre.'
)
this.body,
}) : assert(body == null || child == null,
'The body argument is deprecated, and only one of body or child may be used.');

/// The widget below this widget in the tree.
///
/// {@macro flutter.widgets.ProxyWidget.child}
final Widget? child;

/// The widget to be rendered in the Flutter window that these platform menus
/// are associated with.
/// The widget below this widget in the tree.
///
/// This is typically the body of the application's UI.
final Widget body;
/// This attribute is deprecated, use [child] instead.
@Deprecated(
'Use the child attribute instead. '
'This feature was deprecated after v3.1.0-0.0.pre.'
)
final Widget? body;

/// The list of menu items that are the top level children of the
/// [PlatformMenuBar].
Expand Down Expand Up @@ -512,7 +607,7 @@ class _PlatformMenuBarState extends State<PlatformMenuBar> {
Widget build(BuildContext context) {
// PlatformMenuBar is really about managing the platform menu bar, and
// doesn't do any rendering or event handling in Flutter.
return widget.body;
return widget.child ?? widget.body ?? const SizedBox();
}
}

Expand Down Expand Up @@ -547,6 +642,7 @@ class PlatformMenu extends MenuItem with DiagnosticableTreeMixin {
/// The menu items in the submenu opened by this menu item.
///
/// If this is an empty list, this [PlatformMenu] will be disabled.
@override
final List<MenuItem> menus;

/// Returns all descendant [MenuItem]s of this item.
Expand Down Expand Up @@ -646,6 +742,7 @@ class PlatformMenuItemGroup extends MenuItem {
/// The [MenuItem]s that are members of this menu item group.
///
/// An assertion will be thrown if there isn't at least one member of the group.
@override
final List<MenuItem> members;

@override
Expand All @@ -654,19 +751,32 @@ class PlatformMenuItemGroup extends MenuItem {
required MenuItemSerializableIdGenerator getId,
}) {
assert(members.isNotEmpty, 'There must be at least one member in a PlatformMenuItemGroup');
return serialize(this, delegate, getId: getId);
}

/// Converts the supplied object to the correct channel representation for the
/// 'flutter/menu' channel.
///
/// This API is supplied so that implementers of [PlatformMenuItemGroup] can share
/// this implementation.
static Iterable<Map<String, Object?>> serialize(
MenuItem group,
PlatformMenuDelegate delegate, {
required MenuItemSerializableIdGenerator getId,
}) {
final List<Map<String, Object?>> result = <Map<String, Object?>>[];
result.add(<String, Object?>{
_kIdKey: getId(this),
_kIdKey: getId(group),
_kIsDividerKey: true,
});
for (final MenuItem item in members) {
for (final MenuItem item in group.members) {
result.addAll(item.toChannelRepresentation(
delegate,
getId: getId,
));
}
result.add(<String, Object?>{
_kIdKey: getId(this),
_kIdKey: getId(group),
_kIsDividerKey: true,
});
return result;
Expand Down Expand Up @@ -697,14 +807,16 @@ class PlatformMenuItem extends MenuItem {
required this.label,
this.shortcut,
this.onSelected,
});
this.onSelectedIntent,
}) : assert(onSelected == null || onSelectedIntent == null, 'Only one of onSelected or onSelectedIntent may be specified');

/// The required label used for rendering the menu item.
final String label;

/// The optional shortcut that selects this [PlatformMenuItem].
///
/// This shortcut is only enabled when [onSelected] is set.
@override
final MenuSerializableShortcut? shortcut;

/// An optional callback that is called when this [PlatformMenuItem] is
Expand All @@ -714,6 +826,13 @@ class PlatformMenuItem extends MenuItem {
@override
final VoidCallback? onSelected;

/// An optional intent that is invoked when this [PlatformMenuItem] is
/// selected.
///
/// If unset, this menu item will be disabled.
@override
final Intent? onSelectedIntent;

@override
Iterable<Map<String, Object?>> toChannelRepresentation(
PlatformMenuDelegate delegate, {
Expand Down Expand Up @@ -741,6 +860,9 @@ class PlatformMenuItem extends MenuItem {
};
}

@override
String toStringShort() => '${describeIdentity(this)}($label)';

@override
void debugFillProperties(DiagnosticPropertiesBuilder properties) {
super.debugFillProperties(properties);
Expand Down Expand Up @@ -779,9 +901,7 @@ class PlatformProvidedMenuItem extends PlatformMenuItem {
const PlatformProvidedMenuItem({
required this.type,
this.enabled = true,
}) : super(
label: '', // The label is ignored for standard menus.
);
}) : super(label: ''); // The label is ignored for platform provided menus.

/// The type of default menu this is.
///
Expand Down Expand Up @@ -832,7 +952,7 @@ class PlatformProvidedMenuItem extends PlatformMenuItem {
assert(() {
if (!hasMenu(type)) {
throw ArgumentError(
'Platform ${defaultTargetPlatform.name} has no standard menu for '
'Platform ${defaultTargetPlatform.name} has no platform provided menu for '
'$type. Call PlatformProvidedMenuItem.hasMenu to determine this before '
'instantiating one.',
);
Expand All @@ -856,7 +976,8 @@ class PlatformProvidedMenuItem extends PlatformMenuItem {
}
}

/// The list of possible standard, prebuilt menus for use in a [PlatformMenuBar].
/// The list of possible platform provided, prebuilt menus for use in a
/// [PlatformMenuBar].
///
/// These are menus that the platform typically provides that cannot be
/// reproduced in Flutter without calling platform functions, but are standard
Expand All @@ -870,7 +991,7 @@ class PlatformProvidedMenuItem extends PlatformMenuItem {
/// Add these to your [PlatformMenuBar] using the [PlatformProvidedMenuItem]
/// class.
///
/// You can tell if the platform supports the given standard menu using the
/// You can tell if the platform provides the given menu using the
/// [PlatformProvidedMenuItem.hasMenu] method.
// Must be kept in sync with the plugin code's enum of the same name.
enum PlatformProvidedMenuItemType {
Expand Down