Skip to content

Commit 85960d2

Browse files
authored
Fix Shortcut label for CharacterActivator does not include modifiers (flutter#152233)
## Description This PR fixes the shortcut label for CharacterActivator with modifiers keys. **Before**: ![image](https://github.com/user-attachments/assets/1cb8defe-2600-45ef-878d-fbdde5aaf2ad) **After**: ![image](https://github.com/user-attachments/assets/cd3b1c79-2f23-40fd-b0b9-934fa182fff3) ## Related Issue Fixes flutter#145040. ## Tests Adds 1 test.
1 parent dfedcfe commit 85960d2

File tree

2 files changed

+71
-2
lines changed

2 files changed

+71
-2
lines changed

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

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2156,7 +2156,7 @@ class _LocalizedShortcutLabeler {
21562156
final LogicalKeyboardKey trigger = serialized.trigger!;
21572157
final List<String> modifiers = <String>[
21582158
if (_usesSymbolicModifiers) ...<String>[
2159-
// MacOS/iOS platform convention uses this ordering, with ⌘ always last.
2159+
// macOS/iOS platform convention uses this ordering, with ⌘ always last.
21602160
if (serialized.control!) _getModifierLabel(LogicalKeyboardKey.control, localizations),
21612161
if (serialized.alt!) _getModifierLabel(LogicalKeyboardKey.alt, localizations),
21622162
if (serialized.shift!) _getModifierLabel(LogicalKeyboardKey.shift, localizations),
@@ -2190,7 +2190,24 @@ class _LocalizedShortcutLabeler {
21902190
if (shortcutTrigger != null && shortcutTrigger.isNotEmpty) shortcutTrigger,
21912191
].join(keySeparator);
21922192
} else if (serialized.character != null) {
2193-
return serialized.character!;
2193+
final List<String> modifiers = <String>[
2194+
// Character based shortcuts cannot check shifted keys.
2195+
if (_usesSymbolicModifiers) ...<String>[
2196+
// macOS/iOS platform convention uses this ordering, with ⌘ always last.
2197+
if (serialized.control!) _getModifierLabel(LogicalKeyboardKey.control, localizations),
2198+
if (serialized.alt!) _getModifierLabel(LogicalKeyboardKey.alt, localizations),
2199+
if (serialized.meta!) _getModifierLabel(LogicalKeyboardKey.meta, localizations),
2200+
] else ...<String>[
2201+
// This order matches the LogicalKeySet version.
2202+
if (serialized.alt!) _getModifierLabel(LogicalKeyboardKey.alt, localizations),
2203+
if (serialized.control!) _getModifierLabel(LogicalKeyboardKey.control, localizations),
2204+
if (serialized.meta!) _getModifierLabel(LogicalKeyboardKey.meta, localizations),
2205+
],
2206+
];
2207+
return <String>[
2208+
...modifiers,
2209+
serialized.character!,
2210+
].join(keySeparator);
21942211
}
21952212
throw UnimplementedError('Shortcut labels for ShortcutActivators that do not implement '
21962213
'MenuSerializableShortcut (e.g. ShortcutActivators other than SingleActivator or '

packages/flutter/test/material/menu_anchor_test.dart

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2201,6 +2201,58 @@ void main() {
22012201
skip: kIsWeb && !isCanvasKit, // https://github.com/flutter/flutter/issues/145527
22022202
);
22032203

2204+
// Regression test for https://github.com/flutter/flutter/issues/145040.
2205+
testWidgets('CharacterActivator shortcut mnemonics include modifiers', (WidgetTester tester) async {
2206+
await tester.pumpWidget(
2207+
MaterialApp(
2208+
home: Material(
2209+
child: MenuBar(
2210+
controller: controller,
2211+
children: createTestMenus(
2212+
shortcuts: <TestMenu, MenuSerializableShortcut>{
2213+
TestMenu.subSubMenu110: const CharacterActivator('A', control: true),
2214+
TestMenu.subSubMenu111: const CharacterActivator('B', alt: true),
2215+
TestMenu.subSubMenu112: const CharacterActivator('C', meta: true),
2216+
},
2217+
),
2218+
),
2219+
),
2220+
),
2221+
);
2222+
2223+
// Open a menu initially.
2224+
await tester.tap(find.text(TestMenu.mainMenu1.label));
2225+
await tester.pump();
2226+
2227+
await tester.tap(find.text(TestMenu.subMenu11.label));
2228+
await tester.pump();
2229+
2230+
final Text mnemonic0 = tester.widget(findMnemonic(TestMenu.subSubMenu110.label));
2231+
final Text mnemonic1 = tester.widget(findMnemonic(TestMenu.subSubMenu111.label));
2232+
final Text mnemonic2 = tester.widget(findMnemonic(TestMenu.subSubMenu112.label));
2233+
2234+
switch (defaultTargetPlatform) {
2235+
case TargetPlatform.android:
2236+
case TargetPlatform.fuchsia:
2237+
case TargetPlatform.linux:
2238+
expect(mnemonic0.data, equals('Ctrl+A'));
2239+
expect(mnemonic1.data, equals('Alt+B'));
2240+
expect(mnemonic2.data, equals('Meta+C'));
2241+
case TargetPlatform.windows:
2242+
expect(mnemonic0.data, equals('Ctrl+A'));
2243+
expect(mnemonic1.data, equals('Alt+B'));
2244+
expect(mnemonic2.data, equals('Win+C'));
2245+
case TargetPlatform.iOS:
2246+
case TargetPlatform.macOS:
2247+
expect(mnemonic0.data, equals('⌃ A'));
2248+
expect(mnemonic1.data, equals('⌥ B'));
2249+
expect(mnemonic2.data, equals('⌘ C'));
2250+
}
2251+
},
2252+
variant: TargetPlatformVariant.all(),
2253+
skip: kIsWeb && !isCanvasKit, // https://github.com/flutter/flutter/issues/145527
2254+
);
2255+
22042256
testWidgets('leadingIcon is used when set', (WidgetTester tester) async {
22052257
await tester.pumpWidget(
22062258
MaterialApp(

0 commit comments

Comments
 (0)