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

Allow arrow keys to navigate MenuAnchor independently of global shortcut definition (#155728)

Browse Source 
This PR adjusts the implementation of handling navigational shortcuts (i.e. arrow keys) on `MenuAnchor` and `DropdownMenu`.

## Motivation
The direct outcome of this PR is to allow keyboard to enter submenus on Web: When the focus is on a `MenuAnchor` while the menu is open, pressing arrow keys should move the focus to the menu item.
* Before the PR, this works for all platforms but Web, a problem described in https://github.com/flutter/flutter/issues/119532#issuecomment-2274705565.

It is caused by the fact that `MenuAnchor` does not wrap itself with a `Shortcuts`, and therefore key events when the focus is on a `MenuAnchor` has been working only because the event falls back to the `Shortcuts` widget defined by `WidgetsApp`, whose default value happens to satisfy `MenuAnchor`'s needs - except on Web where arrow keys are defined to scroll instead of traverse. 

Instead of defining this problem as "just a patch for Web", I think it's better to define it as a problem of all platforms: `MenuAnchor`'s shortcuts should be independent of `WidgetsApp.shortcuts`. Because even if `WidgetsApp.shortcuts` is redefined as something else, people should probably still expect arrow keys to work on `MenuAnchor`.

Therefore this PR makes `MenuAnchor` produce a `Shortcuts` by itself.

### Dropdown menu
The fix above breaks `DropdownMenu`. `DropdownMenu` uses `MenuAnchor`, while defining its own shortcuts because, when filter is enabled:
* The left and right arrow keys need to move the text carets instead
* The up and down arrow keys need to "fake" directional navigation - the focus needs to stay on the text field, while some menu item is highlighted as if it is focused.

Before the PR, `DropdownMenu` defines these shortcuts out of `MenuAnchor`. In order for the `DropdownMenu`'s shortcuts to take priority, these shortcuts are moved to between `MenuAnchor` and the `Textfield`.

A test is added to verify that the left/right keys move text carets.

Below are psuedo-widget-trees after the PR:
```
MenuAnchor
 |- Shortcuts(arrows->DirectionalFocusIntent)
    |- MenuAnchor.child
    |- menu

DropdownMenu
 |- Actions(DirectionalFocusIntent->_dropdownMenuNavigation)
    |- MenuAnchor
        |- Shortcuts(arrows->DirectionalFocusIntent)
            |- Shortcuts(leftright->ExtendSelectionByCharacterIntent, updown->_dropdownMenuArrowIntent)
            |  |- TextField
            |     |- EditableText
            |        |- Actions(DirectionalFocusIntent->DirectionalFocusAction.forTextField)
            |- menu
```

## Known issues
After this PR, traversing the menu still have quite a few problems, which are left for other PRs.
This commit is contained in:
Tong Mu 2024-10-03 13:09:19 -07:00 committed by GitHub
parent 500285d39a
commit 59e57437db
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 149 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
packages/flutter/lib/src/material/dropdown_menu.dart
View File

@ -43,12 +43,6 @@ typedef FilterCallback<T> = List<DropdownMenuEntry<T>> Function(List<DropdownMen
/// Used by [DropdownMenu.searchCallback].
typedef SearchCallback<T> = int? Function(List<DropdownMenuEntry<T>> entries, String query);
// Navigation shortcuts to move the selected menu items up or down.
final Map<ShortcutActivator, Intent> _kMenuTraversalShortcuts = <ShortcutActivator, Intent> {
LogicalKeySet(LogicalKeyboardKey.arrowUp): const _ArrowUpIntent(),
LogicalKeySet(LogicalKeyboardKey.arrowDown): const _ArrowDownIntent(),
};
const double _kMinimumWidth = 112.0;
const double _kDefaultHorizontalPadding = 12.0;
@ -944,7 +938,14 @@ class _DropdownMenuState<T> extends State<DropdownMenu<T>> {
return textField;
}
return _DropdownMenuBody(
return Shortcuts(
shortcuts: const <ShortcutActivator, Intent>{
SingleActivator(LogicalKeyboardKey.arrowLeft): ExtendSelectionByCharacterIntent(forward: false, collapseSelection: true),
SingleActivator(LogicalKeyboardKey.arrowRight): ExtendSelectionByCharacterIntent(forward: true, collapseSelection: true),
SingleActivator(LogicalKeyboardKey.arrowUp): _ArrowUpIntent(),
SingleActivator(LogicalKeyboardKey.arrowDown): _ArrowDownIntent(),
},
child: _DropdownMenuBody(
width: widget.width,
children: <Widget>[
textField,
@ -952,6 +953,7 @@ class _DropdownMenuState<T> extends State<DropdownMenu<T>> {
trailingButton,
leadingButton,
],
),
);
},
);
@ -977,9 +979,7 @@ class _DropdownMenuState<T> extends State<DropdownMenu<T>> {
);
}
return Shortcuts(
shortcuts: _kMenuTraversalShortcuts,
child: Actions(
return Actions(
actions: <Type, Action<Intent>>{
_ArrowUpIntent: CallbackAction<_ArrowUpIntent>(
onInvoke: handleUpKeyInvoke,
@ -989,11 +989,17 @@ class _DropdownMenuState<T> extends State<DropdownMenu<T>> {
),
},
child: menuAnchor,
),
);
}
}
// `DropdownMenu` dispatches these private intents on arrow up/down keys.
// They are needed instead of the typical `DirectionalFocusIntent`s because
// `DropdownMenu` does not really navigate the focus tree upon arrow up/down
// keys: the focus stays on the text field and the menu items are given fake
// highlights as if they are focused. Using `DirectionalFocusIntent`s will cause
// the action to be processed by `EditableText`.
class _ArrowUpIntent extends Intent {
const _ArrowUpIntent();
}

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

@ -432,11 +432,22 @@ class _MenuAnchorState extends State<MenuAnchor> {
);
}
return _MenuAnchorScope(
// This `Shortcuts` is needed so that shortcuts work when the focus is on
// MenuAnchor (specifically, the root menu, since submenus have their own
// `Shortcuts`).
return
Shortcuts(
shortcuts: _kMenuTraversalShortcuts,
// Ignore semantics here and since the same information is typically
// also provided by the children.
includeSemantics: false,
child:
_MenuAnchorScope(
anchorKey: _anchorKey,
anchor: this,
isOpen: _isOpen,
child: child,
),
);
}

