Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@yjbanov
Copy link
Contributor

@yjbanov yjbanov commented Jul 2, 2024

Switch to using semantic heading tags (h1, h2, etc).

Fix missing secondary roles: focus, live region, route name, and label.
Improves indexability (flutter/flutter#46789)

@yjbanov yjbanov requested a review from chunhtai July 2, 2024 23:09
@github-actions github-actions bot added the platform-web Code specifically for the web engine label Jul 2, 2024
///
/// Normally, heading elements are not focusable as they do not receive
/// keyboard input. However, when a route is pushed (e.g. a dialog pops up),
/// the it may be desirable to move the screen reader focus to the heading
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: the -> then

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

void addHeadingRole() {
setAriaRole('heading');
}
DomElement createElement() => createDomElement('h${semanticsObject.headingLevel}');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if the headingLevel changes? Does the update automatically create a new element and call this method?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heading level should not change. It confuses screen readers. Web crawlers snapshot the heading at its default value too, so changing has no effect either. We could issue a warning, but I'd rather such warnings are issued by the framework, and the engine will just do a graceful degradation of functionality (in this case the heading level stays the same forever).

@yjbanov yjbanov requested a review from harryterkelsen July 3, 2024 21:55
Copy link
Contributor

@harryterkelsen harryterkelsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@yjbanov yjbanov added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 5, 2024
@auto-submit auto-submit bot merged commit c6cb09d into flutter:main Jul 5, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 5, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 5, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Jul 5, 2024
flutter/engine@4ee09d3...e6b0969

2024-07-05 skia-flutter-autoroll@skia.org Roll Skia from 4a27ce0f92d4 to 172eb4ee4cb3 (1 revision) (flutter/engine#53740)
2024-07-05 yjbanov@google.com [web] use semantic tags for headings (h1, h2, etc), and fix missing secondary roles (flutter/engine#53703)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC aaclarke@google.com,rmistry@google.com,zra@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
victorsanni pushed a commit to victorsanni/flutter that referenced this pull request Jul 8, 2024
…1352)

flutter/engine@4ee09d3...e6b0969

2024-07-05 skia-flutter-autoroll@skia.org Roll Skia from 4a27ce0f92d4 to 172eb4ee4cb3 (1 revision) (flutter/engine#53740)
2024-07-05 yjbanov@google.com [web] use semantic tags for headings (h1, h2, etc), and fix missing secondary roles (flutter/engine#53703)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC aaclarke@google.com,rmistry@google.com,zra@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

autosubmit Merge PR when tree becomes green via auto submit App platform-web Code specifically for the web engine

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants