-
Notifications
You must be signed in to change notification settings - Fork 368
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?
Conversation
|
Thanks for the Pull Request, I can provide some insights on this PR as a contributor just like you. |
lib/widgets/action_sheet.dart
Outdated
| ? CircularProgressIndicator() | ||
| : Text( | ||
| ? Semantics( | ||
| label: 'Loading…', |
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.
Please make sure not to insert hardcoded strings and instead use the zulip localization to fetch those from app_en.arb, this helps zulip to make the app available to user across the globe with different languages.
lib/widgets/message_list.dart
Outdated
| child: const CircularProgressIndicator(), | ||
| ), | ||
| ), | ||
| ); |
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:
| child: const CircularProgressIndicator(), | |
| ), | |
| ), | |
| ); | |
| child: const CircularProgressIndicator()))); |
See in this file (and other examples across the codebase).
lib/widgets/topic_list.dart
Outdated
| child: CircularProgressIndicator(), | ||
| ), | ||
| ), | ||
| ); |
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:
| child: CircularProgressIndicator(), | |
| ), | |
| ), | |
| ); | |
| child: CircularProgressIndicator()))); |
lib/widgets/home.dart
Outdated
| children: [ | ||
| const CircularProgressIndicator(), | ||
| Semantics( | ||
| label: 'Loading…', |
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.
Same as here, use ZulipLocalization.
lib/widgets/topic_list.dart
Outdated
| padding: const EdgeInsets.symmetric(vertical: 16.0), | ||
| child: Semantics( | ||
| textDirection: Directionality.of(context), | ||
| label: 'Loading…', // plain string (not localized) |
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.
Same as Here, use ZulipLocalization.
lib/widgets/message_list.dart
Outdated
| child: Padding( | ||
| padding: const EdgeInsets.symmetric(vertical: 16.0), | ||
| child: Semantics( | ||
| label: 'Loading more messages', |
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.
Use ZulipLocalization.
lib/widgets/message_list.dart
Outdated
| 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:
| child: CircularProgressIndicator(), | |
| ))); // TODO perhaps a different indicator | |
| child: CircularProgressIndicator()))); // TODO perhaps a different indicator |
|
Thanks for taking the time to review this! @MritunjayTiwari14 .I’ll implement the changes you recommended and update the branch. |
Add a localized 'loading' label in multiple widgets and apply it consistently across the UI. Update ARB files and regenerate localization outputs. Update widget tests to use the localized semantics label instead of hardcoded strings.
Fixes: #1962
This PR improves accessibility by adding
Semanticswidgets around loadingindicators so screen readers (TalkBack / VoiceOver) announce that the app is
loading content.
What’s changed
CircularProgressIndicator(and similar loading states) in:Semantics(label: 'Loading…', liveRegion: true)Why
Screen readers were not able to focus on or announce the loading state,
making the app inaccessible while fetching messages or topics.
Testing
-Verified that find.bySemanticsLabel(...) correctly locates the loading indicators.
-Ensured no regressions in navigation or layout.
-flutter analyze reports no new warnings.
Screenshot (TalkBack focus on loading indicator)
This shows the loading indicator now exposes an accessible label via
Semantics().Notes
this new PR contains only the intended accessibility changes.
Related
Replaces the earlier PR (#2017) with a clean version.