Skip to content

Commit e1e4ee9

Browse files
yiiimbleroux
andauthored
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]. <!-- Links --> [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 <bruno.leroux@gmail.com>
1 parent eabed23 commit e1e4ee9

File tree

2 files changed

+147
-30
lines changed

2 files changed

+147
-30
lines changed

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

Lines changed: 50 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -518,6 +518,7 @@ class _DropdownMenuState<T> extends State<DropdownMenu<T>> {
518518
double? leadingPadding;
519519
bool _menuHasEnabledItem = false;
520520
TextEditingController? _localTextEditingController;
521+
final FocusNode _internalFocudeNode = FocusNode();
521522

522523
TextEditingValue get _initialTextEditingValue {
523524
for (final DropdownMenuEntry<T> entry in filteredEntries) {
@@ -554,6 +555,7 @@ class _DropdownMenuState<T> extends State<DropdownMenu<T>> {
554555
_localTextEditingController?.dispose();
555556
_localTextEditingController = null;
556557
}
558+
_internalFocudeNode.dispose();
557559
super.dispose();
558560
}
559561

@@ -832,10 +834,32 @@ class _DropdownMenuState<T> extends State<DropdownMenu<T>> {
832834
_enableFilter = false;
833835
}
834836
controller.open();
837+
_internalFocudeNode.requestFocus();
835838
}
836839
setState(() {});
837840
}
838841

842+
void _handleEditingComplete() {
843+
if (currentHighlight != null) {
844+
final DropdownMenuEntry<T> entry = filteredEntries[currentHighlight!];
845+
if (entry.enabled) {
846+
_localTextEditingController?.value = TextEditingValue(
847+
text: entry.label,
848+
selection: TextSelection.collapsed(offset: entry.label.length),
849+
);
850+
widget.onSelected?.call(entry.value);
851+
}
852+
} else {
853+
if (_controller.isOpen) {
854+
widget.onSelected?.call(null);
855+
}
856+
}
857+
if (!widget.enableSearch) {
858+
currentHighlight = null;
859+
}
860+
_controller.close();
861+
}
862+
839863
@override
840864
Widget build(BuildContext context) {
841865
final TextDirection textDirection = Directionality.of(context);
@@ -934,24 +958,7 @@ class _DropdownMenuState<T> extends State<DropdownMenu<T>> {
934958
textAlignVertical: TextAlignVertical.center,
935959
style: effectiveTextStyle,
936960
controller: _localTextEditingController,
937-
onEditingComplete: () {
938-
if (currentHighlight != null) {
939-
final DropdownMenuEntry<T> entry = filteredEntries[currentHighlight!];
940-
if (entry.enabled) {
941-
_localTextEditingController?.value = TextEditingValue(
942-
text: entry.label,
943-
selection: TextSelection.collapsed(offset: entry.label.length),
944-
);
945-
widget.onSelected?.call(entry.value);
946-
}
947-
} else {
948-
widget.onSelected?.call(null);
949-
}
950-
if (!widget.enableSearch) {
951-
currentHighlight = null;
952-
}
953-
controller.close();
954-
},
961+
onEditingComplete: _handleEditingComplete,
955962
onTap: !widget.enabled ? null : () {
956963
handlePressed(controller);
957964
},
@@ -1041,8 +1048,28 @@ class _DropdownMenuState<T> extends State<DropdownMenu<T>> {
10411048
_ArrowDownIntent: CallbackAction<_ArrowDownIntent>(
10421049
onInvoke: handleDownKeyInvoke,
10431050
),
1051+
_EnterIntent: CallbackAction<_EnterIntent>(
1052+
onInvoke: (_) => _handleEditingComplete(),
1053+
),
10441054
},
1045-
child: menuAnchor,
1055+
child: Stack(
1056+
children: <Widget>[
1057+
// Handling keyboard navigation when the Textfield has no focus.
1058+
Shortcuts(
1059+
shortcuts: const <ShortcutActivator, Intent>{
1060+
SingleActivator(LogicalKeyboardKey.arrowUp): _ArrowUpIntent(),
1061+
SingleActivator(LogicalKeyboardKey.arrowDown): _ArrowDownIntent(),
1062+
SingleActivator(LogicalKeyboardKey.enter): _EnterIntent(),
1063+
},
1064+
child: Focus(
1065+
focusNode: _internalFocudeNode,
1066+
skipTraversal: true,
1067+
child: const SizedBox.shrink(),
1068+
),
1069+
),
1070+
menuAnchor,
1071+
],
1072+
),
10461073
);
10471074
}
10481075
}
@@ -1062,6 +1089,10 @@ class _ArrowDownIntent extends Intent {
10621089
const _ArrowDownIntent();
10631090
}
10641091

