Add support for setting the heading level for web semantics (#97894)

[victorgalo]

This change adds a new property in Semantics widget that would take an integer value corresponding to the heading levels defined by the ARIA heading role. This is necessary in order to get proper accessibility and usability in a website for users who rely on screen readers and other assistive technologies.

Issue fixed by this PR: https://github.com/flutter/flutter/issues/97894

Framework part: https://github.com/flutter/flutter/pull/125771

Pre-launch Checklist

• [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
• [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities.
• [x] I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides].
• [x] I listed at least one issue that this PR fixes in the description above.
• [x] I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See [testing the engine] for instructions on writing and running engine tests.
• [x] I updated/added relevant documentation (doc comments with ///).
• [x] I signed the [CLA].
• [x] All existing and new tests are passing.

[victorgalo]

The build is failing because some integration tests are failing in the framework side, I need to integrate both changes together (framework and engine part) for integration tests to pass. Please, someone can tell me how I should deal with this?.

Maybe @yjbanov can shed some light?

[flutter-dashboard[bot]]

This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold.

[yjbanov]

It's a tough one. We might have to temporarily disable the test, wait for the engine change to roll, then enable it again.

[yjbanov]

Would you be willing to wait for https://github.com/flutter/engine/pull/43159 to land before landing this PR? Currently LabelAndValue is broken in that it steals the ARIA role attribute from more specific roles. After I land that, we can introduce a secondary RoleManager that would manage aria-level without touching the role attribute.

[yjbanov]

Update: https://github.com/flutter/engine/pull/43159 has landed.

[yjbanov]

Here's what I'd recommend doing given the framework established in https://github.com/flutter/engine/pull/43159.

• We create a new class Heading extends RoleManager. This role manager is responsible for the aria-level attribute.
• PrimaryRoleManager.withBasics constructor adds the Heading role manager if applicable. The Heading role is applicable if headingLevel != -1.

Alternatively, we can give the task of setting aria-level to LabelAndValue. I'm not sure if the two are orthogonal to each other 🤔

Let's keep LabelAndValue as is. In a separate PR we can fix the "header" vs "heading" vs "banner" situation.

[odrya12]

hi there is any progress with the last checks?

[victorgalo]

Here's what I'd recommend doing given the framework established in #43159.

• We create a new class Heading extends RoleManager. This role manager is responsible for the aria-level attribute.
• PrimaryRoleManager.withBasics constructor adds the Heading role manager if applicable. The Heading role is applicable if headingLevel != -1.

Alternatively, we can give the task of setting aria-level to LabelAndValue. I'm not sure if the two are orthogonal to each other 🤔

Let's keep LabelAndValue as is. In a separate PR we can fix the "header" vs "heading" vs "banner" situation.

Nice ideas!, thank you very much. I will create a Heading class extending RoleManager as you proposed. Sorry for my late response, I will try to implement it during this week.

[flutter-dashboard[bot]]

This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again.

[yjbanov]

This is getting very close to landing from the web standpoint. I added @chunhtai to check the mobile side of things.

[victorgalo]

This is very close to landing, I think, unless @chunhtai thinks otherwise. Thank you for cleaning up the PR!

Hi @yjbanov, I updated the PR with your latest suggestions, thank you!

[yjbanov]

The code LGTM, but the CI is not happy. There seems to be a mismatch between dart:ui API and the engine implementation?

[victorgalo]

The code LGTM, but the CI is not happy. There seems to be a mismatch between dart:ui API and the engine implementation?

Right, I forgot to add something to the dart:ui, I think now it's fixed.

[gianlucainnocente]

Hello, when do you plan to include this feature into Flutter? Thanks

[kevmoo]

@gianlucainnocente – we need to get CI green and this landed

[gianlucainnocente]

@gianlucainnocente – we need to get CI green and this landed

Hello @kevmoo do you have any plan for this feature?

[victorgalo]

@gianlucainnocente – we need to get CI green and this landed

Hi @kevmoo and @gianlucainnocente, to get CI green I need first this other PR to be landed: https://github.com/flutter/flutter/pull/135077

[FrancescoAbbondo]

Hello @victorgalo, can you give me a date (even indicative) for the release of this feature into Flutter? When the PR flutter/flutter#135077 will be approved? Thanks a lot

[gianlucainnocente]

@victorgalo Will this PR also add the support for the AccessibilityHeadingLevel attribute on iOS? (ref: https://developer.apple.com/documentation/swiftui/accessibilityheadinglevel) And what about android? Thank you

[victorgalo]

@victorgalo Will this PR also add the support for the AccessibilityHeadingLevel attribute on iOS? (ref: https://developer.apple.com/documentation/swiftui/accessibilityheadinglevel) And what about android? Thank you

Hi, not in this one, sorry. But it could be added in another PR after this one is landed.

[kevmoo]

https://github.com/flutter/flutter/pull/135077 landed – can we move forward here?

[victorgalo]

flutter/flutter#135077 landed – can we move forward here?

Hi, I was getting it ready but now some test in Linux Web Framework test suite is failing. Sorry but I need help to interpret the output, I can't see in the log details about what it is failing nor in the execution details or the stdout. Can anyone give me a tip on how to see what is failing?

[yjbanov]

Nice! Looks like an infra timeout. Can you rebase the PR? If it keeps happening we can pull in infra folks.

[auto-submit[bot]]

auto label is removed for flutter/engine/41435, due to This PR has not met approval requirements for merging. The PR author is not a member of flutter-hackers and needs 1 more review(s) in order to merge this PR.

• Merge guidelines: A PR needs at least one approved review if the author is already part of flutter-hackers or two member reviews if the author is not a flutter-hacker before re-applying the autosubmit label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

[victorgalo]

Nice! Looks like an infra timeout. Can you rebase the PR? If it keeps happening we can pull in infra folks.

Hi @yjbanov, I tried with rebase, but no luck with the infra tests :(

[chunhtai]

Hi @victorgalo can you fix the ci so that we can merge this pr?

[auto-submit[bot]]

auto label is removed for flutter/engine/41435, due to - The status or check suite Linux linux_web_engine has failed. Please fix the issues identified (or deflake) before re-applying this label.

[chunhtai]

Hi @victorgalo there is some analyzer warning

Analyzing web_ui...

info - lib/src/engine/semantics/semantics.dart:1659:27 - This case is covered by the previous cases. Try removing the case clause, or restructuring the preceding patterns. - unreachable_switch_case

1 issue found.

Since this have been open for a while, I can also take over if you want. let me know how you want to proceed

[auto-submit[bot]]

auto label is removed for flutter/engine/41435, due to - The status or check suite Linux Web Framework tests has failed. Please fix the issues identified (or deflake) before re-applying this label.

[victorgalo]

Hi @victorgalo there is some analyzer warning

Analyzing web_ui...

info - lib/src/engine/semantics/semantics.dart:1659:27 - This case is covered by the previous cases. Try removing the case clause, or restructuring the preceding patterns. - unreachable_switch_case

1 issue found.

Since this have been open for a while, I can also take over if you want. let me know how you want to proceed

Hi @chunhtai. Oh right, sorry I missed that warning after the rebase I did. Now it's fixed and I'm at the point where I was stuck, can you please help me understand why it's failing now?, I'd rather finish it myself but may need some guidance on this last step if you don't mind, sorry for the trouble!

Thank you!