From e1e4ee9a016e00bc4905c15e2e4626883ca2e756 Mon Sep 17 00:00:00 2001 From: yim Date: Thu, 5 Dec 2024 02:08:52 +0800 Subject: [PATCH] Fix `DropdownMenu` focus (#156412) Fixes #143505 I am unable to test sending `TextInputClient.performAction` when the enter key is pressed on desktop platforms. I have created an issue for this: #156414. The current test approach is to check whether the `DropdownMenu` has focus. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [ ] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md --------- Co-authored-by: Bruno Leroux --- .../lib/src/material/dropdown_menu.dart | 69 ++++++++--- .../test/material/dropdown_menu_test.dart | 108 ++++++++++++++++-- 2 files changed, 147 insertions(+), 30 deletions(-) diff --git a/packages/flutter/lib/src/material/dropdown_menu.dart b/packages/flutter/lib/src/material/dropdown_menu.dart index ab16bc12bf..e078e057a6 100644 --- a/packages/flutter/lib/src/material/dropdown_menu.dart +++ b/packages/flutter/lib/src/material/dropdown_menu.dart @@ -518,6 +518,7 @@ class _DropdownMenuState extends State> { double? leadingPadding; bool _menuHasEnabledItem = false; TextEditingController? _localTextEditingController; + final FocusNode _internalFocudeNode = FocusNode(); TextEditingValue get _initialTextEditingValue { for (final DropdownMenuEntry entry in filteredEntries) { @@ -554,6 +555,7 @@ class _DropdownMenuState extends State> { _localTextEditingController?.dispose(); _localTextEditingController = null; } + _internalFocudeNode.dispose(); super.dispose(); } @@ -832,10 +834,32 @@ class _DropdownMenuState extends State> { _enableFilter = false; } controller.open(); + _internalFocudeNode.requestFocus(); } setState(() {}); } + void _handleEditingComplete() { + if (currentHighlight != null) { + final DropdownMenuEntry entry = filteredEntries[currentHighlight!]; + if (entry.enabled) { + _localTextEditingController?.value = TextEditingValue( + text: entry.label, + selection: TextSelection.collapsed(offset: entry.label.length), + ); + widget.onSelected?.call(entry.value); + } + } else { + if (_controller.isOpen) { + widget.onSelected?.call(null); + } + } + if (!widget.enableSearch) { + currentHighlight = null; + } + _controller.close(); + } + @override Widget build(BuildContext context) { final TextDirection textDirection = Directionality.of(context); @@ -934,24 +958,7 @@ class _DropdownMenuState extends State> { textAlignVertical: TextAlignVertical.center, style: effectiveTextStyle, controller: _localTextEditingController, - onEditingComplete: () { - if (currentHighlight != null) { - final DropdownMenuEntry entry = filteredEntries[currentHighlight!]; - if (entry.enabled) { - _localTextEditingController?.value = TextEditingValue( - text: entry.label, - selection: TextSelection.collapsed(offset: entry.label.length), - ); - widget.onSelected?.call(entry.value); - } - } else { - widget.onSelected?.call(null); - } - if (!widget.enableSearch) { - currentHighlight = null; - } - controller.close(); - }, + onEditingComplete: _handleEditingComplete, onTap: !widget.enabled ? null : () { handlePressed(controller); }, @@ -1041,8 +1048,28 @@ class _DropdownMenuState extends State> { _ArrowDownIntent: CallbackAction<_ArrowDownIntent>( onInvoke: handleDownKeyInvoke, ), + _EnterIntent: CallbackAction<_EnterIntent>( + onInvoke: (_) => _handleEditingComplete(), + ), }, - child: menuAnchor, + child: Stack( + children: [ + // Handling keyboard navigation when the Textfield has no focus. + Shortcuts( + shortcuts: const { + SingleActivator(LogicalKeyboardKey.arrowUp): _ArrowUpIntent(), + SingleActivator(LogicalKeyboardKey.arrowDown): _ArrowDownIntent(), + SingleActivator(LogicalKeyboardKey.enter): _EnterIntent(), + }, + child: Focus( + focusNode: _internalFocudeNode, + skipTraversal: true, + child: const SizedBox.shrink(), + ), + ), + menuAnchor, + ], + ), ); } } @@ -1062,6 +1089,10 @@ class _ArrowDownIntent extends Intent { const _ArrowDownIntent(); } +class _EnterIntent extends Intent { + const _EnterIntent(); +} + class _DropdownMenuBody extends MultiChildRenderObjectWidget { const _DropdownMenuBody({ super.children, diff --git a/packages/flutter/test/material/dropdown_menu_test.dart b/packages/flutter/test/material/dropdown_menu_test.dart index 33c0a6db7d..a609fea039 100644 --- a/packages/flutter/test/material/dropdown_menu_test.dart +++ b/packages/flutter/test/material/dropdown_menu_test.dart @@ -4,6 +4,7 @@ import 'dart:ui'; +import 'package:flutter/foundation.dart'; import 'package:flutter/material.dart'; import 'package:flutter/rendering.dart'; import 'package:flutter/services.dart'; @@ -1874,8 +1875,7 @@ void main() { await tester.pumpAndSettle(); expect(menuAnchor.controller!.isOpen, true); - // Simulate `TextInputAction.done` on textfield - await tester.testTextInput.receiveAction(TextInputAction.done); + await tester.sendKeyEvent(LogicalKeyboardKey.enter); await tester.pumpAndSettle(); expect(menuAnchor.controller!.isOpen, false); }); @@ -1915,31 +1915,29 @@ void main() { // Open the menu await tester.tap(find.byType(DropdownMenu)); await tester.pump(); - final bool isMobile = switch (themeData.platform) { TargetPlatform.android || TargetPlatform.iOS || TargetPlatform.fuchsia => true, TargetPlatform.macOS || TargetPlatform.linux || TargetPlatform.windows => false, }; - int expectedCount = isMobile ? 0 : 1; + int expectedCount = 1; // Test onSelected on key press await simulateKeyDownEvent(LogicalKeyboardKey.arrowDown); await tester.pumpAndSettle(); - await tester.testTextInput.receiveAction(TextInputAction.done); + await tester.sendKeyEvent(LogicalKeyboardKey.enter); await tester.pumpAndSettle(); expect(selectionCount, expectedCount); - // The desktop platform closed the menu when a completion action is pressed. So we need to reopen it. - if (!isMobile) { - await tester.tap(find.byType(DropdownMenu)); - await tester.pump(); - } + + // Open the menu + await tester.tap(find.byType(DropdownMenu)); + await tester.pump(); // Disabled item doesn't trigger onSelected callback. final Finder item1 = findMenuItemButton('Item 1'); await tester.tap(item1); await tester.pumpAndSettle(); - expect(controller.text, isMobile ? '' : 'Item 0'); + expect(controller.text, 'Item 0'); expect(selectionCount, expectedCount); final Finder item2 = findMenuItemButton('Item 2'); @@ -3576,6 +3574,94 @@ void main() { expect(tester.getRect(findMenuMaterial()).top, textFieldBottom); }); }); + + // Regression test for https://github.com/flutter/flutter/issues/143505. + testWidgets('Using keyboard navigation to select', (WidgetTester tester) async { + final FocusNode focusNode = FocusNode(); + addTearDown(focusNode.dispose); + TestMenu? selectedMenu; + await tester.pumpWidget( + MaterialApp( + home: Material( + child: Center( + child: DropdownMenu( + focusNode: focusNode, + dropdownMenuEntries: menuChildren, + onSelected: (TestMenu? menu) { + selectedMenu = menu; + }, + ), + ), + ), + ), + ); + // Pressing the tab key 3 times moves the focus to the icon button. + for (int i = 0; i < 3; i++) { + await tester.sendKeyEvent(LogicalKeyboardKey.tab); + await tester.pump(); + } + + // Now the focus is on the icon button. + final Element iconButton = tester.firstElement(find.byIcon(Icons.arrow_drop_down)); + expect(Focus.of(iconButton).hasPrimaryFocus, isTrue); + + await tester.sendKeyEvent(LogicalKeyboardKey.enter); + await tester.pump(); + + await tester.sendKeyEvent(LogicalKeyboardKey.arrowDown); + await tester.pump(); + + await tester.sendKeyEvent(LogicalKeyboardKey.enter); + await tester.pump(); + + expect(selectedMenu, TestMenu.mainMenu0); + }, variant: TargetPlatformVariant.all()); + + // Regression test for https://github.com/flutter/flutter/issues/143505. + testWidgets('Using keyboard navigation to select and without setting the FocusNode parameter', + (WidgetTester tester) async { + TestMenu? selectedMenu; + await tester.pumpWidget( + MaterialApp( + home: Material( + child: Center( + child: DropdownMenu( + dropdownMenuEntries: menuChildren, + onSelected: (TestMenu? menu) { + selectedMenu = menu; + }, + ), + ), + ), + ), + ); + // If there is no `FocusNode`, by default, `TextField` can receive focus + // on desktop platforms, but not on mobile platforms. Therefore, on desktop + // platforms, it takes 3 tabs to reach the icon button. + final int tabCount = switch (defaultTargetPlatform) { + TargetPlatform.iOS || TargetPlatform.android || TargetPlatform.fuchsia => 2, + TargetPlatform.macOS || TargetPlatform.linux || TargetPlatform.windows => 3, + }; + for (int i = 0; i < tabCount; i++) { + await tester.sendKeyEvent(LogicalKeyboardKey.tab); + await tester.pump(); + } + + // Now the focus is on the icon button. + final Element iconButton = tester.firstElement(find.byIcon(Icons.arrow_drop_down)); + expect(Focus.of(iconButton).hasPrimaryFocus, isTrue); + + await tester.sendKeyEvent(LogicalKeyboardKey.enter); + await tester.pump(); + + await tester.sendKeyEvent(LogicalKeyboardKey.arrowDown); + await tester.pump(); + + await tester.sendKeyEvent(LogicalKeyboardKey.enter); + await tester.pump(); + + expect(selectedMenu, TestMenu.mainMenu0); + }, variant: TargetPlatformVariant.all()); } enum TestMenu {