-
Notifications
You must be signed in to change notification settings - Fork 370
Add Semantics labels to loading indicators and update related tests #2025
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
base: main
Are you sure you want to change the base?
Changes from 2 commits
0d44768
03f9ba9
0920772
0360488
e643330
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 |
|---|---|---|
|
|
@@ -1419,5 +1419,9 @@ | |
| "zulipAppTitle": "Zulip", | ||
| "@zulipAppTitle": { | ||
| "description": "The name of Zulip. This should be either 'Zulip' or a transliteration." | ||
| } | ||
| }, | ||
| "loading": "Loading…", | ||
| "@loading": { | ||
| "description": "Label shown for loading indicators" | ||
| } | ||
|
Comment on lines
1491
to
1495
Collaborator
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. nit: follow indentation style |
||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -253,10 +253,16 @@ class BottomSheetEmptyContentPlaceholder extends StatelessWidget { | |||||
| @override | ||||||
| Widget build(BuildContext context) { | ||||||
| final designVariables = DesignVariables.of(context); | ||||||
|
|
||||||
| final loc = ZulipLocalizations.of(context); | ||||||
|
||||||
| final loc = ZulipLocalizations.of(context); | |
| final zulipLocalizations = ZulipLocalizations.of(context); |
(Like everywhere else we do this.)
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.
The formatting of this code looks really messy; please follow the project's style in the use of spaces, indentation, newlines, etc. (here and throughout the PR).
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1029,7 +1029,16 @@ 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: zulipLocalizations.loading, | ||
| textDirection: Directionality.of(context), | ||
| liveRegion: true, | ||
| child: const CircularProgressIndicator()))); | ||
| } | ||
|
|
||
| if (model.items.isEmpty && model.haveNewest && model.haveOldest) { | ||
| final String header; | ||
|
|
@@ -1283,10 +1292,15 @@ class _MessageListLoadingMore extends StatelessWidget { | |
|
|
||
| @override | ||
| Widget build(BuildContext context) { | ||
| return const Center( | ||
| final loc = ZulipLocalizations.of(context); | ||
| return Center( | ||
| child: Padding( | ||
| padding: EdgeInsets.symmetric(vertical: 16.0), | ||
| child: CircularProgressIndicator())); // TODO perhaps a different indicator | ||
| child: Semantics( | ||
| textDirection: Directionality.of(context), | ||
|
||
| label: loc.loading, | ||
| liveRegion: true, | ||
| child: CircularProgressIndicator()))); // TODO perhaps a different indicator | ||
| } | ||
| } | ||
|
|
||
|
|
||
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.
nit: "Label shown for loading indicators" is misleading; this isn't shown in the app's visual UI. Rather, it's information presented to screen-reader software, to help create an interface for users with vision loss etc.
How about: "Semantic label for a loading indicator."