Adds dialog and alertdialog role #162692

chunhtai · 2025-02-04T21:05:25Z

fixes #162124
fixes #157207
fixes #157204

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.

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

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

yjbanov · 2025-02-06T19:23:43Z

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

+/// reader better understand this section of screen.
+///
+/// Screen-readers takes advantage of "aria-label" to describe the visual.
+class SemanticDialog extends SemanticRole with RouteLike {


Might be simpler to maintain all route semantics in the same file, route.dart. WDYT?

yjbanov · 2025-02-06T19:25:31Z

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

+import 'semantics.dart';
+import 'route.dart';
+
+/// Indicates the container as a pop dialog


nit: end with a period.

yjbanov · 2025-02-06T19:26:01Z

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

+/// Setting this role will also set aria-modal to true, which helps screen
+/// reader better understand this section of screen.
+///
+/// Screen-readers takes advantage of "aria-label" to describe the visual.


nit: s/takes/take/

yjbanov · 2025-02-06T19:27:35Z

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

+/// Setting this role will also set aria-modal to true, which helps screen
+/// reader better understand this section of screen.
+///
+/// Screen-readers takes advantage of "aria-label" to describe the visual.


The situation with labeling is a bit more complicated, what with all the namesRoute. Just mentioning aria-label alone may be a bit misleading.

yjbanov · 2025-02-06T19:30:12Z

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

 import '../semantics.dart';
 import '../util.dart';

+mixin RouteLike on SemanticRole {


It seems like all our route types are expressible using a simple class hierarchy:

class SemanticRouteBase extends SemanticRole; class SemanticRoute extends SemanticRouteBase; class SemanticDialog extends SemanticRouteBase; class SemanticAlertDialog extends SemanticRouteBase;

yjbanov · 2025-02-06T19:32:08Z

engine/src/flutter/lib/web_ui/test/engine/semantics/semantics_test.dart

  group('tabs', () {
    _testTabs();
  });
+  group('dialogs', () {


I'd either put this into the existing 'route' group (see comment about combining all routes into the same library), or put it next to the 'route' group since the two a closely related.

yjbanov · 2025-02-06T19:32:36Z

engine/src/flutter/lib/web_ui/test/engine/semantics/semantics_test.dart

  });
 }

+void _testDialogs() {


Consider moving into/closer to the route tests.

yjbanov · 2025-02-06T19:35:20Z

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

-      child: dialogChild,
+    return Semantics(
+      role: SemanticsRole.alertDialog,
+      child: Dialog(


To avoid having two roles set on the dialog that need to be resolve by merging or something, can we instead have the Dialog constructor take an optional semanticsRole argument (similar to semanticLabel that many of our widgets accept):

Dialog({ SemanticsRole semanticRole = SemanticsRole.dialog, ... other args ... });

yjbanov · 2025-02-10T19:40:53Z

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

-class SemanticRoute extends SemanticRole {
-  SemanticRoute(SemanticsObject semanticsObject)
-    : super.blank(EngineSemanticsRole.route, semanticsObject) {
+class SemanticRoutBase extends SemanticRole {


s/Rout/Route/

yjbanov · 2025-02-10T19:41:54Z

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


+/// Denotes that all descendant nodes are inside a route.
+///
+/// Routes can include dialogs, pop-up menus, sub-screens, and more.


This role is no longer used for dialogs, correct?

yjbanov · 2025-02-10T19:44:34Z

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

-  SemanticRoute(SemanticsObject semanticsObject)
-    : super.blank(EngineSemanticsRole.route, semanticsObject) {
+class SemanticRoutBase extends SemanticRole {
+  SemanticRoutBase(EngineSemanticsRole kind, SemanticsObject object) : super.blank(kind, object) {


This class seems to hard-code "dialog" ARIA. Should we use different ARIA roles for different types of routes? E.g. the SemanticAlertDialog could use https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/alertdialog_role

looks like i missed this when i refactored, will fix

yjbanov

yjbanov · 2025-02-10T23:09:02Z

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

+///
+/// See also:
+///
+///   * [RouteName], which provides a description for this route in the absent


nit: s/absent/absence/

thanks, for some reason my ide told me absence is wrong, but it is not. will fix

Roll Flutter from 892f9c1 to e8f34a9 (71 revisions) flutter/flutter@892f9c1...e8f34a9 2025-02-12 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Roll Skia from f31c733c86c4 to 25937c31f153 (2 revisions) (#163127)" (flutter/flutter#163133) 2025-02-12 [email protected] Roll Skia from f31c733c86c4 to 25937c31f153 (2 revisions) (flutter/flutter#163127) 2025-02-12 [email protected] Update .ci.yaml to support Fuchsia cherrypick branches (flutter/flutter#163000) 2025-02-12 [email protected] Roll Skia from 6f17f2ebb2e5 to f31c733c86c4 (1 revision) (flutter/flutter#163112) 2025-02-12 [email protected] Roll Skia from a9dbb2479c26 to 6f17f2ebb2e5 (2 revisions) (flutter/flutter#163109) 2025-02-12 [email protected] [devicelab] dont strip symbols in platform views layout test. (flutter/flutter#163101) 2025-02-12 [email protected] [Impeller] mirror tile mode requires highp for Adreno. (flutter/flutter#163066) 2025-02-12 [email protected] Roll Skia from 5b56d9a91633 to a9dbb2479c26 (6 revisions) (flutter/flutter#163100) 2025-02-12 [email protected] Roll Dart SDK from d9d7f103b6b7 to fcef25f18e4d (3 revisions) (flutter/flutter#163098) 2025-02-12 [email protected] Generate a correct `.flutter-plugin-dependencies` file for iOS/macOS projects (flutter/flutter#162834) 2025-02-12 [email protected] Remove unsound artifacts, remove `*Sound` qualifier. (flutter/flutter#163015) 2025-02-12 [email protected] [Impeller] libImpeller: Add support for Metal and Vulkan rendering. (flutter/flutter#161547) 2025-02-11 [email protected] Marks Mac_benchmark basic_material_app_macos__compile to be flaky (flutter/flutter#162365) 2025-02-11 [email protected] Roll pub packages (flutter/flutter#163083) 2025-02-11 [email protected] Adds hasSelectedState parameter to matchesSemantics for migration (flutter/flutter#163014) 2025-02-11 [email protected] fix: Dispose codec after completing frame creation (flutter/flutter#159945) 2025-02-11 [email protected] [ios][secure_paste]show menu item based on info sent from framework (flutter/flutter#161103) 2025-02-11 [email protected] Update conductor to support monorepos (flutter/flutter#161704) 2025-02-11 [email protected] [Android] fix hcpp tapping, again, and add test. (flutter/flutter#163035) 2025-02-11 [email protected] Add new builder for experiment with dynamic modules. (flutter/flutter#162855) 2025-02-11 [email protected] Roll vulkan-deps to 9edf248c597b (flutter/flutter#162549) 2025-02-11 [email protected] Adds dialog and alertdialog role (flutter/flutter#162692) 2025-02-11 [email protected] Roll Dart SDK from 99789828cc95 to d9d7f103b6b7 (12 revisions) (flutter/flutter#163060) 2025-02-11 [email protected] [ Widget Preview ] Cleanup PreviewDetector code (flutter/flutter#163050) 2025-02-11 [email protected] Fix `SkiaException` -> `TestFailure`, add tests. (flutter/flutter#163054) 2025-02-11 [email protected] [Android] fix hcpp overlay layer intersection. (flutter/flutter#163024) 2025-02-11 [email protected] [ Widget Preview ] Update generated scaffold project to include early preview rendering (flutter/flutter#162847) 2025-02-11 [email protected] [Embedder] Implement merged platform and UI thread (flutter/flutter#162944) 2025-02-11 [email protected] [Android] Remove overlay when platform views are removed from screen. (flutter/flutter#162908) 2025-02-11 [email protected] Roll Dart to 3.8.0-76.0.dev (flutter/flutter#162913) 2025-02-11 [email protected] [Android] add HCPP platform views benchmark and integration test. (flutter/flutter#163018) 2025-02-11 [email protected] Roll Skia from 8c377e8bedd2 to 5b56d9a91633 (9 revisions) (flutter/flutter#163021) 2025-02-11 [email protected] Try golden-testing on a Mokey (`bringup: true`), retry on an emulator (flutter/flutter#163029) 2025-02-11 [email protected] Fix Linux keyboard support for AltGr (flutter/flutter#162495) 2025-02-11 [email protected] Update gen_keycodes output to new engine location. (flutter/flutter#162479) 2025-02-10 [email protected] [Android] add runtime flag to determine if HCPP is supported. (flutter/flutter#163004) 2025-02-10 [email protected] [iOS][Engine] Fix view removal process for AutofillContextAction.cancel (flutter/flutter#160653) 2025-02-10 [email protected] Re-land #162644: Remove `--verbose` from devicelab task executions. (flutter/flutter#163017) 2025-02-10 [email protected] Include device lab version for how to run test (flutter/flutter#163010) 2025-02-10 [email protected] Change the default optimization level to `-O2` for wasm in release mode. (flutter/flutter#162917) 2025-02-10 [email protected] [web] robustify safaridriver launch sequence (flutter/flutter#162919) 2025-02-10 [email protected] Return more eagerly when toggling service extensions (flutter/flutter#162774) 2025-02-10 [email protected] Fix doc reference typos (flutter/flutter#162893) 2025-02-10 [email protected] Roll Skia from 180ed4fc263d to 8c377e8bedd2 (4 revisions) (flutter/flutter#162998) 2025-02-10 [email protected] FYI matanlurey (does not require review, but probably should) on dev/test infra. (flutter/flutter#162642) 2025-02-10 [email protected] [Impeller] rrect_blur: scale max radius clamp by transform (flutter/flutter#161238) ...

chunhtai force-pushed the issues/157204 branch from fcb31f0 to 1e34753 Compare February 5, 2025 23:34

kevmoo reviewed Feb 5, 2025

View reviewed changes

packages/flutter/lib/src/semantics/semantics.dart Outdated Show resolved Hide resolved

chunhtai force-pushed the issues/157204 branch from 1e34753 to 7fb60d5 Compare February 6, 2025 17:49

chunhtai requested review from hannah-hyj, kevmoo and yjbanov February 6, 2025 19:16

chunhtai marked this pull request as ready for review February 6, 2025 19:16

yjbanov reviewed Feb 6, 2025

View reviewed changes

chunhtai force-pushed the issues/157204 branch from 99f6a75 to 7eb9af4 Compare February 7, 2025 00:06

chunhtai requested a review from yjbanov February 10, 2025 19:34

chunhtai force-pushed the issues/157204 branch from 7eb9af4 to 2aced4b Compare February 10, 2025 19:35

yjbanov reviewed Feb 10, 2025

View reviewed changes

chunhtai requested a review from yjbanov February 10, 2025 22:55

yjbanov approved these changes Feb 10, 2025

View reviewed changes

chunhtai added 9 commits February 10, 2025 15:26

Adds dialog and alertdialog role

ba68727

add test

7184963

update

cd83f9e

update

aa9a7ae

update

0a151c2

Update

4929b12

addressing comments

a62b6f2

update

5316cf0

fix analyzer

a4e3a98

engine-flutter-autoroll mentioned this pull request Feb 12, 2025

Roll Flutter from 892f9c13a2bb to cdce7658473f (70 revisions) flutter/packages#8612

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 12, 2025

21471aa Adds dialog and alertdialog role (flutter/flutter#162692)

c2c53d7

engine-flutter-autoroll mentioned this pull request Feb 12, 2025

Roll Flutter from 892f9c13a2bb to e8f34a9eea90 (71 revisions) flutter/packages#8614

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 12, 2025

21471aa Adds dialog and alertdialog role (flutter/flutter#162692)

b8962e9

engine-flutter-autoroll mentioned this pull request Feb 12, 2025

Roll Flutter from 892f9c13a2bb to 90a4323cab4e (72 revisions) flutter/packages#8616

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 12, 2025

21471aa Adds dialog and alertdialog role (flutter/flutter#162692)

ef4a2e5

engine-flutter-autoroll mentioned this pull request Feb 12, 2025

Roll Flutter from 892f9c13a2bb to 09fdf159b911 (74 revisions) flutter/packages#8617

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 12, 2025

21471aa Adds dialog and alertdialog role (flutter/flutter#162692)

c4063c4

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025

21471aa Adds dialog and alertdialog role (flutter/flutter#162692)

11dca31

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025

21471aa Adds dialog and alertdialog role (flutter/flutter#162692)

57f2b14

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025

21471aa Adds dialog and alertdialog role (flutter/flutter#162692)

2ed55d8

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 14, 2025

21471aa Adds dialog and alertdialog role (flutter/flutter#162692)

78744b8

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 14, 2025

21471aa Adds dialog and alertdialog role (flutter/flutter#162692)

7ef4b2a

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 14, 2025

21471aa Adds dialog and alertdialog role (flutter/flutter#162692)

9c4e915

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 14, 2025

21471aa Adds dialog and alertdialog role (flutter/flutter#162692)

59efbfb

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 14, 2025

21471aa Adds dialog and alertdialog role (flutter/flutter#162692)

f2e820b

Piinks mentioned this pull request Feb 14, 2025

Flutter Notable Commits - 2025 #161460

Open

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 15, 2025

21471aa Adds dialog and alertdialog role (flutter/flutter#162692)

c640f10

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 15, 2025

21471aa Adds dialog and alertdialog role (flutter/flutter#162692)

253f3e4

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 16, 2025

21471aa Adds dialog and alertdialog role (flutter/flutter#162692)

94d4fe7

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 16, 2025

21471aa Adds dialog and alertdialog role (flutter/flutter#162692)

48e2b5f

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 17, 2025

21471aa Adds dialog and alertdialog role (flutter/flutter#162692)

8c57c64

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 17, 2025

21471aa Adds dialog and alertdialog role (flutter/flutter#162692)

3e3342b

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 17, 2025

21471aa Adds dialog and alertdialog role (flutter/flutter#162692)

4472d1f

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 20, 2025

21471aa Adds dialog and alertdialog role (flutter/flutter#162692)

fb8a893

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 20, 2025

21471aa Adds dialog and alertdialog role (flutter/flutter#162692)

e90baad

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 21, 2025

21471aa Adds dialog and alertdialog role (flutter/flutter#162692)

52e1469

Adds dialog and alertdialog role #162692

Adds dialog and alertdialog role #162692

Uh oh!

Conversation

chunhtai commented Feb 4, 2025

Pre-launch Checklist

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

yjbanov 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants