Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Use announce function in live region #38084

Merged
auto-submit merged 4 commits into from
Dec 7, 2022
Merged

Use announce function in live region #38084

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ee03ee7
use announce function in live region
nbayati Dec 5, 2022
11d1548
unit tests
nbayati Dec 5, 2022
86f5b7c
createtouch
nbayati Dec 6, 2022
7b6fe94
throw an error for unimplemented functions in the mock class
nbayati Dec 6, 2022
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
Prev Previous commit
Next Next commit
unit tests
  • Loading branch information
@nbayati
nbayati committed Dec 5, 2022
commit 11d15485dddb3bf2ce6b1d4ed36534f6bfdf0bf1
12 changes: 6 additions & 6 deletions lib/web_ui/lib/src/engine/semantics/live_region.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,24 +17,24 @@ class LiveRegion extends RoleManager {
LiveRegion(SemanticsObject semanticsObject)
: super(Role.labelAndValue, semanticsObject);

String? lastAnnouncement;
String? _lastAnnouncement;

@override
void update() {
// Avoid announcing the same message over and over.
Copy link
Contributor

@yjbanov yjbanov Dec 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the logic that protects us from announcing the same message again is missing a test. It can be tested by clearing the contents of ariaLiveElementFor(Assertiveness.polite) after the first announcement, issuing an update with the same message, and verifying that the element remains empty.

Copy link
Contributor Author

@nbayati nbayati Dec 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on our conversation, I implemented a mock class for AccessibilityAnnouncements and added this test case.

if (lastAnnouncement != semanticsObject.label)
if (_lastAnnouncement != semanticsObject.label)
{
Copy link
Contributor

@yjbanov yjbanov Dec 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: leave the open brace on the previous line

Copy link
Contributor Author

@nbayati nbayati Dec 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

lastAnnouncement = semanticsObject.label;
if (lastAnnouncement != null) {
_lastAnnouncement = semanticsObject.label;
if (_lastAnnouncement != null) {
accessibilityAnnouncements.announce(
lastAnnouncement! , Assertiveness.polite
_lastAnnouncement! , Assertiveness.polite
);
}
}
}

void _cleanupDom() {
lastAnnouncement = null;
_lastAnnouncement = null;
Copy link
Contributor

@yjbanov yjbanov Dec 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_lastAnnouncement is not a DOM element. It will be garbage-collected automatically. I think we can remove _cleanupDom and the dispose methods, and save a few bytes in payload size.

Copy link
Contributor Author

@nbayati nbayati Dec 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I have to keep dispose though because otherwise it throws an error about it not being implemented.

}

@override
Expand Down
13 changes: 4 additions & 9 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 @@ -1744,7 +1744,7 @@ void _testImage() {
}

void _testLiveRegion() {
test('renders a live region if there is a label', () async {
test('announces the label after an update', () async {
semantics()
..debugOverrideTimestampFunction(() => _testTime)
..semanticsEnabled = true;
Expand All @@ -1759,14 +1759,11 @@ void _testLiveRegion() {
);
semantics().updateSemantics(builder.build());

expectSemanticsTree('''
<sem aria-label="This is a snackbar" aria-live="polite" style="$rootSemanticStyle"></sem>
''');

expect(accessibilityAnnouncements.ariaLiveElementFor(Assertiveness.polite).text, 'This is a snackbar');
semantics().semanticsEnabled = false;
});

test('does not render a live region if there is no label', () async {
test('does not announce anything if there is no label', () async {
semantics()
..debugOverrideTimestampFunction(() => _testTime)
..semanticsEnabled = true;
Expand All @@ -1780,9 +1777,7 @@ void _testLiveRegion() {
);
semantics().updateSemantics(builder.build());

Copy link
Contributor

@yjbanov yjbanov Dec 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I'd add expect(mockAccessibilityAnnouncements.announceInvoked, 1); here too to ensure that the count increases after the first update and then stays stable afterwards.

Copy link
Contributor Author

@nbayati nbayati Dec 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! Done!

expectSemanticsTree('''
<sem style="$rootSemanticStyle"></sem>
''');
expect(accessibilityAnnouncements.ariaLiveElementFor(Assertiveness.polite).text, '');

semantics().semanticsEnabled = false;
});
Expand Down