From 75d93e1c44b6a769d652ee409ec064f234fccd5a Mon Sep 17 00:00:00 2001 From: Hannah Jin Date: Mon, 14 Oct 2024 13:36:24 -0700 Subject: [PATCH] Update dropdown menu semantics to fix its a11y issues (#156709) fixes https://github.com/flutter/flutter/issues/133742 because the _initialMenu is invisible, we should not add semantics nodes to it ## Pre-launch Checklist - [ ] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [ ] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [ ] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [ ] I signed the [CLA]. - [ ] I listed at least one issue that this PR fixes in the description above. - [ ] I updated/added relevant documentation (doc comments with `///`). - [ ] I added new tests to check the change I am making, or this PR is [test-exempt]. - [ ] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [ ] 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 --- .../lib/src/material/dropdown_menu.dart | 54 +++++++++++-------- .../test/material/dropdown_menu_test.dart | 37 ++++++++++++- 2 files changed, 67 insertions(+), 24 deletions(-) diff --git a/packages/flutter/lib/src/material/dropdown_menu.dart b/packages/flutter/lib/src/material/dropdown_menu.dart index 9c11c5523e8..1ea2aa525d3 100644 --- a/packages/flutter/lib/src/material/dropdown_menu.dart +++ b/packages/flutter/lib/src/material/dropdown_menu.dart @@ -674,6 +674,7 @@ class _DropdownMenuState extends State> { TextDirection textDirection, { int? focusedIndex, bool enableScrollToHighlight = true, + bool excludeSemantics = false, }) { final List result = []; for (int i = 0; i < filteredEntries.length; i++) { @@ -743,28 +744,31 @@ class _DropdownMenuState extends State> { ); } - final Widget menuItemButton = MenuItemButton( - key: enableScrollToHighlight ? buttonItemKeys[i] : null, - style: effectiveStyle, - leadingIcon: entry.leadingIcon, - trailingIcon: entry.trailingIcon, - closeOnActivate: widget.closeBehavior == DropdownMenuCloseBehavior.all, - onPressed: entry.enabled && widget.enabled - ? () { - _localTextEditingController?.value = TextEditingValue( - text: entry.label, - selection: TextSelection.collapsed(offset: entry.label.length), - ); - currentHighlight = widget.enableSearch ? i : null; - widget.onSelected?.call(entry.value); - _enableFilter = false; - if (widget.closeBehavior == DropdownMenuCloseBehavior.self) { - _controller.close(); + final Widget menuItemButton = ExcludeSemantics( + excluding: excludeSemantics, + child: MenuItemButton( + key: enableScrollToHighlight ? buttonItemKeys[i] : null, + style: effectiveStyle, + leadingIcon: entry.leadingIcon, + trailingIcon: entry.trailingIcon, + closeOnActivate: widget.closeBehavior == DropdownMenuCloseBehavior.all, + onPressed: entry.enabled && widget.enabled + ? () { + _localTextEditingController?.value = TextEditingValue( + text: entry.label, + selection: TextSelection.collapsed(offset: entry.label.length), + ); + currentHighlight = widget.enableSearch ? i : null; + widget.onSelected?.call(entry.value); + _enableFilter = false; + if (widget.closeBehavior == DropdownMenuCloseBehavior.self) { + _controller.close(); + } } - } - : null, - requestFocusOnHover: false, - child: label, + : null, + requestFocusOnHover: false, + child: label, + ), ); result.add(menuItemButton); } @@ -828,7 +832,13 @@ class _DropdownMenuState extends State> { @override Widget build(BuildContext context) { final TextDirection textDirection = Directionality.of(context); - _initialMenu ??= _buildButtons(widget.dropdownMenuEntries, textDirection, enableScrollToHighlight: false); + _initialMenu ??= _buildButtons( + widget.dropdownMenuEntries, + textDirection, + enableScrollToHighlight: false, + // The _initialMenu is invisible, we should not add semantics nodes to it + excludeSemantics: true, + ); final DropdownMenuThemeData theme = DropdownMenuTheme.of(context); final DropdownMenuThemeData defaults = _DropdownMenuDefaultsM3(context); diff --git a/packages/flutter/test/material/dropdown_menu_test.dart b/packages/flutter/test/material/dropdown_menu_test.dart index bcc77b90f6b..17c069482c3 100644 --- a/packages/flutter/test/material/dropdown_menu_test.dart +++ b/packages/flutter/test/material/dropdown_menu_test.dart @@ -2286,7 +2286,6 @@ void main() { }); testWidgets('Semantics does not include hint when input is not empty', (WidgetTester tester) async { - final ThemeData themeData = ThemeData(); const String hintText = 'I am hintText'; TestMenu? selectedValue; final TextEditingController controller = TextEditingController(); @@ -2295,7 +2294,6 @@ void main() { await tester.pumpWidget( StatefulBuilder( builder: (BuildContext context, StateSetter setState) => MaterialApp( - theme: themeData, home: Scaffold( body: Center( child: DropdownMenu( @@ -2329,6 +2327,41 @@ void main() { expect(node.value, 'Item 3'); }); + + testWidgets('Semantics does not include initial menu buttons', (WidgetTester tester) async { + final TextEditingController controller = TextEditingController(); + addTearDown(controller.dispose); + + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: Center( + child: DropdownMenu( + requestFocusOnTap: true, + dropdownMenuEntries: menuChildren, + onSelected: (TestMenu? value) {}, + controller: controller, + ), + ), + ), + ), + ); + // The menu buttons should not be visible and should not be in the semantics tree. + for (final String label in TestMenu.values.map((TestMenu menu) => menu.label)) { + expect(find.bySemanticsLabel(label), findsNothing); + } + + // Open the menu. + await tester.tap(find.widgetWithIcon(IconButton, Icons.arrow_drop_down).first); + await tester.pump(); + + // The menu buttons should be visible and in the semantics tree. + for (final String label in TestMenu.values.map((TestMenu menu) => menu.label)) { + expect(find.bySemanticsLabel(label), findsOneWidget); + } + + }); + testWidgets('helperText is not visible when errorText is not null', (WidgetTester tester) async { final ThemeData themeData = ThemeData(); const String helperText = 'I am helperText';