Reorders menu item button shortcuts on Mac-like platforms (#129309)

The Apple Human Interface Guidelines give a specific ordering of the symbols
used as modifier keys in menu shortcuts.  These guidelines are encoded into
the native Cocoa or UIKit menu classes, and are intended to ensure that symbols
are always aligned into columns of like symbols and do not move around in the
case of dynamic menu items (for example, holding Option will transform certain
menu items into different versions that take the Option key, so the Option key
symbol appears to the left of most other symbols to avoid reordering).

The Material spec says to use symbols for modifier keys on Mac and iOS, as this
is what users there are familiar with.  It stands to reason that we should
follow the platform guidelines fully, so this changes the MenuItemButton class
to honor the HIG-compliant symbol ordering on Mac and iOS.

Fixed: #129308

packages/flutter/lib/src/material/menu_anchor.dart

@@ -1072,7 +1072,7 @@ class _MenuItemButtonState extends State<MenuItemButton> {
       ),
     );
 
-    if (_platformSupportsAccelerators() && widget.enabled) {
+    if (_platformSupportsAccelerators && widget.enabled) {
       child = MenuAcceleratorCallbackBinding(
         onInvoke: _handleSelect,
         child: child,
@@ -1920,7 +1920,7 @@ class _SubmenuButtonState extends State<SubmenuButton> {
           ),
         );
 
-        if (_enabled && _platformSupportsAccelerators()) {
+        if (_enabled && _platformSupportsAccelerators) {
           return MenuAcceleratorCallbackBinding(
             onInvoke: () => toggleShowMenu(context),
             hasSubmenu: true,
@@ -2049,33 +2049,44 @@ class _LocalizedShortcutLabeler {
   String getShortcutLabel(MenuSerializableShortcut shortcut, MaterialLocalizations localizations) {
     final ShortcutSerialization serialized = shortcut.serializeForMenu();
     final String keySeparator;
-    switch (defaultTargetPlatform) {
-      case TargetPlatform.iOS:
-      case TargetPlatform.macOS:
+    if (_usesSymbolicModifiers) {
         // Use "⌃ ⇧ A" style on macOS and iOS.
         keySeparator = ' ';
-      case TargetPlatform.android:
-      case TargetPlatform.fuchsia:
-      case TargetPlatform.linux:
-      case TargetPlatform.windows:
+    } else {
         // Use "Ctrl+Shift+A" style.
         keySeparator = '+';
     }
     if (serialized.trigger != null) {
       final List<String> modifiers = <String>[];
       final LogicalKeyboardKey trigger = serialized.trigger!;
-      // These should be in this order, to match the LogicalKeySet version.
-      if (serialized.alt!) {
-        modifiers.add(_getModifierLabel(LogicalKeyboardKey.alt, localizations));
-      }
-      if (serialized.control!) {
-        modifiers.add(_getModifierLabel(LogicalKeyboardKey.control, localizations));
-      }
-      if (serialized.meta!) {
-        modifiers.add(_getModifierLabel(LogicalKeyboardKey.meta, localizations));
-      }
-      if (serialized.shift!) {
-        modifiers.add(_getModifierLabel(LogicalKeyboardKey.shift, localizations));
+      if (_usesSymbolicModifiers) {
+        // macOS/iOS platform convention uses this ordering, with ⌘ always last.
+        if (serialized.control!) {
+          modifiers.add(_getModifierLabel(LogicalKeyboardKey.control, localizations));
+        }
+        if (serialized.alt!) {
+          modifiers.add(_getModifierLabel(LogicalKeyboardKey.alt, localizations));
+        }
+        if (serialized.shift!) {
+          modifiers.add(_getModifierLabel(LogicalKeyboardKey.shift, localizations));
+        }
+        if (serialized.meta!) {
+          modifiers.add(_getModifierLabel(LogicalKeyboardKey.meta, localizations));
+        }
+      } else {
+        // These should be in this order, to match the LogicalKeySet version.
+        if (serialized.alt!) {
+          modifiers.add(_getModifierLabel(LogicalKeyboardKey.alt, localizations));
+        }
+        if (serialized.control!) {
+          modifiers.add(_getModifierLabel(LogicalKeyboardKey.control, localizations));
+        }
+        if (serialized.meta!) {
+          modifiers.add(_getModifierLabel(LogicalKeyboardKey.meta, localizations));
+        }
+        if (serialized.shift!) {
+          modifiers.add(_getModifierLabel(LogicalKeyboardKey.shift, localizations));
+        }
       }
       String? shortcutTrigger;
       final int logicalKeyId = trigger.keyId;
@@ -2848,7 +2859,7 @@ class _MenuAcceleratorLabelState extends State<MenuAcceleratorLabel> {
   @override
   void initState() {
     super.initState();
-    if (_platformSupportsAccelerators()) {
+    if (_platformSupportsAccelerators) {
       _showAccelerators = _altIsPressed();
       HardwareKeyboard.instance.addHandler(_handleKeyEvent);
     }
@@ -2857,9 +2868,9 @@ class _MenuAcceleratorLabelState extends State<MenuAcceleratorLabel> {
 
   @override
   void dispose() {
-    assert(_platformSupportsAccelerators() || _shortcutRegistryEntry == null);
+    assert(_platformSupportsAccelerators || _shortcutRegistryEntry == null);
     _displayLabel = '';
-    if (_platformSupportsAccelerators()) {
+    if (_platformSupportsAccelerators) {
       _shortcutRegistryEntry?.dispose();
       _shortcutRegistryEntry = null;
       _shortcutRegistry = null;
@@ -2872,7 +2883,7 @@ class _MenuAcceleratorLabelState extends State<MenuAcceleratorLabel> {
   @override
   void didChangeDependencies() {
     super.didChangeDependencies();
-    if (!_platformSupportsAccelerators()) {
+    if (!_platformSupportsAccelerators) {
       return;
     }
     _binding = MenuAcceleratorCallbackBinding.maybeOf(context);
@@ -2900,7 +2911,7 @@ class _MenuAcceleratorLabelState extends State<MenuAcceleratorLabel> {
   }
 
   bool _handleKeyEvent(KeyEvent event) {
-    assert(_platformSupportsAccelerators());
+    assert(_platformSupportsAccelerators);
     final bool altIsPressed = _altIsPressed();
     if (altIsPressed != _showAccelerators) {
       setState(() {
@@ -2913,7 +2924,7 @@ class _MenuAcceleratorLabelState extends State<MenuAcceleratorLabel> {
   }
 
   void _updateAcceleratorShortcut() {
-    assert(_platformSupportsAccelerators());
+    assert(_platformSupportsAccelerators);
     _shortcutRegistryEntry?.dispose();
     _shortcutRegistryEntry = null;
     // Before registering an accelerator as a shortcut it should meet these
@@ -3566,23 +3577,38 @@ bool _debugMenuInfo(String message, [Iterable<String>? details]) {
   return true;
 }
 
-bool _platformSupportsAccelerators() {
+/// Whether [defaultTargetPlatform] is an Apple platform (Mac or iOS).
+bool get _isApple {
   switch (defaultTargetPlatform) {
+    case TargetPlatform.iOS:
+    case TargetPlatform.macOS:
+      return true;
     case TargetPlatform.android:
     case TargetPlatform.fuchsia:
     case TargetPlatform.linux:
     case TargetPlatform.windows:
-      return true;
-    case TargetPlatform.iOS:
-    case TargetPlatform.macOS:
-      // On iOS and macOS, pressing the Option key (a.k.a. the Alt key) causes a
-      // different set of characters to be generated, and the native menus don't
-      // support accelerators anyhow, so we just disable accelerators on these
-      // platforms.
       return false;
   }
 }
 
+/// Whether [defaultTargetPlatform] is one that uses symbolic shortcuts.
+///
+/// Mac and iOS use special symbols for modifier keys instead of their names,
+/// render them in a particular order defined by Apple's human interface
+/// guidelines, and format them so that the modifier keys always align.
+bool get _usesSymbolicModifiers {
+  return _isApple;
+}
+
+
+bool get _platformSupportsAccelerators {
+  // On iOS and macOS, pressing the Option key (a.k.a. the Alt key) causes a
+  // different set of characters to be generated, and the native menus don't
+  // support accelerators anyhow, so we just disable accelerators on these
+  // platforms.
+  return !_isApple;
+}
+
 // BEGIN GENERATED TOKEN PROPERTIES - Menu
 
 // Do not edit by hand. The code between the "BEGIN GENERATED" and

packages/flutter/test/material/menu_anchor_test.dart

@@ -2944,7 +2944,17 @@ void main() {
         shift: true,
         alt: true,
       );
-      final String allExpected = <String>[expectedAlt, expectedCtrl, expectedMeta, expectedShift, 'A'].join(expectedSeparator);
+      late String allExpected;
+      switch (defaultTargetPlatform) {
+        case TargetPlatform.android:
+        case TargetPlatform.fuchsia:
+        case TargetPlatform.linux:
+        case TargetPlatform.windows:
+          allExpected = <String>[expectedAlt, expectedCtrl, expectedMeta, expectedShift, 'A'].join(expectedSeparator);
+        case TargetPlatform.iOS:
+        case TargetPlatform.macOS:
+          allExpected = <String>[expectedCtrl, expectedAlt, expectedShift, expectedMeta, 'A'].join(expectedSeparator);
+      }
       const CharacterActivator charShortcuts = CharacterActivator('ñ');
       const String charExpected = 'ñ';
       await tester.pumpWidget(