Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "[web:a11y] make header a proper <header> (#55747)" #55993

Merged
merged 1 commit into from
Oct 21, 2024
Merged

Revert "[web:a11y] make header a proper <header> (#55747)" #55993

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions ci/licenses_golden/licenses_flutter
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43886,7 +43886,6 @@ ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/semantics.dart + ../../../flu
ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/semantics/accessibility.dart + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/semantics/checkable.dart + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/semantics/focusable.dart + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/semantics/header.dart + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/semantics/heading.dart + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/semantics/image.dart + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/semantics/incrementable.dart + ../../../flutter/LICENSE
Expand Down Expand Up @@ -46765,7 +46764,6 @@ FILE: ../../../flutter/lib/web_ui/lib/src/engine/semantics.dart
FILE: ../../../flutter/lib/web_ui/lib/src/engine/semantics/accessibility.dart
FILE: ../../../flutter/lib/web_ui/lib/src/engine/semantics/checkable.dart
FILE: ../../../flutter/lib/web_ui/lib/src/engine/semantics/focusable.dart
FILE: ../../../flutter/lib/web_ui/lib/src/engine/semantics/header.dart
FILE: ../../../flutter/lib/web_ui/lib/src/engine/semantics/heading.dart
FILE: ../../../flutter/lib/web_ui/lib/src/engine/semantics/image.dart
FILE: ../../../flutter/lib/web_ui/lib/src/engine/semantics/incrementable.dart
Expand Down
1 change: 0 additions & 1 deletion lib/web_ui/lib/src/engine.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,6 @@ export 'engine/scene_view.dart';
export 'engine/semantics/accessibility.dart';
export 'engine/semantics/checkable.dart';
export 'engine/semantics/focusable.dart';
export 'engine/semantics/header.dart';
export 'engine/semantics/heading.dart';
export 'engine/semantics/image.dart';
export 'engine/semantics/incrementable.dart';
Expand Down
1 change: 0 additions & 1 deletion lib/web_ui/lib/src/engine/semantics.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
export 'semantics/accessibility.dart';
export 'semantics/checkable.dart';
export 'semantics/focusable.dart';
export 'semantics/header.dart';
export 'semantics/heading.dart';
export 'semantics/image.dart';
export 'semantics/incrementable.dart';
Expand Down
44 changes: 0 additions & 44 deletions lib/web_ui/lib/src/engine/semantics/header.dart
View file
Open in desktop

This file was deleted.

35 changes: 10 additions & 25 deletions lib/web_ui/lib/src/engine/semantics/semantics.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import '../window.dart';
import 'accessibility.dart';
import 'checkable.dart';
import 'focusable.dart';
import 'header.dart';
import 'heading.dart';
import 'image.dart';
import 'incrementable.dart';
Expand Down Expand Up @@ -397,17 +396,14 @@ enum SemanticRoleKind {
/// The node's role is to host a platform view.
platformView,

/// Contains a link.
link,

/// Denotes a header.
header,

/// A role used when a more specific role cannot be assigend to
/// a [SemanticsObject].
///
/// Provides a label or a value.
generic,

/// Contains a link.
link,
}

/// Responsible for setting the `role` ARIA attribute, for attaching
Expand Down Expand Up @@ -681,18 +677,23 @@ final class GenericRole extends SemanticRole {
return;
}

// Assign one of two roles to the element: group or text.
// Assign one of three roles to the element: group, heading, text.
//
// - "group" is used when the node has children, irrespective of whether the
// node is marked as a header or not. This is because marking a group
// as a "heading" will prevent the AT from reaching its children.
// - "heading" is used when the framework explicitly marks the node as a
// heading and the node does not have children.
// - If a node has a label and no children, assume is a paragraph of text.
// In HTML text has no ARIA role. It's just a DOM node with text inside
// it. Previously, role="text" was used, but it was only supported by
// Safari, and it was removed starting Safari 17.
if (semanticsObject.hasChildren) {
labelAndValue!.preferredRepresentation = LabelRepresentation.ariaLabel;
setAriaRole('group');
} else if (semanticsObject.hasFlag(ui.SemanticsFlag.isHeader)) {
labelAndValue!.preferredRepresentation = LabelRepresentation.domText;
setAriaRole('heading');
} else {
labelAndValue!.preferredRepresentation = LabelRepresentation.sizedSpan;
removeAttribute('role');
Expand Down Expand Up @@ -1260,24 +1261,11 @@ class SemanticsObject {
bool get isTextField => hasFlag(ui.SemanticsFlag.isTextField);

/// Whether this object represents a heading element.
///
/// Typically, a heading is a prominent piece of text that describes what the
/// rest of the screen or page is about.
///
/// Not to be confused with [isHeader].
bool get isHeading => headingLevel != 0;

/// Whether this object represents an interactive link.
/// Whether this object represents an editable text field.
bool get isLink => hasFlag(ui.SemanticsFlag.isLink);

/// Whether this object represents a header.
///
/// A header is a group of widgets that introduce the content of the screen
/// or a page.
///
/// Not to be confused with [isHeading].
bool get isHeader => hasFlag(ui.SemanticsFlag.isHeader);

/// Whether this object needs screen readers attention right away.
bool get isLiveRegion =>
hasFlag(ui.SemanticsFlag.isLiveRegion) &&
Expand Down Expand Up @@ -1691,8 +1679,6 @@ class SemanticsObject {
return SemanticRoleKind.route;
} else if (isLink) {
return SemanticRoleKind.link;
} else if (isHeader) {
return SemanticRoleKind.header;
} else {
return SemanticRoleKind.generic;
}
Expand All @@ -1710,7 +1696,6 @@ class SemanticsObject {
SemanticRoleKind.platformView => SemanticPlatformView(this),
SemanticRoleKind.link => SemanticLink(this),
SemanticRoleKind.heading => SemanticHeading(this),
SemanticRoleKind.header => SemanticHeader(this),
SemanticRoleKind.generic => GenericRole(this),
};
}
Expand Down
14 changes: 10 additions & 4 deletions lib/web_ui/test/engine/semantics/semantics_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -736,7 +736,7 @@ class MockSemanticsEnabler implements SemanticsEnabler {
}

void _testHeader() {
test('renders a header with a label and uses a sized span for label', () {
test('renders heading role for headers', () {
semantics()
..debugOverrideTimestampFunction(() => _testTime)
..semanticsEnabled = true;
Expand All @@ -752,13 +752,19 @@ void _testHeader() {

owner().updateSemantics(builder.build());
expectSemanticsTree(owner(), '''
<header><span>Header of the page</span></header>
<sem role="heading">Header of the page</sem>
''');

semantics().semanticsEnabled = false;
});

test('renders a header with children and uses aria-label', () {
// When a header has child elements, role="heading" prevents AT from reaching
// child elements. To fix that role="group" is used, even though that causes
// the heading to not be announced as a heading. If the app really needs the
// heading to be announced as a heading, the developer can restructure the UI
// such that the heading is not a parent node, but a side-note, e.g. preceding
// the child list.
test('uses group role for headers when children are present', () {
semantics()
..debugOverrideTimestampFunction(() => _testTime)
..semanticsEnabled = true;
Expand All @@ -782,7 +788,7 @@ void _testHeader() {

owner().updateSemantics(builder.build());
expectSemanticsTree(owner(), '''
<header aria-label="Header of the page"><sem-c><sem></sem></sem-c></header>
<sem role="group" aria-label="Header of the page"><sem-c><sem></sem></sem-c></sem>
''');

semantics().semanticsEnabled = false;
Expand Down