Feat: Add a11y for loading indicators #165173
Conversation
a4da150 to
f6e5619
Compare
| @@ -530,6 +530,8 @@ class SemanticsFlag { | |||
| static const int _kHasSelectedStateIndex = 1 << 28; | |||
| static const int _kHasRequiredStateIndex = 1 << 29; | |||
| static const int _kIsRequiredIndex = 1 << 30; | |||
| static const int _kIsProgressBarIndex = 1 << 31; | |||
| static const int _kIsLoadingSpinnerIndex = 1 << 32; | |||
There was a problem hiding this comment.
This flag can at most fit 32 bit, so this is likely not going to work for platform that uses 32 bit int.
Also we already have progress bar and loadingspinner in https://main-api.flutter.dev/flutter/dart-ui/SemanticsRole.html
you can directly use that.
However these 2 enums are currently not wired up engine, so we will need to update the engine still
There was a problem hiding this comment.
actually do we need these flags in this pr? it looks like they are not used
There was a problem hiding this comment.
Sure, I can remove these.
| semanticsObject, | ||
| preferredLabelRepresentation: LabelRepresentation.ariaLabel, | ||
| ) { | ||
| setAriaRole('loadingSpinner'); |
There was a problem hiding this comment.
there is no loading spiner role in web
There was a problem hiding this comment.
Sure, I will remove this.
| semanticsObject, | ||
| preferredLabelRepresentation: LabelRepresentation.ariaLabel, | ||
| ) { | ||
| setAriaRole('progressbar'); |
There was a problem hiding this comment.
based on the https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Reference/Roles/progressbar_role
we should add aria-valuemin, aria-valuemax, aria-valuenow if possible.
There was a problem hiding this comment.
Sure, i will add that.
| return _buildIndicator(context, _controller.value, textDirection); | ||
| }, | ||
| return Semantics( | ||
| role: SemanticsRole.loadingSpinner, |
There was a problem hiding this comment.
this is not a spinner, right?
There was a problem hiding this comment.
It is. Because if value is not given, it will be in loading mode else it will be in progreessbar mode.
| enabled: widget.value != null, | ||
| role: SemanticsRole.progressBar, | ||
| child: Semantics( | ||
| enabled: widget.value == null, |
There was a problem hiding this comment.
we should probably only assign progressbar or loadingspinner but not both
There was a problem hiding this comment.
At a time, it only assign one.
Because if value is not given its loadingSpinner else its progressBar.
f6e5619 to
0034f92
Compare
Feat: Add a11y for loading indicators
fixes: #161631
Pre-launch Checklist
///).