Skip to content

Commit

Permalink
address comments; improve semantic tests
Browse files Browse the repository at this point in the history
  • Loading branch information
yjbanov committed Aug 2, 2023
1 parent e834fe6 commit 07cfaf1
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 40 deletions.
10 changes: 2 additions & 8 deletions lib/web_ui/lib/src/engine/pointer_binding.dart
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ class ClickDebouncer {
}

if (isListening) {
// There's a pending queue of pointer events. Prefer sendind the tap action
// There's a pending queue of pointer events. Prefer sending the tap action
// instead of pointer events, because the pointer events may not land on the
// combined semantic node and miss the click/tap.
final DebounceState state = _state!;
Expand Down Expand Up @@ -415,15 +415,9 @@ class ClickDebouncer {
///
/// This object can be used as if it was just initialized.
void reset() {
final DebounceState? state = _state;
_state?.timer.cancel();
_state = null;
_lastFlushedPointerUpTimeStamp = null;

if (state == null) {
return;
}

state.timer.cancel();
}
}

Expand Down
4 changes: 4 additions & 0 deletions lib/web_ui/lib/src/engine/semantics/tappable.dart
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ class Tappable extends RoleManager {
}

void _updateAttribute() {
// The `flt-tappable` attribute marks the element for the ClickDebouncer to
// to know that it should debounce click events on this element. The
// contract is that the element that has this attribute is also the element
// that receives pointer and "click" events.
if (_isListening) {
semanticsObject.element.setAttribute('flt-tappable', '');
} else {
Expand Down
20 changes: 13 additions & 7 deletions lib/web_ui/test/common/matchers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -293,17 +293,23 @@ String canonicalizeHtml(
html_package.Element.tag(replacementTag);

if (mode != HtmlComparisonMode.noAttributes) {
original.attributes.forEach((dynamic name, String value) {
if (name is! String) {
throw ArgumentError('"$name" should be String but was ${name.runtimeType}.');
}
// Sort the attributes so tests are not sensitive to their order, which
// does not matter in terms of functionality.
final List<String> attributeNames = original.attributes.keys.cast<String>().toList();
attributeNames.sort();
for (final String name in attributeNames) {
final String value = original.attributes[name]!;
if (name == 'style') {
return;
// The style attribute is handled separately because it contains substructure.
continue;
}
if (name.startsWith('aria-')) {

// These are the only attributes we're interested in testing. This list
// can change over time.
if (name.startsWith('aria-') || name.startsWith('flt-') || name == 'role') {
replacement.attributes[name] = value;
}
});
}

if (original.attributes.containsKey('style')) {
final String styleValue = original.attributes['style']!;
Expand Down
4 changes: 2 additions & 2 deletions lib/web_ui/test/engine/pointer_binding_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3088,7 +3088,7 @@ void testMain() {
},
);

group('$ClickDebouncer', () {
group('ClickDebouncer', () {
_testClickDebouncer();
});
}
Expand Down Expand Up @@ -3159,7 +3159,7 @@ void _testClickDebouncer() {
expect(binding.clickDebouncer.isDebouncing, false);

// This test DOM element is missing the `flt-tappable` attribute on purpose
// so that the debouncer does not debounce events and simply let's
// so that the debouncer does not debounce events and simply lets
// everything through.
final DomElement testElement = createDomElement('flt-semantics');
flutterViewEmbedder.semanticsHostElement!.appendChild(testElement);
Expand Down
46 changes: 23 additions & 23 deletions lib/web_ui/test/engine/semantics/semantics_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ void _testEngineSemanticsOwner() {
expectSemanticsTree('''
<sem style="$rootSemanticStyle">
<sem-c>
<sem aria-label="Hello"></sem>
<sem role="text" aria-label="Hello"></sem>
</sem-c>
</sem>''');

Expand All @@ -387,7 +387,7 @@ void _testEngineSemanticsOwner() {
expectSemanticsTree('''
<sem style="$rootSemanticStyle">
<sem-c>
<sem aria-label="World"></sem>
<sem role="text" aria-label="World"></sem>
</sem-c>
</sem>''');

Expand All @@ -397,7 +397,7 @@ void _testEngineSemanticsOwner() {
expectSemanticsTree('''
<sem style="$rootSemanticStyle">
<sem-c>
<sem></sem>
<sem role="text"></sem>
</sem-c>
</sem>''');

Expand Down Expand Up @@ -430,7 +430,7 @@ void _testEngineSemanticsOwner() {
expectSemanticsTree('''
<sem style="$rootSemanticStyle">
<sem-c>
<sem aria-label="tooltip\nHello"></sem>
<sem role="text" aria-label="tooltip\nHello"></sem>
</sem-c>
</sem>''');

Expand All @@ -440,7 +440,7 @@ void _testEngineSemanticsOwner() {
expectSemanticsTree('''
<sem style="$rootSemanticStyle">
<sem-c>
<sem></sem>
<sem role="text"></sem>
</sem-c>
</sem>''');

Expand Down Expand Up @@ -1388,7 +1388,7 @@ void _testIncrementables() {
semantics().updateSemantics(builder.build());
expectSemanticsTree('''
<sem style="$rootSemanticStyle">
<input aria-valuenow="1" aria-valuetext="d" aria-valuemax="1" aria-valuemin="1">
<input role="slider" aria-valuenow="1" aria-valuetext="d" aria-valuemax="1" aria-valuemin="1">
</sem>''');

final SemanticsObject node = semantics().debugSemanticsTree![0]!;
Expand Down Expand Up @@ -1421,7 +1421,7 @@ void _testIncrementables() {
semantics().updateSemantics(builder.build());
expectSemanticsTree('''
<sem style="$rootSemanticStyle">
<input aria-valuenow="1" aria-valuetext="d" aria-valuemax="2" aria-valuemin="1">
<input role="slider" aria-valuenow="1" aria-valuetext="d" aria-valuemax="2" aria-valuemin="1">
</sem>''');

final DomHTMLInputElement input =
Expand Down Expand Up @@ -1454,7 +1454,7 @@ void _testIncrementables() {
semantics().updateSemantics(builder.build());
expectSemanticsTree('''
<sem style="$rootSemanticStyle">
<input aria-valuenow="1" aria-valuetext="d" aria-valuemax="1" aria-valuemin="0">
<input role="slider" aria-valuenow="1" aria-valuetext="d" aria-valuemax="1" aria-valuemin="0">
</sem>''');

final DomHTMLInputElement input =
Expand Down Expand Up @@ -1489,7 +1489,7 @@ void _testIncrementables() {
semantics().updateSemantics(builder.build());
expectSemanticsTree('''
<sem style="$rootSemanticStyle">
<input aria-valuenow="1" aria-valuetext="d" aria-valuemax="2" aria-valuemin="0">
<input role="slider" aria-valuenow="1" aria-valuetext="d" aria-valuemax="2" aria-valuemin="0">
</sem>''');

semantics().semanticsEnabled = false;
Expand Down Expand Up @@ -1632,7 +1632,7 @@ void _testCheckables() {

semantics().updateSemantics(builder.build());
expectSemanticsTree('''
<sem aria-label="test label" role="switch" aria-checked="true" style="$rootSemanticStyle"></sem>
<sem aria-label="test label" flt-tappable role="switch" aria-checked="true" style="$rootSemanticStyle"></sem>
''');

final SemanticsObject node = semantics().debugSemanticsTree![0]!;
Expand Down Expand Up @@ -1690,7 +1690,7 @@ void _testCheckables() {

semantics().updateSemantics(builder.build());
expectSemanticsTree('''
<sem role="switch" aria-checked="false" style="$rootSemanticStyle"></sem>
<sem role="switch" flt-tappable aria-checked="false" style="$rootSemanticStyle"></sem>
''');

semantics().semanticsEnabled = false;
Expand All @@ -1716,7 +1716,7 @@ void _testCheckables() {

semantics().updateSemantics(builder.build());
expectSemanticsTree('''
<sem role="checkbox" aria-checked="true" style="$rootSemanticStyle"></sem>
<sem role="checkbox" flt-tappable aria-checked="true" style="$rootSemanticStyle"></sem>
''');

semantics().semanticsEnabled = false;
Expand Down Expand Up @@ -1766,7 +1766,7 @@ void _testCheckables() {

semantics().updateSemantics(builder.build());
expectSemanticsTree('''
<sem role="checkbox" aria-checked="false" style="$rootSemanticStyle"></sem>
<sem role="checkbox" flt-tappable aria-checked="false" style="$rootSemanticStyle"></sem>
''');

semantics().semanticsEnabled = false;
Expand All @@ -1793,7 +1793,7 @@ void _testCheckables() {

semantics().updateSemantics(builder.build());
expectSemanticsTree('''
<sem role="radio" aria-checked="true" style="$rootSemanticStyle"></sem>
<sem role="radio" flt-tappable aria-checked="true" style="$rootSemanticStyle"></sem>
''');

semantics().semanticsEnabled = false;
Expand Down Expand Up @@ -1845,7 +1845,7 @@ void _testCheckables() {

semantics().updateSemantics(builder.build());
expectSemanticsTree('''
<sem role="radio" aria-checked="false" style="$rootSemanticStyle"></sem>
<sem role="radio" flt-tappable aria-checked="false" style="$rootSemanticStyle"></sem>
''');

semantics().semanticsEnabled = false;
Expand Down Expand Up @@ -1918,7 +1918,7 @@ void _testTappable() {
tester.apply();

expectSemanticsTree('''
<sem role="button" style="$rootSemanticStyle"></sem>
<sem role="button" flt-tappable style="$rootSemanticStyle"></sem>
''');

final SemanticsObject node = semantics().debugSemanticsTree![0]!;
Expand Down Expand Up @@ -1979,14 +1979,14 @@ void _testTappable() {
'<sem role="button" aria-disabled="true" style="$rootSemanticStyle"></sem>');

updateTappable(enabled: true);
expectSemanticsTree('<sem role="button" style="$rootSemanticStyle"></sem>');
expectSemanticsTree('<sem role="button" flt-tappable style="$rootSemanticStyle"></sem>');

updateTappable(enabled: false);
expectSemanticsTree(
'<sem role="button" aria-disabled="true" style="$rootSemanticStyle"></sem>');

updateTappable(enabled: true);
expectSemanticsTree('<sem role="button" style="$rootSemanticStyle"></sem>');
expectSemanticsTree('<sem role="button" flt-tappable style="$rootSemanticStyle"></sem>');

semantics().semanticsEnabled = false;
});
Expand Down Expand Up @@ -2623,11 +2623,11 @@ void _testDialog() {
tester.apply();

expectSemanticsTree('''
<sem aria-describedby="flt-semantic-node-2" style="$rootSemanticStyle">
<sem role="dialog" aria-describedby="flt-semantic-node-2" style="$rootSemanticStyle">
<sem-c>
<sem>
<sem-c>
<sem aria-label="$label"></sem>
<sem role="text" aria-label="$label"></sem>
</sem-c>
</sem>
</sem-c>
Expand Down Expand Up @@ -2716,7 +2716,7 @@ void _testDialog() {
<sem-c>
<sem>
<sem-c>
<sem aria-label="Hello"></sem>
<sem role="text" aria-label="Hello"></sem>
</sem-c>
</sem>
</sem-c>
Expand Down Expand Up @@ -2853,9 +2853,9 @@ void _testFocusable() {
}

expectSemanticsTree('''
<sem role="group" style="$rootSemanticStyle">
<sem style="$rootSemanticStyle">
<sem-c>
<sem aria-label="focusable text"></sem>
<sem role="text" aria-label="focusable text"></sem>
</sem-c>
</sem>
''');
Expand Down
21 changes: 21 additions & 0 deletions lib/web_ui/test/engine/semantics/semantics_tester.dart
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,27 @@ class SemanticsTester {
/// Verifies the HTML structure of the current semantics tree.
void expectSemanticsTree(String semanticsHtml) {
const List<String> ignoredAttributes = <String>['pointer-events'];
// print('\n============================================================================');
// print('''
// semanticsHtml:
// $semanticsHtml
// '''.trim());
// print('----------------------------------------------------------------------------');
// print('''
// canonicalizeHtml(semanticsHtml):
// ${canonicalizeHtml(semanticsHtml)}
// '''.trim());
// print('----------------------------------------------------------------------------');
// print('''
// canonicalizeHtml(appHostNode.querySelector('flt-semantics')!.outerHTML!, ignoredAttributes: ignoredAttributes):
// ${canonicalizeHtml(appHostNode.querySelector('flt-semantics')!.outerHTML!, ignoredAttributes: ignoredAttributes)}
// '''.trim());
// print('''
// appHostNode.querySelector('flt-semantics')!.outerHTML!:
// ${appHostNode.querySelector('flt-semantics')!.outerHTML!}
// '''.trim());
// print('============================================================================\n');

expect(
canonicalizeHtml(appHostNode.querySelector('flt-semantics')!.outerHTML!, ignoredAttributes: ignoredAttributes),
canonicalizeHtml(semanticsHtml),
Expand Down

0 comments on commit 07cfaf1

Please sign in to comment.