Feat: Add a11y for loading indicators #165173

rkishan516 · 2025-03-14T07:10:06Z

Feat: Add a11y for loading indicators
fixes: #161631

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.

chunhtai · 2025-03-19T18:55:32Z

engine/src/flutter/lib/ui/semantics.dart

@@ -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;


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

actually do we need these flags in this pr? it looks like they are not used

Sure, I can remove these.

chunhtai · 2025-03-19T19:01:40Z

engine/src/flutter/lib/web_ui/lib/src/engine/semantics/progress_bar.dart

+        semanticsObject,
+        preferredLabelRepresentation: LabelRepresentation.ariaLabel,
+      ) {
+    setAriaRole('loadingSpinner');


there is no loading spiner role in web

Sure, I will remove this.

chunhtai · 2025-03-19T19:02:26Z

engine/src/flutter/lib/web_ui/lib/src/engine/semantics/progress_bar.dart

+        semanticsObject,
+        preferredLabelRepresentation: LabelRepresentation.ariaLabel,
+      ) {
+    setAriaRole('progressbar');


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.

Sure, i will add that.

chunhtai · 2025-03-19T19:03:51Z

packages/flutter/lib/src/material/progress_indicator.dart

-        return _buildIndicator(context, _controller.value, textDirection);
-      },
+    return Semantics(
+      role: SemanticsRole.loadingSpinner,


this is not a spinner, right?

It is. Because if value is not given, it will be in loading mode else it will be in progreessbar mode.

chunhtai · 2025-03-19T19:04:31Z

packages/flutter/lib/src/material/progress_indicator.dart

+      enabled: widget.value != null,
+      role: SemanticsRole.progressBar,
+      child: Semantics(
+        enabled: widget.value == null,


we should probably only assign progressbar or loadingspinner but not both

At a time, it only assign one.
Because if value is not given its loadingSpinner else its progressBar.

dkwingsmt · 2025-04-02T18:29:22Z

Is this PR ready for another round of review? (from triage)

rkishan516 · 2025-04-03T01:06:57Z

Is this PR ready for another round of review? (from triage)

Yes @dkwingsmt.

chunhtai · 2025-04-03T20:57:36Z

packages/flutter/lib/src/material/progress_indicator.dart

-        }
-    }
+    return Semantics(
+      value: widget.value != null ? '${(widget.value ?? 0) * 100}%' : null,


Suggested change

value: widget.value != null ? '${(widget.value ?? 0) * 100}%' : null,

value: widget.value != null ? '${widget.value! * 100}%' : null,

also should it have the % after the number?

I have removed this.

chunhtai · 2025-04-03T21:02:33Z

engine/src/flutter/lib/web_ui/lib/src/engine/semantics/progress_bar.dart

+    setAriaRole('progressbar');
+
+    // Progress indicators in Flutter use values between 0.0 and 1.0
+    setAttribute('aria-valuemin', "0");


we may need a a new property for maxValue and minValue to propergate the value range. Hardcoded to 0 and 100 may be too opinionated

Add new properties maxValue and minValue.

chunhtai

this overall lgtm to me, just the property type should be string so that it matches the SemanticsProperties.value

chunhtai · 2025-05-08T18:46:36Z

engine/src/flutter/lib/ui/semantics.dart

@@ -1854,6 +1862,8 @@ base class _NativeSemanticsUpdateBuilder extends NativeFieldWrapperClass1
    List<String>? controlsNodes,
    int validationResultIndex,
    int inputType,
+    double minValue,


This probably need to be string to match value

chunhtai · 2025-05-08T18:49:32Z

packages/flutter/lib/src/semantics/semantics.dart

+  ///
+  /// {@endtemplate}
+  ///
+  /// {@macro flutter.semantics.SemanticsProperties.maxValue}


you don't need to re macro here, the doc in @template in this section is still visible when compiling the doc. using macro here will cause it to duplicate the same doc.

The doc should also mention this is used in conjunction with [value]. and a typical usage is a progress indicator

chunhtai · 2025-05-27T21:29:32Z

packages/flutter/lib/src/material/progress_indicator.dart

+      label: semanticsLabel,
+      role:
+          expandedSemanticsValue != null ? SemanticsRole.progressBar : SemanticsRole.loadingSpinner,
+      minValue: '0.0',


shouldn't this be 0 and maxValue 100?

chunhtai · 2025-05-27T21:30:34Z

packages/flutter/lib/src/semantics/semantics.dart

@@ -135,15 +135,15 @@ sealed class _DebugSemanticsRoleChecks {
    SemanticsRole.status => _noLiveRegion,
    SemanticsRole.list => _noCheckRequired,
    SemanticsRole.listItem => _semanticsListItem,
+    SemanticsRole.loadingSpinner => _noCheckRequired,
+    SemanticsRole.progressBar => _noCheckRequired,


this should check if it has value, minValue, maxvalue. and ideally value should be within min and max

Sure, let me push the change.

chunhtai

ＬＧＴＭ

chunhtai · 2025-06-12T22:41:47Z

Hi @flutter-zl, can you take a secondary review since this also touch some of the web engine code?

Also looks like this pr may need a rebase, the ci seems to act weird

flutter-zl · 2025-06-13T16:42:48Z

LGTM.

flutter-zl

LGTM

auto-submit · 2025-06-13T17:20:44Z

autosubmit label was removed for flutter/flutter/165173, because - The status or check suite Linux linux_android_debug_engine has failed. Please fix the issues identified (or deflake) before re-applying this label.

The status or check suite Linux linux_android_debug_engine_ddm has failed. Please fix the issues identified (or deflake) before re-applying this label.
The status or check suite Linux linux_fuchsia has failed. Please fix the issues identified (or deflake) before re-applying this label.

rkishan516 · 2025-06-14T05:12:47Z

@chunhtai I don't know why tests are failing, can you please have a look ?

rkishan516 force-pushed the a11y-loading-indicator branch 2 times, most recently from a4da150 to f6e5619 Compare March 15, 2025 08:09

github-actions bot added the a: tests "flutter test", flutter_test, or one of our tests label Mar 15, 2025

dkwingsmt requested a review from chunhtai March 19, 2025 18:30

chunhtai reviewed Mar 19, 2025

View reviewed changes

rkishan516 force-pushed the a11y-loading-indicator branch from f6e5619 to 0034f92 Compare March 20, 2025 03:27

github-actions bot removed the a: tests "flutter test", flutter_test, or one of our tests label Mar 20, 2025

chunhtai self-requested a review April 2, 2025 18:33

rkishan516 force-pushed the a11y-loading-indicator branch from 0034f92 to b7a864f Compare April 3, 2025 01:06

chunhtai reviewed Apr 3, 2025

View reviewed changes

rkishan516 force-pushed the a11y-loading-indicator branch from b7a864f to 3f35fa6 Compare April 4, 2025 01:30

github-actions bot added a: tests "flutter test", flutter_test, or one of our tests platform-windows Building on or for Windows specifically a: desktop Running on desktop labels Apr 4, 2025

rkishan516 force-pushed the a11y-loading-indicator branch 6 times, most recently from d61aae5 to 050ace5 Compare April 5, 2025 01:42

rkishan516 force-pushed the a11y-loading-indicator branch from 050ace5 to cf1d583 Compare April 22, 2025 13:14

chunhtai self-requested a review April 22, 2025 16:54

rkishan516 force-pushed the a11y-loading-indicator branch from cf1d583 to 99965e8 Compare April 23, 2025 01:09

rkishan516 force-pushed the a11y-loading-indicator branch from 6d45f11 to 5e7af8b Compare May 8, 2025 09:03

chunhtai reviewed May 8, 2025

View reviewed changes

chunhtai requested a review from mdebbar May 8, 2025 18:52

rkishan516 force-pushed the a11y-loading-indicator branch 4 times, most recently from 41174e0 to 6fca6dc Compare May 13, 2025 02:45

chunhtai self-requested a review May 21, 2025 21:41

chunhtai reviewed May 27, 2025

View reviewed changes

rkishan516 force-pushed the a11y-loading-indicator branch 2 times, most recently from 6454d9d to d3c22fc Compare June 2, 2025 08:56

chunhtai self-requested a review June 4, 2025 21:47

rkishan516 force-pushed the a11y-loading-indicator branch from d3c22fc to 8e7c200 Compare June 11, 2025 15:08

chunhtai approved these changes Jun 12, 2025

View reviewed changes

chunhtai requested a review from flutter-zl June 12, 2025 22:40

rkishan516 force-pushed the a11y-loading-indicator branch from 8e7c200 to 094acc9 Compare June 13, 2025 03:02

rkishan516 added 5 commits June 13, 2025 21:29

Feat: Add a11y for loading indicators

d30056e

feat: add minValue and maxValue for semantics

483f9f8

refactor: change type for minValue and maxValue to String

a821821

Fix: Update progressbar semantics checker

eeb8f7e

fix: UpdateNode default values

b5abb59

rkishan516 force-pushed the a11y-loading-indicator branch from 094acc9 to b5abb59 Compare June 13, 2025 16:00

flutter-zl approved these changes Jun 13, 2025

View reviewed changes

chunhtai added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 13, 2025

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jun 13, 2025

Merge branch 'master' into a11y-loading-indicator

63de0af

	value: widget.value != null ? '${(widget.value ?? 0) * 100}%' : null,
	value: widget.value != null ? '${widget.value! * 100}%' : null,

Feat: Add a11y for loading indicators #165173

Are you sure you want to change the base?

Feat: Add a11y for loading indicators #165173

Conversation

rkishan516 commented Mar 14, 2025

Pre-launch Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dkwingsmt commented Apr 2, 2025

Uh oh!

rkishan516 commented Apr 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chunhtai left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chunhtai left a comment

Choose a reason for hiding this comment

Uh oh!

chunhtai commented Jun 12, 2025

Uh oh!

flutter-zl commented Jun 13, 2025

Uh oh!

flutter-zl left a comment

Choose a reason for hiding this comment

Uh oh!

auto-submit bot commented Jun 13, 2025

Uh oh!

rkishan516 commented Jun 14, 2025

Uh oh!

Uh oh!