Skip to content

accessibility: Add Semantics to loading indicators so screen readers announce loading #2017

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

Closed
cybernova2 wants to merge 1 commit into from
Closed

accessibility: Add Semantics to loading indicators so screen readers announce loading #2017

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
10 changes: 9 additions & 1 deletion lib/widgets/action_sheet.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,13 @@ class BottomSheetEmptyContentPlaceholder extends StatelessWidget {
final designVariables = DesignVariables.of(context);

final child = loading
? CircularProgressIndicator()
? Semantics(
textDirection: Directionality.of(context),
focusable: true,
liveRegion: true,
label: 'Loading…',
child: CircularProgressIndicator(),
)
: Text(
textAlign: TextAlign.center,
style: TextStyle(
Expand Down Expand Up @@ -628,6 +634,8 @@ class ChannelFeedButton extends ActionSheetMenuItemButton {
}
}



class CopyChannelLinkButton extends ActionSheetMenuItemButton {
const CopyChannelLinkButton({
super.key,
Expand Down
7 changes: 6 additions & 1 deletion lib/widgets/home.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,12 @@ class _LoadingPlaceholderPageState extends State<_LoadingPlaceholderPage> {
child: Column(
mainAxisSize: MainAxisSize.min,
children: [
const CircularProgressIndicator(),
Semantics(
label: 'Loading…',
textDirection: Directionality.of(context),
liveRegion: true,
child:const CircularProgressIndicator(),
),
Visibility(
visible: showTryAnotherAccount,
maintainSize: true,
Expand Down
29 changes: 25 additions & 4 deletions lib/widgets/message_list.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1029,7 +1029,20 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat
Widget build(BuildContext context) {
final zulipLocalizations = ZulipLocalizations.of(context);

if (!model.fetched) return const Center(child: CircularProgressIndicator());
if (!model.fetched) {
return Center(
child: Padding(
padding: const EdgeInsets.symmetric(vertical: 16.0),
child: Semantics(
label: 'Loading more messages',
textDirection: Directionality.of(context),
liveRegion: true,
child: const CircularProgressIndicator(),
),
),
);
}


if (model.items.isEmpty && model.haveNewest && model.haveOldest) {
final String header;
Expand Down Expand Up @@ -1283,10 +1296,18 @@ class _MessageListLoadingMore extends StatelessWidget {

@override
Widget build(BuildContext context) {
return const Center(
return Center(
child: Padding(
padding: EdgeInsets.symmetric(vertical: 16.0),
child: CircularProgressIndicator())); // TODO perhaps a different indicator
padding: const EdgeInsets.symmetric(vertical: 16.0),
child: Semantics(
textDirection: Directionality.of(context),
label: 'Loading more messages',
liveRegion: true,
child: const CircularProgressIndicator(),
),
),
);
// TODO perhaps a different indicator
}
}

Expand Down
14 changes: 13 additions & 1 deletion lib/widgets/topic_list.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,21 @@ class _TopicListState extends State<_TopicList> with PerAccountStoreAwareStateMi
@override
Widget build(BuildContext context) {
if (lastFetchedTopics == null) {
return const Center(child: CircularProgressIndicator());
return Center(
child: Padding(
padding: const EdgeInsets.symmetric(vertical: 16.0),
child: Semantics(
textDirection: Directionality.of(context),
label: 'Loading…', // plain string (not localized)
liveRegion: true,
child: CircularProgressIndicator(),
),
),
);

}


// TODO(design) handle the rare case when `lastFetchedTopics` is empty

// This is adapted from parts of the build method on [_InboxPageState].
Expand Down
224 changes: 136 additions & 88 deletions test/widgets/home_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -397,117 +397,165 @@ void main () {
}

testWidgets('smoke', (tester) async {
addTearDown(testBinding.reset);
testBinding.globalStore.loadPerAccountDuration = loadPerAccountDuration;
await prepare(tester);
await tester.pump(loadPerAccountDuration);
checkOnHomePage(tester, expectedAccount: eg.selfAccount);
final SemanticsHandle semanticsHandle = tester.ensureSemantics();
try {
addTearDown(testBinding.reset);
testBinding.globalStore.loadPerAccountDuration = loadPerAccountDuration;
await prepare(tester);

// While loading, the loading page should be shown and have the semantics label.
checkOnLoadingPage();
expect(find.bySemanticsLabel('Loading…'), findsOneWidget);

await tester.pump(loadPerAccountDuration);
checkOnHomePage(tester, expectedAccount: eg.selfAccount);
} finally {
semanticsHandle.dispose();
}
});

testWidgets('"Try another account" button appears after timeout', (tester) async {
addTearDown(testBinding.reset);
testBinding.globalStore.loadPerAccountDuration = loadPerAccountDuration;
await prepare(tester);
checkOnLoadingPage();
check(find.text('Try another account').hitTestable()).findsNothing();

await tester.pump(kTryAnotherAccountWaitPeriod);
checkOnLoadingPage();
check(find.text('Try another account').hitTestable()).findsOne();

await tester.pump(loadPerAccountDuration);
checkOnHomePage(tester, expectedAccount: eg.selfAccount);
testWidgets('"Try another account" button appears after timeout', (tester) async {
final SemanticsHandle semanticsHandle = tester.ensureSemantics();
try {
addTearDown(testBinding.reset);
testBinding.globalStore.loadPerAccountDuration = loadPerAccountDuration;
await prepare(tester);
checkOnLoadingPage();
// No try-another button immediately.
check(find.text('Try another account').hitTestable()).findsNothing();

// The loading semantics should already be present.
expect(find.bySemanticsLabel('Loading…'), findsOneWidget);

await tester.pump(kTryAnotherAccountWaitPeriod);
checkOnLoadingPage();
check(find.text('Try another account').hitTestable()).findsOne();

await tester.pump(loadPerAccountDuration);
checkOnHomePage(tester, expectedAccount: eg.selfAccount);
} finally {
semanticsHandle.dispose();
}
});

testWidgets('while loading, go back from ChooseAccountPage', (tester) async {
testBinding.globalStore.loadPerAccountDuration = loadPerAccountDuration;
await prepare(tester);
await tester.pump(kTryAnotherAccountWaitPeriod);
await tapTryAnotherAccount(tester);

lastPoppedRoute = null;
await tester.tap(find.byType(BackButton));
await tester.pump();
check(lastPoppedRoute).isA<MaterialWidgetRoute>().page.isA<ChooseAccountPage>();
await tester.pump(
(lastPoppedRoute as TransitionRoute).reverseTransitionDuration
// TODO not sure why a 1ms fudge is needed; investigate.
+ Duration(milliseconds: 1));
checkOnLoadingPage();

await tester.pump(loadPerAccountDuration);
checkOnHomePage(tester, expectedAccount: eg.selfAccount);
testWidgets('while loading, go back from ChooseAccountPage', (tester) async {
final SemanticsHandle semanticsHandle = tester.ensureSemantics();
try {
testBinding.globalStore.loadPerAccountDuration = loadPerAccountDuration;
await prepare(tester);
await tester.pump(kTryAnotherAccountWaitPeriod);
await tapTryAnotherAccount(tester);

lastPoppedRoute = null;
await tester.tap(find.byType(BackButton));
await tester.pump();
check(lastPoppedRoute).isA<MaterialWidgetRoute>().page.isA<ChooseAccountPage>();
await tester.pump(
(lastPoppedRoute as TransitionRoute).reverseTransitionDuration
// TODO not sure why a 1ms fudge is needed; investigate.
+ Duration(milliseconds: 1));
checkOnLoadingPage();

// Semantics check: loading label present
expect(find.bySemanticsLabel('Loading…'), findsOneWidget);

await tester.pump(loadPerAccountDuration);
checkOnHomePage(tester, expectedAccount: eg.selfAccount);
} finally {
semanticsHandle.dispose();
}
});

testWidgets('while loading, choose a different account', (tester) async {
testBinding.globalStore.loadPerAccountDuration = loadPerAccountDuration;
await prepare(tester);
await tester.pump(kTryAnotherAccountWaitPeriod);
await tapTryAnotherAccount(tester);

testBinding.globalStore.loadPerAccountDuration = loadPerAccountDuration * 2;
await chooseAccountWithEmail(tester, eg.otherAccount.email);

await tester.pump(loadPerAccountDuration);
// The second loadPerAccount is still pending.
checkOnLoadingPage();

await tester.pump(loadPerAccountDuration);
// The second loadPerAccount finished.
checkOnHomePage(tester, expectedAccount: eg.otherAccount);
testWidgets('while loading, choose a different account', (tester) async {
final SemanticsHandle semanticsHandle = tester.ensureSemantics();
try {
testBinding.globalStore.loadPerAccountDuration = loadPerAccountDuration;
await prepare(tester);
await tester.pump(kTryAnotherAccountWaitPeriod);
await tapTryAnotherAccount(tester);

testBinding.globalStore.loadPerAccountDuration = loadPerAccountDuration * 2;
await chooseAccountWithEmail(tester, eg.otherAccount.email);

// While the second account is still loading, we should be on a loading page.
await tester.pump(loadPerAccountDuration);
checkOnLoadingPage();
expect(find.bySemanticsLabel('Loading…'), findsOneWidget);

await tester.pump(loadPerAccountDuration);
// The second load finished.
checkOnHomePage(tester, expectedAccount: eg.otherAccount);
} finally {
semanticsHandle.dispose();
}
});

testWidgets('while loading, choosing an account disallows going back', (tester) async {
testBinding.globalStore.loadPerAccountDuration = loadPerAccountDuration;
await prepare(tester);
await tester.pump(kTryAnotherAccountWaitPeriod);
await tapTryAnotherAccount(tester);

// While still loading, choose a different account.
await chooseAccountWithEmail(tester, eg.otherAccount.email);

// User cannot go back because the navigator stack
// was cleared after choosing an account.
check(getRouteOf(tester, find.byType(CircularProgressIndicator)))
.isNotNull().isFirst.isTrue();

await tester.pump(loadPerAccountDuration); // wait for loadPerAccount
checkOnHomePage(tester, expectedAccount: eg.otherAccount);
testWidgets('while loading, choosing an account disallows going back', (tester) async {
final SemanticsHandle semanticsHandle = tester.ensureSemantics();
try {
testBinding.globalStore.loadPerAccountDuration = loadPerAccountDuration;
await prepare(tester);
await tester.pump(kTryAnotherAccountWaitPeriod);
await tapTryAnotherAccount(tester);

// While still loading, choose a different account.
await chooseAccountWithEmail(tester, eg.otherAccount.email);

// User cannot go back because the navigator stack was cleared after choosing an account.
check(getRouteOf(tester, find.byType(CircularProgressIndicator)))
.isNotNull().isFirst.isTrue();

// Semantics check: ensure loading label present and first route is our account route.
expect(find.bySemanticsLabel('Loading…'), findsOneWidget);

await tester.pump(loadPerAccountDuration); // wait for loadPerAccount
checkOnHomePage(tester, expectedAccount: eg.otherAccount);
} finally {
semanticsHandle.dispose();
}
});

testWidgets('while loading, go to nested levels of ChooseAccountPage', (tester) async {
testBinding.globalStore.loadPerAccountDuration = loadPerAccountDuration;
final thirdAccount = eg.account(user: eg.thirdUser);
await testBinding.globalStore.add(thirdAccount, eg.initialSnapshot(
realmUsers: [eg.thirdUser]));
await prepare(tester);

await tester.pump(kTryAnotherAccountWaitPeriod);
// While still loading the first account, choose a different account.
await tapTryAnotherAccount(tester);
await chooseAccountWithEmail(tester, eg.otherAccount.email);
// User cannot go back because the navigator stack
// was cleared after choosing an account.
check(getRouteOf(tester, find.byType(CircularProgressIndicator)))
.isA<MaterialAccountWidgetRoute>()
testWidgets('while loading, go to nested levels of ChooseAccountPage', (tester) async {
final SemanticsHandle semanticsHandle = tester.ensureSemantics();
try {
testBinding.globalStore.loadPerAccountDuration = loadPerAccountDuration;
final thirdAccount = eg.account(user: eg.thirdUser);
await testBinding.globalStore.add(thirdAccount, eg.initialSnapshot(
realmUsers: [eg.thirdUser]));
await prepare(tester);

await tester.pump(kTryAnotherAccountWaitPeriod);
await tapTryAnotherAccount(tester);
await chooseAccountWithEmail(tester, eg.otherAccount.email);
check(getRouteOf(tester, find.byType(CircularProgressIndicator)))
.isA<MaterialAccountWidgetRoute>()
..isFirst.isTrue()
..accountId.equals(eg.otherAccount.id);

await tester.pump(kTryAnotherAccountWaitPeriod);
// While still loading the second account, choose a different account.
await tapTryAnotherAccount(tester);
await chooseAccountWithEmail(tester, thirdAccount.email);
// User cannot go back because the navigator stack
// was cleared after choosing an account.
check(getRouteOf(tester, find.byType(CircularProgressIndicator)))
.isA<MaterialAccountWidgetRoute>()
// Semantics check: loading label present
expect(find.bySemanticsLabel('Loading…'), findsOneWidget);

await tester.pump(kTryAnotherAccountWaitPeriod);
await tapTryAnotherAccount(tester);
await chooseAccountWithEmail(tester, thirdAccount.email);
check(getRouteOf(tester, find.byType(CircularProgressIndicator)))
.isA<MaterialAccountWidgetRoute>()
..isFirst.isTrue()
..accountId.equals(thirdAccount.id);

await tester.pump(loadPerAccountDuration); // wait for loadPerAccount
checkOnHomePage(tester, expectedAccount: thirdAccount);
await tester.pump(loadPerAccountDuration); // wait for loadPerAccount
checkOnHomePage(tester, expectedAccount: thirdAccount);
} finally {
semanticsHandle.dispose();
}
});


testWidgets('after finishing loading, go back from ChooseAccountPage', (tester) async {
testBinding.globalStore.loadPerAccountDuration = loadPerAccountDuration;
await prepare(tester);
Expand Down
Loading