Skip to content

fixed issue to where screen reader reads all buttons when opening datepicker#152705

Merged
auto-submit[bot] merged 16 commits intoflutter:masterfrom
DBowen33:datepicker-sr-issue-fix
Aug 22, 2024
Merged

fixed issue to where screen reader reads all buttons when opening datepicker#152705
auto-submit[bot] merged 16 commits intoflutter:masterfrom
DBowen33:datepicker-sr-issue-fix

Conversation

@DBowen33
Copy link
Copy Markdown
Contributor

@DBowen33 DBowen33 commented Aug 1, 2024

removed container=true property from Semantics wrapper of datepicker dialog. This prevents the screen reader from reading every button in the datepicker dialog whenever a user opens it, reducing SR noise.

Before: https://screencast.googleplex.com/cast/Njc0Mzc1MDA5MTk5NzE4NHw2NTU5ODI4YS0xNA
After: https://screencast.googleplex.com/cast/NTYxMDUwOTIxMzYzMDQ2NHxjOWQ2M2YzNy1hYQ

fixes b/345297872

NOTE: Please let me know if a test is needed for this, since it is only deleting code.

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Aug 1, 2024
@DBowen33 DBowen33 changed the title fixed issue to where screen reader reads all button when opening date… fixed issue to where screen reader reads all button when opening datepicker Aug 1, 2024
@DBowen33 DBowen33 changed the title fixed issue to where screen reader reads all button when opening datepicker fixed issue to where screen reader reads all buttons when opening datepicker Aug 1, 2024
@DBowen33 DBowen33 marked this pull request as ready for review August 1, 2024 23:26
@hannah-hyj
Copy link
Copy Markdown
Member

Yes a PR like this need a test, because the behavior is changed.

@DBowen33
Copy link
Copy Markdown
Contributor Author

DBowen33 commented Aug 2, 2024

Added test

@DBowen33 DBowen33 requested a review from hannah-hyj August 2, 2024 21:46
@DBowen33 DBowen33 force-pushed the datepicker-sr-issue-fix branch from ef3baaf to 50dbe75 Compare August 6, 2024 21:37
@hannah-hyj
Copy link
Copy Markdown
Member

hannah-hyj commented Aug 14, 2024

Removing it will also cause mobile a11y behavior to change. Semantics container here controls if the date picker header is a node in the semantics tree, This line was added in #143117 to solve issue #141992. The date picker header being a node is consistent with native a11y behavior. So I think we should keep the Semantics container for the date picker header.

As for SR, i do agree reading out all buttons is too much, does SR always read out all buttons if they are at the same level?

@DBowen33
Copy link
Copy Markdown
Contributor Author

DBowen33 commented Aug 16, 2024

@Hangyujin thank you for the feedback. I went ahead and reverted the change and instead added a container: true to the month toggle section. This solves the behavior of reading all of the interactive elements in the datepicker dialog and instead just reads the heading when user opens datepicker dialog and focus goes on it.

Also added explicitChildNodes: true so that the previous and next buttons don't get grouped into the Semantics node of the month selector toggle button on the top. This prevents the SR from reading out the next and previous buttons when the user is focused on selecting a new day.

Also removed excludeSemantics: true from the datepicker toggle button because I noticed that when user is focused on that button, the Semantics information was not being read out to the user. Not quite sure why that was added on that.

Before: https://screencast.googleplex.com/cast/Njc0Mzc1MDA5MTk5NzE4NHw2NTU5ODI4YS0xNA
After:
Screen recording 2024-08-15 5.24.19 PM.webm

@DBowen33
Copy link
Copy Markdown
Contributor Author

I'm also having some trouble creating a test case to reflect these new changes, if you have any ideas on what direction to take for this it would be greatly appreciated! :)

@hannah-hyj
Copy link
Copy Markdown
Member

hannah-hyj commented Aug 16, 2024

The new changes looks good to me!

For tests:
You tests already reflect the excludeSemantics changes.

As for container: true , You can either use final SemanticsTester semantics = SemanticsTester(tester); and test the semantics tree changes or use tester.getSemantics() to find a semantics node and test it.

I tried on my mobile phone and from the semantics tree it looks like adding container: true in the date picker Semantics will remove the isFocusable flag in the date picker SemanticsNode. You can add a test to reflect that.

@DBowen33 DBowen33 force-pushed the datepicker-sr-issue-fix branch from 3794775 to 7955c1e Compare August 19, 2024 15:26
@DBowen33 DBowen33 force-pushed the datepicker-sr-issue-fix branch from 7955c1e to 7f24c10 Compare August 19, 2024 15:27
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 27, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 28, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 28, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 28, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 28, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 28, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 28, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 29, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 29, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 29, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 29, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 29, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 29, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 29, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 29, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 29, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 29, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 30, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 30, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 30, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 30, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 30, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 30, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 30, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 30, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 31, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 31, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 1, 2024
Buchimi pushed a commit to Buchimi/flutter that referenced this pull request Sep 2, 2024
…epicker (flutter#152705)

removed container=true property from Semantics wrapper of datepicker dialog. This prevents the screen reader from reading every button in the datepicker dialog whenever a user opens it, reducing SR noise.

Before: https://screencast.googleplex.com/cast/Njc0Mzc1MDA5MTk5NzE4NHw2NTU5ODI4YS0xNA
After: https://screencast.googleplex.com/cast/NTYxMDUwOTIxMzYzMDQ2NHxjOWQ2M2YzNy1hYQ

fixes b/345297872

NOTE: Please let me know if a test is needed for this, since it is only deleting code.
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants