Skip to content

Commit

Permalink
Revert "[web:a11y] make header a proper <header> (flutter#55747)" (fl…
Browse files Browse the repository at this point in the history
…utter#55993)

This reverts commit 2fbb0c1.

This broke a customer: flutter#55747 (comment)
  • Loading branch information
yjbanov authored Oct 21, 2024
1 parent 4e6405e commit d302cc9
Show file tree
Hide file tree
Showing 6 changed files with 20 additions and 77 deletions.
2 changes: 0 additions & 2 deletions ci/licenses_golden/licenses_flutter
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
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
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

This file was deleted.

35 changes: 10 additions & 25 deletions lib/web_ui/lib/src/engine/semantics/semantics.dart
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 @@ -692,18 +688,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 @@ -1271,24 +1272,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 @@ -1702,8 +1690,6 @@ class SemanticsObject {
return SemanticRoleKind.route;
} else if (isLink) {
return SemanticRoleKind.link;
} else if (isHeader) {
return SemanticRoleKind.header;
} else {
return SemanticRoleKind.generic;
}
Expand All @@ -1721,7 +1707,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
Original file line number Diff line number Diff line change
Expand Up @@ -739,7 +739,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 @@ -755,13 +755,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 @@ -785,7 +791,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

0 comments on commit d302cc9

Please sign in to comment.