Modify dataMap to include assertiveness only if it's not set to polite#108396
Modify dataMap to include assertiveness only if it's not set to polite#108396nbayati merged 2 commits intoflutter:masterfrom
Conversation
…e to prevent google3 tests from breaking
flutter-dashboard
bot
added
a: accessibility
nbayati
changed the title
| }; | ||
|
|
||
| if (assertiveness == Assertiveness.assertive) { | ||
| dataMap['assertiveness']=assertiveness.index; |
| 'textDirection': textDirection.index, | ||
| }; | ||
|
|
||
| if (assertiveness == Assertiveness.assertive) { |
There was a problem hiding this comment.
Please use switch/case for enums: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#avoid-using-if-chains-or--or--with-enum-values
| 'textDirection': textDirection.index, | ||
| }; | ||
|
|
||
| if (assertiveness == Assertiveness.assertive) { |
There was a problem hiding this comment.
Overall, this is a pretty strange design. Can we just fix up the failing tests in google?
There was a problem hiding this comment.
This PR is accompanied by an engine change as well, so my recommendation is to avoid touching too many repositories at once, which is achieved by making it non-breaking.
Having said that, fixing downstream tests sounds like a good idea. We can do it in multiple phases. Phase one - this change - introduce the API in a non-breaking fashion and stabilize it. Phase two - update tests to accept assertiveness: 0. Phase three - remove the condition.
There was a problem hiding this comment.
To be very clear: I don't think it is great practice that this was submitted while we were still discussing the best course forward. There should have at the very least been a TODO left here to remind future readers why we are (temporarily) doing this weirdness. Also, please clean this up ASAP once this has rolled into google.
| 'textDirection': textDirection.index, | ||
| }; | ||
|
|
||
| if (assertiveness == Assertiveness.assertive) { |
There was a problem hiding this comment.
This PR is accompanied by an engine change as well, so my recommendation is to avoid touching too many repositories at once, which is achieved by making it non-breaking.
Having said that, fixing downstream tests sounds like a good idea. We can do it in multiple phases. Phase one - this change - introduce the API in a non-breaking fashion and stabilize it. Phase two - update tests to accept assertiveness: 0. Phase three - remove the condition.
flutter#108396) * Modify dataMap to include assertiveness only if it's not set to polite to prevent google3 tests from breaking * small code style enhancement
3 participants
Add an optional flag to Flutter's accessibility announcer service to support two different modes of aria announcements in flutter web. Developers can make the announcement assertive or polite using this flag. The change on the web engine side is tracked here: flutter/engine#34640
The first PR was breaking some of the Google tests and had to get reverted. The tests were breaking because they were receiving an additional piece of information (
assertiveness) in the dataMap string. In this fix, we don't includeassertivenessif it has the default value ofpolite.Fixes #104109
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.