-
Notifications
You must be signed in to change notification settings - Fork 29.7k
[VPAT][A11y] Announce Autocomplete search results status #173480
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[VPAT][A11y] Announce Autocomplete search results status #173480
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request enhances the accessibility of the Autocomplete widget by announcing search result status changes, which is a great improvement for users relying on screen readers. It also includes a fix to prevent tooltips from appearing during route transitions. The changes are well-implemented and include corresponding tests. I have one minor suggestion to improve code readability in autocomplete.dart.
| return; | ||
| } | ||
| if (_options.isEmpty != options.isEmpty) { | ||
| _announceSemantics(_options.isEmpty); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While the logic here is correct, it's a bit confusing to read. The call to _announceSemantics uses the state of the old options list (_options.isEmpty) to determine the announcement for the new options list.
Inside this if block, _options.isEmpty is logically equivalent to !options.isEmpty, but using the latter makes the intent much clearer: the announcement should be based on whether the new options list has results. This improves readability and maintainability, which is a core principle of the Flutter style guide.1
_announceSemantics(!options.isEmpty);
Style Guide References
Footnotes
justinmc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with nits 👍
|
|
||
| /// The semantics label used for [RawAutocomplete] when the options list goes | ||
| /// from non-empty to empty. | ||
| String get searchResultsUnavailable => 'Search results not available'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you get these strings from somewhere official? I just want to verify that this is exactly what it should say.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Kinda, the strings are from the google bug description, but even in there it doesn't seem 'official'.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we want official sentence, we need to ask material team
| if (_options.isEmpty != options.isEmpty) { | ||
| _announceSemantics(options.isNotEmpty); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Just wondering, would it be better to make this announcement at the bottom of this method when the options have already been shown/updated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Somewhat related to Gemini's comment #173480 (comment).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This conditional has to happen before updating _options, because we only want to announce when the options view appears or disappears, not when it merely changes.
But the code after this block depends on _options getting updated. _updateHighlight and _updateOptionsViewVisibility.
|
|
||
| /// The semantics label used for [RawAutocomplete] when the options list goes | ||
| /// from non-empty to empty. | ||
| String get searchResultsUnavailable => 'Search results not available'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we want official sentence, we need to ask material team
chunhtai
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
) Status announcements 'Search results found' and 'no results found' were observed on a search view on the iOS sheets app. Fixes [[VPAT][A11y] autocomplete must announce status when search result is available](flutter#173064) Fixes b/429094918
flutter/flutter@f4334d2...52af7a5 2025-08-15 [email protected] Roll Packages from 09533b7 to 5c52c55 (6 revisions) (flutter/flutter#173854) 2025-08-15 [email protected] Roll Skia from 46ec77ae3954 to 5654ac32ede0 (1 revision) (flutter/flutter#173848) 2025-08-15 [email protected] Roll Skia from 162f47d6b6bd to 46ec77ae3954 (2 revisions) (flutter/flutter#173833) 2025-08-15 [email protected] Roll Dart SDK from c7faab270f27 to cc008dc8e7aa (2 revisions) (flutter/flutter#173826) 2025-08-15 [email protected] Roll Skia from ad5d04000101 to 162f47d6b6bd (5 revisions) (flutter/flutter#173822) 2025-08-15 [email protected] Update the RBE configuration for the recent Clang update (flutter/flutter#173803) 2025-08-15 [email protected] Stop writing legacy `FLUTTER_ROOT/version` file (by default?) (flutter/flutter#172793) 2025-08-15 [email protected] Remove `luci_flags.parallel_download_builds` and friends (flutter/flutter#173799) 2025-08-14 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Predictive back route transitions by default (#165832)" (flutter/flutter#173809) 2025-08-14 [email protected] Roll Skia from dca5f05fee87 to ad5d04000101 (8 revisions) (flutter/flutter#173798) 2025-08-14 [email protected] [web] Cleanup usages of deprecated `routeUpdated` message (flutter/flutter#173782) 2025-08-14 [email protected] Make sure that DataTable, DataColumn, DataRow, and DataCell don't crash in 0x0 environment (flutter/flutter#173515) 2025-08-14 [email protected] Make sure that a TableRowInkWell doesn't crash in 0x0 environment (flutter/flutter#173627) 2025-08-14 [email protected] Make sure that a DatePickerDialog doesn't crash in 0x0 environment (flutter/flutter#173677) 2025-08-14 [email protected] Return result of setting OpenGL contexts back to Flutter (flutter/flutter#173757) 2025-08-14 [email protected] Read `bin/cache/flutter.version.json` instead of `version` for `flutter_gallery` (flutter/flutter#173797) 2025-08-14 [email protected] Predictive back route transitions by default (flutter/flutter#165832) 2025-08-14 [email protected] feat: add onLongPressUp callback to InkWell widget (flutter/flutter#173221) 2025-08-14 [email protected] Roll Dart SDK from 214a7f829913 to c7faab270f27 (1 revision) (flutter/flutter#173792) 2025-08-14 [email protected] Add error handling for `Element` lifecycle user callbacks (flutter/flutter#173148) 2025-08-14 [email protected] Roll Fuchsia Linux SDK from I1TfNmsqTp7t3rO8e... to zWRpLglb48zC1vZLv... (flutter/flutter#173784) 2025-08-14 [email protected] [Range slider] Tap on active range, the thumb closest to the mouse cursor should move to the cursor position. (flutter/flutter#173725) 2025-08-14 [email protected] Roll Packages from 6cb9113 to 09533b7 (4 revisions) (flutter/flutter#173789) 2025-08-14 [email protected] Implements the Android native stretch effect as a fragment shader (Impeller-only). (flutter/flutter#169293) 2025-08-14 [email protected] Sync `CHANGELOG.md` (3.35 -> `master`) (flutter/flutter#173790) 2025-08-14 [email protected] [VPAT][A11y] Announce Autocomplete search results status (flutter/flutter#173480) 2025-08-14 [email protected] Fix InputDecorator label padding (flutter/flutter#173344) 2025-08-14 [email protected] Fix default minimumSize in dropdownMenu when maximumSize is null (flutter/flutter#169438) 2025-08-14 [email protected] Thread sub-builders for every engine-uploading builder (flutter/flutter#173742) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
) Status announcements 'Search results found' and 'no results found' were observed on a search view on the iOS sheets app. Fixes [[VPAT][A11y] autocomplete must announce status when search result is available](flutter#173064) Fixes b/429094918
…r#9832) flutter/flutter@f4334d2...52af7a5 2025-08-15 [email protected] Roll Packages from 09533b7 to 5c52c55 (6 revisions) (flutter/flutter#173854) 2025-08-15 [email protected] Roll Skia from 46ec77ae3954 to 5654ac32ede0 (1 revision) (flutter/flutter#173848) 2025-08-15 [email protected] Roll Skia from 162f47d6b6bd to 46ec77ae3954 (2 revisions) (flutter/flutter#173833) 2025-08-15 [email protected] Roll Dart SDK from c7faab270f27 to cc008dc8e7aa (2 revisions) (flutter/flutter#173826) 2025-08-15 [email protected] Roll Skia from ad5d04000101 to 162f47d6b6bd (5 revisions) (flutter/flutter#173822) 2025-08-15 [email protected] Update the RBE configuration for the recent Clang update (flutter/flutter#173803) 2025-08-15 [email protected] Stop writing legacy `FLUTTER_ROOT/version` file (by default?) (flutter/flutter#172793) 2025-08-15 [email protected] Remove `luci_flags.parallel_download_builds` and friends (flutter/flutter#173799) 2025-08-14 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Predictive back route transitions by default (#165832)" (flutter/flutter#173809) 2025-08-14 [email protected] Roll Skia from dca5f05fee87 to ad5d04000101 (8 revisions) (flutter/flutter#173798) 2025-08-14 [email protected] [web] Cleanup usages of deprecated `routeUpdated` message (flutter/flutter#173782) 2025-08-14 [email protected] Make sure that DataTable, DataColumn, DataRow, and DataCell don't crash in 0x0 environment (flutter/flutter#173515) 2025-08-14 [email protected] Make sure that a TableRowInkWell doesn't crash in 0x0 environment (flutter/flutter#173627) 2025-08-14 [email protected] Make sure that a DatePickerDialog doesn't crash in 0x0 environment (flutter/flutter#173677) 2025-08-14 [email protected] Return result of setting OpenGL contexts back to Flutter (flutter/flutter#173757) 2025-08-14 [email protected] Read `bin/cache/flutter.version.json` instead of `version` for `flutter_gallery` (flutter/flutter#173797) 2025-08-14 [email protected] Predictive back route transitions by default (flutter/flutter#165832) 2025-08-14 [email protected] feat: add onLongPressUp callback to InkWell widget (flutter/flutter#173221) 2025-08-14 [email protected] Roll Dart SDK from 214a7f829913 to c7faab270f27 (1 revision) (flutter/flutter#173792) 2025-08-14 [email protected] Add error handling for `Element` lifecycle user callbacks (flutter/flutter#173148) 2025-08-14 [email protected] Roll Fuchsia Linux SDK from I1TfNmsqTp7t3rO8e... to zWRpLglb48zC1vZLv... (flutter/flutter#173784) 2025-08-14 [email protected] [Range slider] Tap on active range, the thumb closest to the mouse cursor should move to the cursor position. (flutter/flutter#173725) 2025-08-14 [email protected] Roll Packages from 6cb9113 to 09533b7 (4 revisions) (flutter/flutter#173789) 2025-08-14 [email protected] Implements the Android native stretch effect as a fragment shader (Impeller-only). (flutter/flutter#169293) 2025-08-14 [email protected] Sync `CHANGELOG.md` (3.35 -> `master`) (flutter/flutter#173790) 2025-08-14 [email protected] [VPAT][A11y] Announce Autocomplete search results status (flutter/flutter#173480) 2025-08-14 [email protected] Fix InputDecorator label padding (flutter/flutter#173344) 2025-08-14 [email protected] Fix default minimumSize in dropdownMenu when maximumSize is null (flutter/flutter#169438) 2025-08-14 [email protected] Thread sub-builders for every engine-uploading builder (flutter/flutter#173742) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
) Status announcements 'Search results found' and 'no results found' were observed on a search view on the iOS sheets app. Fixes [[VPAT][A11y] autocomplete must announce status when search result is available](flutter#173064) Fixes b/429094918
) Status announcements 'Search results found' and 'no results found' were observed on a search view on the iOS sheets app. Fixes [[VPAT][A11y] autocomplete must announce status when search result is available](flutter#173064) Fixes b/429094918
) Status announcements 'Search results found' and 'no results found' were observed on a search view on the iOS sheets app. Fixes [[VPAT][A11y] autocomplete must announce status when search result is available](flutter#173064) Fixes b/429094918
Status announcements 'Search results found' and 'no results found' were observed on a search view on the iOS sheets app.
Fixes [VPAT][A11y] autocomplete must announce status when search result is available
Fixes b/429094918