firka/flutter
firka/flutter
4
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix Shortcut label for CharacterActivator does not include modifiers (#152233)

Browse Source 
## 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 https://github.com/flutter/flutter/issues/145040.

## Tests

Adds 1 test.
This commit is contained in:
Bruno Leroux 2024-07-30 09:03:00 +02:00 committed by GitHub
parent dfedcfe702
commit 85960d24b9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 71 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
packages/flutter/lib/src/material/menu_anchor.dart
View File

@ -2156,7 +2156,7 @@ class _LocalizedShortcutLabeler {
final LogicalKeyboardKey trigger = serialized.trigger!;
final List<String> modifiers = <String>[
if (_usesSymbolicModifiers) ...<String>[
// MacOS/iOS platform convention uses this ordering, with ⌘ always last.
// macOS/iOS platform convention uses this ordering, with ⌘ always last.
if (serialized.control!) _getModifierLabel(LogicalKeyboardKey.control, localizations),
if (serialized.alt!) _getModifierLabel(LogicalKeyboardKey.alt, localizations),
if (serialized.shift!) _getModifierLabel(LogicalKeyboardKey.shift, localizations),
@ -2190,7 +2190,24 @@ class _LocalizedShortcutLabeler {
if (shortcutTrigger != null && shortcutTrigger.isNotEmpty) shortcutTrigger,
].join(keySeparator);
} else if (serialized.character != null) {
return serialized.character!;
final List<String> modifiers = <String>[
// Character based shortcuts cannot check shifted keys.
if (_usesSymbolicModifiers) ...<String>[
// macOS/iOS platform convention uses this ordering, with ⌘ always last.
if (serialized.control!) _getModifierLabel(LogicalKeyboardKey.control, localizations),
if (serialized.alt!) _getModifierLabel(LogicalKeyboardKey.alt, localizations),
if (serialized.meta!) _getModifierLabel(LogicalKeyboardKey.meta, localizations),
] else ...<String>[
// This order matches the LogicalKeySet version.
if (serialized.alt!) _getModifierLabel(LogicalKeyboardKey.alt, localizations),
if (serialized.control!) _getModifierLabel(LogicalKeyboardKey.control, localizations),
if (serialized.meta!) _getModifierLabel(LogicalKeyboardKey.meta, localizations),
],
];
return <String>[
...modifiers,
serialized.character!,
].join(keySeparator);
}
throw UnimplementedError('Shortcut labels for ShortcutActivators that do not implement '
'MenuSerializableShortcut (e.g. ShortcutActivators other than SingleActivator or '

52
packages/flutter/test/material/menu_anchor_test.dart
View File

@ -2201,6 +2201,58 @@ void main() {
skip: kIsWeb && !isCanvasKit, // https://github.com/flutter/flutter/issues/145527
);
// Regression test for https://github.com/flutter/flutter/issues/145040.
testWidgets('CharacterActivator shortcut mnemonics include modifiers', (WidgetTester tester) async {
await tester.pumpWidget(
MaterialApp(
home: Material(
child: MenuBar(
controller: controller,
children: createTestMenus(
shortcuts: <TestMenu, MenuSerializableShortcut>{
TestMenu.subSubMenu110: const CharacterActivator('A', control: true),
TestMenu.subSubMenu111: const CharacterActivator('B', alt: true),
TestMenu.subSubMenu112: const CharacterActivator('C', meta: true),
},
),
),
),
),
);
// Open a menu initially.
await tester.tap(find.text(TestMenu.mainMenu1.label));
await tester.pump();
await tester.tap(find.text(TestMenu.subMenu11.label));
await tester.pump();
final Text mnemonic0 = tester.widget(findMnemonic(TestMenu.subSubMenu110.label));
final Text mnemonic1 = tester.widget(findMnemonic(TestMenu.subSubMenu111.label));
final Text mnemonic2 = tester.widget(findMnemonic(TestMenu.subSubMenu112.label));
switch (defaultTargetPlatform) {
case TargetPlatform.android:
case TargetPlatform.fuchsia:
case TargetPlatform.linux:
expect(mnemonic0.data, equals('Ctrl+A'));
expect(mnemonic1.data, equals('Alt+B'));
expect(mnemonic2.data, equals('Meta+C'));
case TargetPlatform.windows:
expect(mnemonic0.data, equals('Ctrl+A'));
expect(mnemonic1.data, equals('Alt+B'));
expect(mnemonic2.data, equals('Win+C'));
case TargetPlatform.iOS:
case TargetPlatform.macOS:
expect(mnemonic0.data, equals('⌃ A'));
expect(mnemonic1.data, equals('⌥ B'));
expect(mnemonic2.data, equals('⌘ C'));
}
},
variant: TargetPlatformVariant.all(),
skip: kIsWeb && !isCanvasKit, // https://github.com/flutter/flutter/issues/145527
);
testWidgets('leadingIcon is used when set', (WidgetTester tester) async {
await tester.pumpWidget(
MaterialApp(