1092+
class _EnterIntent extends Intent {
1093+
const _EnterIntent();
1094+
}
1095+
10651096
class _DropdownMenuBody extends MultiChildRenderObjectWidget {
10661097
const _DropdownMenuBody({
10671098
super.children,

packages/flutter/test/material/dropdown_menu_test.dart

Lines changed: 97 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import 'dart:ui';
66

7+
import 'package:flutter/foundation.dart';
78
import 'package:flutter/material.dart';
89
import 'package:flutter/rendering.dart';
910
import 'package:flutter/services.dart';
@@ -1874,8 +1875,7 @@ void main() {
18741875
await tester.pumpAndSettle();
18751876
expect(menuAnchor.controller!.isOpen, true);
18761877

1877-
// Simulate `TextInputAction.done` on textfield
1878-
await tester.testTextInput.receiveAction(TextInputAction.done);
1878+
await tester.sendKeyEvent(LogicalKeyboardKey.enter);
18791879
await tester.pumpAndSettle();
18801880
expect(menuAnchor.controller!.isOpen, false);
18811881
});
@@ -1915,31 +1915,29 @@ void main() {
19151915
// Open the menu
19161916
await tester.tap(find.byType(DropdownMenu<TestMenu>));
19171917
await tester.pump();
1918-
19191918
final bool isMobile = switch (themeData.platform) {
19201919
TargetPlatform.android || TargetPlatform.iOS || TargetPlatform.fuchsia => true,
19211920
TargetPlatform.macOS || TargetPlatform.linux || TargetPlatform.windows => false,
19221921
};
1923-
int expectedCount = isMobile ? 0 : 1;
1922+
int expectedCount = 1;
19241923

19251924
// Test onSelected on key press
19261925
await simulateKeyDownEvent(LogicalKeyboardKey.arrowDown);
19271926
await tester.pumpAndSettle();
1928-
await tester.testTextInput.receiveAction(TextInputAction.done);
1927+
await tester.sendKeyEvent(LogicalKeyboardKey.enter);
19291928
await tester.pumpAndSettle();
19301929
expect(selectionCount, expectedCount);
1931-
// The desktop platform closed the menu when a completion action is pressed. So we need to reopen it.
1932-
if (!isMobile) {
1933-
await tester.tap(find.byType(DropdownMenu<TestMenu>));
1934-
await tester.pump();
1935-
}
1930+
1931+
// Open the menu
1932+
await tester.tap(find.byType(DropdownMenu<TestMenu>));
1933+
await tester.pump();
19361934

19371935
// Disabled item doesn't trigger onSelected callback.
19381936
final Finder item1 = findMenuItemButton('Item 1');
19391937
await tester.tap(item1);
19401938
await tester.pumpAndSettle();
19411939

1942-
expect(controller.text, isMobile ? '' : 'Item 0');
1940+
expect(controller.text, 'Item 0');
19431941
expect(selectionCount, expectedCount);
19441942

19451943
final Finder item2 = findMenuItemButton('Item 2');
@@ -3576,6 +3574,94 @@ void main() {
35763574
expect(tester.getRect(findMenuMaterial()).top, textFieldBottom);
35773575
});
35783576
});
3577+
3578+
// Regression test for https://github.com/flutter/flutter/issues/143505.
3579+
testWidgets('Using keyboard navigation to select', (WidgetTester tester) async {
3580+
final FocusNode focusNode = FocusNode();
3581+
addTearDown(focusNode.dispose);
3582+
TestMenu? selectedMenu;
3583+
await tester.pumpWidget(
3584+
MaterialApp(
3585+
home: Material(
3586+
child: Center(
3587+
child: DropdownMenu<TestMenu>(
3588+
focusNode: focusNode,
3589+
dropdownMenuEntries: menuChildren,
3590+
onSelected: (TestMenu? menu) {
3591+
selectedMenu = menu;
3592+
},
3593+
),
3594+
),
3595+
),
3596+
),
3597+
);
3598+
// Pressing the tab key 3 times moves the focus to the icon button.
3599+
for (int i = 0; i < 3; i++) {
3600+
await tester.sendKeyEvent(LogicalKeyboardKey.tab);
3601+
await tester.pump();
3602+
}
3603+
3604+
// Now the focus is on the icon button.
3605+
final Element iconButton = tester.firstElement(find.byIcon(Icons.arrow_drop_down));
3606+
expect(Focus.of(iconButton).hasPrimaryFocus, isTrue);
3607+
3608+
await tester.sendKeyEvent(LogicalKeyboardKey.enter);
3609+
await tester.pump();
3610+
3611+
await tester.sendKeyEvent(LogicalKeyboardKey.arrowDown);
3612+
await tester.pump();
3613+
3614+
await tester.sendKeyEvent(LogicalKeyboardKey.enter);
3615+
await tester.pump();
3616+
3617+
expect(selectedMenu, TestMenu.mainMenu0);
3618+
}, variant: TargetPlatformVariant.all());
3619+
3620+
// Regression test for https://github.com/flutter/flutter/issues/143505.
3621+
testWidgets('Using keyboard navigation to select and without setting the FocusNode parameter',
3622+
(WidgetTester tester) async {
3623+
TestMenu? selectedMenu;
3624+
await tester.pumpWidget(
3625+
MaterialApp(
3626+
home: Material(
3627+
child: Center(
3628+
child: DropdownMenu<TestMenu>(
3629+
dropdownMenuEntries: menuChildren,
3630+
onSelected: (TestMenu? menu) {
3631+
selectedMenu = menu;
3632+
},
3633+
),
3634+
),
3635+
),
3636+
),
3637+
);
3638+
// If there is no `FocusNode`, by default, `TextField` can receive focus
3639+
// on desktop platforms, but not on mobile platforms. Therefore, on desktop
3640+
// platforms, it takes 3 tabs to reach the icon button.
3641+
final int tabCount = switch (defaultTargetPlatform) {
3642+
TargetPlatform.iOS || TargetPlatform.android || TargetPlatform.fuchsia => 2,
3643+
TargetPlatform.macOS || TargetPlatform.linux || TargetPlatform.windows => 3,
3644+
};
3645+
for (int i = 0; i < tabCount; i++) {
3646+
await tester.sendKeyEvent(LogicalKeyboardKey.tab);
3647+
await tester.pump();
3648+
}
3649+
3650+
// Now the focus is on the icon button.
3651+
final Element iconButton = tester.firstElement(find.byIcon(Icons.arrow_drop_down));
3652+
expect(Focus.of(iconButton).hasPrimaryFocus, isTrue);
3653+
3654+
await tester.sendKeyEvent(LogicalKeyboardKey.enter);
3655+
await tester.pump();
3656+
3657+
await tester.sendKeyEvent(LogicalKeyboardKey.arrowDown);
3658+
await tester.pump();
3659+
3660+
await tester.sendKeyEvent(LogicalKeyboardKey.enter);
3661+
await tester.pump();
3662+
3663+
expect(selectedMenu, TestMenu.mainMenu0);
3664+
}, variant: TargetPlatformVariant.all());
35793665
}
35803666

35813667
enum TestMenu {

0 commit comments

Comments
 (0)