-
Notifications
You must be signed in to change notification settings - Fork 6k
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.
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
Add support for setting the heading level for web semantics (#97894) #41435
Changes from 32 commits
a341b26
eff0206
7f8ef2c
9029d63
260d4bd
baea4c1
bf0fe7f
b773f81
90de9fc
c9ac350
c8ccd31
0241f07
6298a5f
36c5fd6
30153e0
5460388
a526070
dceef3c
3a6e45d
b9bfb88
664c1e4
e6a8c6a
470f67f
a56c398
b31bd9f
8305e4b
d860f21
a16a8f7
70efe89
ddeaefc
bcd3205
9d25b32
268e31c
a7bb630
0f72806
79e4728
4b69d9f
c83039b
b351c1e
5705912
89ef536
e60c0a7
bb787e0
7199da1
76b65f6
ee38434
41f77aa
e38e3f3
1e570fb
ac1ec5e
0b9efa6
4a54812
6d8d511
2586dbc
fc0a3a5
ca2d289
5e05f24
9231840
c52b545
33a246c
1b0e241
e918e20
4383464
5186055
af5e70a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -790,6 +790,18 @@ abstract class SemanticsUpdateBuilder { | |
/// z-direction starting at `elevation`. Basically, in the z-direction the | ||
/// node starts at `elevation` above the parent and ends at `elevation` + | ||
/// `thickness` above the parent. | ||
/// | ||
/// The `headingLevel` describes that this node is a heading and the hierarchy | ||
/// level this node represents as a heading. A value of -1 indicates that this | ||
/// node is not a heading. A value of 1 or greater indicates that this node is | ||
/// a heading at the specified level. The valid value range is from 1 to 6, | ||
/// inclusive. This attribute is only used for Web platform, and it will have | ||
/// no effect on other platforms. | ||
/// | ||
/// See also: | ||
/// | ||
/// * https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/heading_role | ||
/// * https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-level | ||
void updateNode({ | ||
required int id, | ||
required int flags, | ||
|
@@ -825,6 +837,7 @@ abstract class SemanticsUpdateBuilder { | |
required Int32List childrenInTraversalOrder, | ||
required Int32List childrenInHitTestOrder, | ||
required Int32List additionalActions, | ||
required int headingLevel, | ||
}); | ||
|
||
/// Update the custom semantics action associated with the given `id`. | ||
|
@@ -896,8 +909,13 @@ base class _NativeSemanticsUpdateBuilder extends NativeFieldWrapperClass1 implem | |
required Int32List childrenInTraversalOrder, | ||
required Int32List childrenInHitTestOrder, | ||
required Int32List additionalActions, | ||
required int headingLevel, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same |
||
}) { | ||
assert(_matrix4IsValid(transform)); | ||
assert ( | ||
headingLevel == -1 || (headingLevel >= 1 && headingLevel <= 6), | ||
'Heading level must be between 1 and 6, or -1 to indicate that this node is not a heading.' | ||
); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we also want to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right!, done There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This probably can't be 0 as well? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, done |
||
_updateNode( | ||
id, | ||
flags, | ||
|
@@ -936,6 +954,7 @@ base class _NativeSemanticsUpdateBuilder extends NativeFieldWrapperClass1 implem | |
childrenInTraversalOrder, | ||
childrenInHitTestOrder, | ||
additionalActions, | ||
headingLevel, | ||
); | ||
} | ||
@Native< | ||
|
@@ -976,7 +995,8 @@ base class _NativeSemanticsUpdateBuilder extends NativeFieldWrapperClass1 implem | |
Handle, | ||
Handle, | ||
Handle, | ||
Handle)>(symbol: 'SemanticsUpdateBuilder::updateNode') | ||
Handle, | ||
Int32)>(symbol: 'SemanticsUpdateBuilder::updateNode') | ||
external void _updateNode( | ||
int id, | ||
int flags, | ||
|
@@ -1349,7 +1369,8 @@ base class _NativeSemanticsUpdateBuilderNew extends NativeFieldWrapperClass1 imp | |
Float64List transform, | ||
Int32List childrenInTraversalOrder, | ||
Int32List childrenInHitTestOrder, | ||
Int32List additionalActions); | ||
Int32List additionalAction, | ||
int headingLevel); | ||
|
||
@override | ||
void updateCustomAction({required int id, String? label, String? hint, int overrideId = -1}) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -283,6 +283,7 @@ class SemanticsUpdateBuilderNew { | |
required Int32List childrenInTraversalOrder, | ||
required Int32List childrenInHitTestOrder, | ||
required Int32List additionalActions, | ||
required int headingLevel, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same here |
||
}) { | ||
if (transform.length != 16) { | ||
throw ArgumentError('transform argument must have 16 entries.'); | ||
|
@@ -417,6 +418,7 @@ class SemanticsUpdateBuilder { | |
childrenInHitTestOrder: childrenInHitTestOrder, | ||
additionalActions: additionalActions, | ||
platformViewId: platformViewId, | ||
headingLevel: headingLevel, | ||
)); | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
// Copyright 2013 The Flutter Authors. All rights reserved. | ||
// Use of this source code is governed by a BSD-style license that can be | ||
// found in the LICENSE file. | ||
|
||
import '../dom.dart'; | ||
import 'semantics.dart'; | ||
|
||
/// Renders semantics objects as headings with the corresponding | ||
/// level (h1 ... h6). | ||
class Heading extends PrimaryRoleManager { | ||
Heading(SemanticsObject semanticsObject) | ||
: super.withBasics(PrimaryRole.heading, semanticsObject) { | ||
addHeadingRole(); | ||
} | ||
|
||
static const int defaultHeadingLevel = 1; | ||
|
||
@override | ||
void update() { | ||
super.update(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can return early if the heading level field is not dirty. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, I added it. |
||
|
||
if (!semanticsObject.isHeadingLevelDirty) { | ||
return; | ||
} | ||
|
||
if (semanticsObject.headingLevel != -1) { | ||
addHeadingLevel(semanticsObject.headingLevel); | ||
} else { | ||
addHeadingLevel(defaultHeadingLevel); | ||
} | ||
} | ||
|
||
void addHeadingRole() { | ||
setAriaRole('heading'); | ||
} | ||
|
||
void addHeadingLevel(int headingLevel) { | ||
semanticsObject.element.setAttribute('aria-level', headingLevel); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -660,6 +660,28 @@ void _testHeader() { | |
owner().updateSemantics(builder.build()); | ||
expectSemanticsTree(owner(), ''' | ||
<sem role="group" aria-label="Header of the page" style="$rootSemanticStyle"><sem-c><sem></sem></sem-c></sem> | ||
'''); | ||
|
||
semantics().semanticsEnabled = false; | ||
}); | ||
|
||
test('renders aria-level tag for headings with heading level', () { | ||
semantics() | ||
..debugOverrideTimestampFunction(() => _testTime) | ||
..semanticsEnabled = true; | ||
|
||
final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); | ||
updateNode( | ||
builder, | ||
headingLevel: 2, | ||
label: 'Heading of the page', | ||
transform: Matrix4.identity().toFloat64(), | ||
rect: const ui.Rect.fromLTRB(0, 0, 100, 50), | ||
); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's also test that we can update it and clear There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hey @yjbanov, I'd say i's not a meaningful state, aria-level is required for "heading" role. Do you think I still should add it?. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nope, if it's not an expected state, we don't need to test it. |
||
|
||
semantics().updateSemantics(builder.build()); | ||
expectSemanticsTree(''' | ||
<sem role="heading" aria-label="Heading of the page" aria-level="2" style="$rootSemanticStyle"></sem> | ||
'''); | ||
|
||
semantics().semanticsEnabled = false; | ||
|
@@ -3070,6 +3092,7 @@ void updateNode( | |
Int32List? childrenInTraversalOrder, | ||
Int32List? childrenInHitTestOrder, | ||
Int32List? additionalActions, | ||
int headingLevel = -1, | ||
}) { | ||
transform ??= Float64List.fromList(Matrix4.identity().storage); | ||
childrenInTraversalOrder ??= Int32List(0); | ||
|
@@ -3110,6 +3133,7 @@ void updateNode( | |
childrenInTraversalOrder: childrenInTraversalOrder, | ||
childrenInHitTestOrder: childrenInHitTestOrder, | ||
additionalActions: additionalActions, | ||
headingLevel: headingLevel, | ||
); | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's make this not required and give it a default value of 0