40
packages/flutter/test/material/dropdown_menu_test.dart
View File

@ -1221,6 +1221,46 @@ void main() {
expect(item5material.color, Colors.transparent); // the previous item should not be highlighted.
}, variant: TargetPlatformVariant.desktop());
testWidgets('Left and right keys can move text field selection', (WidgetTester tester) async {
final TextEditingController controller = TextEditingController();
addTearDown(controller.dispose);
final ThemeData themeData = ThemeData();
await tester.pumpWidget(MaterialApp(
theme: themeData,
home: Scaffold(
body: DropdownMenu<TestMenu>(
requestFocusOnTap: true,
enableFilter: true,
filterCallback: (List<DropdownMenuEntry<TestMenu>> entries, String filter) {
return entries.where((DropdownMenuEntry<TestMenu> element) => element.label.contains(filter)).toList();
},
dropdownMenuEntries: menuChildren,
controller: controller,
),
),
));
// Open the menu.
await tester.tap(find.byType(DropdownMenu<TestMenu>));
await tester.pump();
await tester.enterText(find.byType(TextField).first, 'example');
await tester.pumpAndSettle();
expect(controller.text, 'example');
expect(controller.selection, const TextSelection.collapsed(offset: 7));
// Press left key, the caret should move left.
await tester.sendKeyEvent(LogicalKeyboardKey.arrowLeft);
await tester.pumpAndSettle();
expect(controller.selection, const TextSelection.collapsed(offset: 6));
// Press Right key, the caret should move right.
await tester.sendKeyEvent(LogicalKeyboardKey.arrowRight);
await tester.pumpAndSettle();
expect(controller.selection, const TextSelection.collapsed(offset: 7));
}, variant: TargetPlatformVariant.desktop());
// Regression test for https://github.com/flutter/flutter/issues/147253.
testWidgets('Down key and up key can navigate on desktop platforms '
'when a label text contains another label text', (WidgetTester tester) async {

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

@ -2285,6 +2285,66 @@ void main() {
expect(opened, isEmpty);
expect(closed, isNotEmpty);
});
// Regression test for
// https://github.com/flutter/flutter/issues/119532#issuecomment-2274705565.
testWidgets('Shortcuts of MenuAnchor do not rely on WidgetsApp.shortcuts', (WidgetTester tester) async {
// MenuAnchor used to rely on WidgetsApp.shortcuts for menu navigation,
// which is a problem for Web because the Web uses a special set of
// default shortcuts that define arrow keys as scrolling instead of
// traversing, and therefore arrow keys won't enter submenus when the
// focus is on MenuAnchor.
//
// This test verifies that `MenuAnchor`'s shortcuts continues to work even
// when `WidgetsApp.shortcuts` contains nothing.
final FocusNode childNode = FocusNode(debugLabel: 'Dropdown Inkwell');
addTearDown(childNode.dispose);
await tester.pumpWidget(
MaterialApp(
// Clear WidgetsApp.shortcuts to make sure MenuAnchor doesn't rely on
// it.
shortcuts: const <ShortcutActivator, Intent>{},
home: Scaffold(
body: MenuAnchor(
childFocusNode: childNode,
menuChildren: List<Widget>.generate(3, (int i) =>
MenuItemButton(
child: Text('Submenu item $i'),
onPressed: () {},
)
),
builder: (BuildContext context, MenuController controller, Widget? child) {
return InkWell(
focusNode: childNode,
onTap: controller.open,
child: const Text('Main button'),
);
},
),
),
),
);
listenForFocusChanges();
// Open the drop down menu and focus on the MenuAnchor.
await tester.tap(find.text('Main button'));
await tester.pumpAndSettle();
expect(find.text('Submenu item 0'), findsOneWidget);
// Press arrowDown, and the first submenu button should be focused.
// This is the critical part. It used to not work on Web.
await tester.sendKeyEvent(LogicalKeyboardKey.arrowDown);
await tester.pump();
expect(focusedMenu, equals('MenuItemButton(Text("Submenu item 0"))'));
// Press arrowDown, and the second submenu button should be focused.
await tester.sendKeyEvent(LogicalKeyboardKey.arrowDown);
await tester.pump();
expect(focusedMenu, equals('MenuItemButton(Text("Submenu item 1"))'));
});
});
group('Accelerators', () {