Skip to content

Commit 0920772

Browse files
cybernova2cybernova2
committed
all requested review resolved
1 parent 03f9ba9 commit 0920772

File tree

9 files changed

+37
-49
lines changed

9 files changed

+37
-49
lines changed

‎assets/l10n/app_en.arb‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1420,8 +1420,8 @@
14201420
"@zulipAppTitle": {
14211421
"description": "The name of Zulip. This should be either 'Zulip' or a transliteration."
14221422
},
1423-
"loading": "Loading…",
1424-
"@loading": {
1425-
"description": "Label shown for loading indicators"
1423+
"loading": "Loading…",
1424+
"@loading": {
1425+
"description": "Semantic label for a loading indicator"
14261426
}
14271427
}

‎lib/generated/l10n/zulip_localizations.dart‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2067,7 +2067,7 @@ abstract class ZulipLocalizations {
20672067
/// **'Zulip'**
20682068
String get zulipAppTitle;
20692069

2070-
/// Label shown for loading indicators
2070+
/// Semantic label for a loading indicator
20712071
///
20722072
/// In en, this message translates to:
20732073
/// **'Loading…'**

‎lib/widgets/action_sheet.dart‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -253,15 +253,12 @@ class BottomSheetEmptyContentPlaceholder extends StatelessWidget {
253253
@override
254254
Widget build(BuildContext context) {
255255
final designVariables = DesignVariables.of(context);
256-
final loc = ZulipLocalizations.of(context);
257-
final child = loading
258-
? Semantics(
259-
label:loc.loading ,
260-
textDirection: Directionality.of(context),
256+
final zulipLocalizations = ZulipLocalizations.of(context);
257+
final child = loading ? Semantics(
258+
label:zulipLocalizations.loading ,
261259
liveRegion: true,
262260
focusable: true,
263-
child: CircularProgressIndicator(),
264-
)
261+
child: CircularProgressIndicator())
265262
: Text(
266263
textAlign: TextAlign.center,
267264
style: TextStyle(

‎lib/widgets/home.dart‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,6 @@ class _LoadingPlaceholderPageState extends State<_LoadingPlaceholderPage> {
248248
children: [
249249
Semantics(
250250
label: zulipLocalizations.loading,
251-
textDirection: Directionality.of(context),
252251
liveRegion: true,
253252
child:const CircularProgressIndicator(),
254253
),

‎lib/widgets/message_list.dart‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1031,13 +1031,10 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat
10311031

10321032
if (!model.fetched) {
10331033
return Center(
1034-
child: Padding(
1035-
padding: const EdgeInsets.symmetric(vertical: 16.0),
10361034
child: Semantics(
10371035
label: zulipLocalizations.loading,
1038-
textDirection: Directionality.of(context),
10391036
liveRegion: true,
1040-
child: const CircularProgressIndicator())));
1037+
child: const CircularProgressIndicator()));
10411038
}
10421039

10431040
if (model.items.isEmpty && model.haveNewest && model.haveOldest) {
@@ -1292,15 +1289,12 @@ class _MessageListLoadingMore extends StatelessWidget {
12921289

12931290
@override
12941291
Widget build(BuildContext context) {
1295-
final loc = ZulipLocalizations.of(context);
1292+
final zulipLocalizations = ZulipLocalizations.of(context);
12961293
return Center(
1297-
child: Padding(
1298-
padding: EdgeInsets.symmetric(vertical: 16.0),
12991294
child: Semantics(
1300-
textDirection: Directionality.of(context),
1301-
label: loc.loading,
1295+
label: zulipLocalizations.loading,
13021296
liveRegion: true,
1303-
child: CircularProgressIndicator()))); // TODO perhaps a different indicator
1297+
child: CircularProgressIndicator())); // TODO perhaps a different indicator
13041298
}
13051299
}
13061300

‎lib/widgets/topic_list.dart‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -166,16 +166,14 @@ class _TopicListState extends State<_TopicList> with PerAccountStoreAwareStateMi
166166
@override
167167
Widget build(BuildContext context) {
168168
if (lastFetchedTopics == null) {
169-
final loc = ZulipLocalizations.of(context);
169+
final zulipLocalizations = ZulipLocalizations.of(context);
170170

171171
return Center(
172-
child: Padding(
173-
padding: const EdgeInsets.symmetric(vertical: 16.0),
174172
child: Semantics(
175-
textDirection: Directionality.of(context),
176-
label: loc.loading,
173+
label: zulipLocalizations.loading,
177174
liveRegion: true,
178-
child: CircularProgressIndicator(),),),);}
175+
child: CircularProgressIndicator()));
176+
}
179177

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

‎test/widgets/home_test.dart‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -399,15 +399,15 @@ void main () {
399399

400400
testWidgets('smoke', (tester) async {
401401
final SemanticsHandle semanticsHandle = tester.ensureSemantics();
402-
final loc = await ZulipLocalizations.delegate.load(const Locale('en'));
402+
final zulipLocalizations= await ZulipLocalizations.delegate.load(const Locale('en'));
403403
try {
404404
addTearDown(testBinding.reset);
405405
testBinding.globalStore.loadPerAccountDuration = loadPerAccountDuration;
406406
await prepare(tester);
407407

408408
// While loading, the loading page should be shown and have the semantics label.
409409
checkOnLoadingPage();
410-
expect(find.bySemanticsLabel(loc.loading), findsOneWidget);
410+
expect(find.bySemanticsLabel(zulipLocalizations.loading), findsOneWidget);
411411

412412
await tester.pump(loadPerAccountDuration);
413413
checkOnHomePage(tester, expectedAccount: eg.selfAccount);
@@ -418,7 +418,7 @@ void main () {
418418

419419
testWidgets('"Try another account" button appears after timeout', (tester) async {
420420
final SemanticsHandle semanticsHandle = tester.ensureSemantics();
421-
final loc = await ZulipLocalizations.delegate.load(const Locale('en'));
421+
final zulipLocalizations = await ZulipLocalizations.delegate.load(const Locale('en'));
422422
try {
423423
addTearDown(testBinding.reset);
424424
testBinding.globalStore.loadPerAccountDuration = loadPerAccountDuration;
@@ -428,7 +428,7 @@ void main () {
428428
check(find.text('Try another account').hitTestable()).findsNothing();
429429

430430
// The loading semantics should already be present.
431-
expect(find.bySemanticsLabel(loc.loading), findsOneWidget);
431+
expect(find.bySemanticsLabel(zulipLocalizations.loading), findsOneWidget);
432432

433433
await tester.pump(kTryAnotherAccountWaitPeriod);
434434
checkOnLoadingPage();
@@ -443,7 +443,7 @@ void main () {
443443

444444
testWidgets('while loading, go back from ChooseAccountPage', (tester) async {
445445
final SemanticsHandle semanticsHandle = tester.ensureSemantics();
446-
final loc = await ZulipLocalizations.delegate.load(const Locale('en'));
446+
final zulipLocalizations = await ZulipLocalizations.delegate.load(const Locale('en'));
447447
try {
448448
testBinding.globalStore.loadPerAccountDuration = loadPerAccountDuration;
449449
await prepare(tester);
@@ -456,12 +456,12 @@ void main () {
456456
check(lastPoppedRoute).isA<MaterialWidgetRoute>().page.isA<ChooseAccountPage>();
457457
await tester.pump(
458458
(lastPoppedRoute as TransitionRoute).reverseTransitionDuration
459-
// TODO not sure why a 1ms fudge is needed; investigate.
460-
+ Duration(milliseconds: 1));
459+
// TODO not sure why a 1ms fudge is needed; investigate.
460+
+ Duration(milliseconds: 1));
461461
checkOnLoadingPage();
462462

463463
// Semantics check: loading label present
464-
expect(find.bySemanticsLabel(loc.loading), findsOneWidget);
464+
expect(find.bySemanticsLabel(zulipLocalizations.loading), findsOneWidget);
465465

466466
await tester.pump(loadPerAccountDuration);
467467
checkOnHomePage(tester, expectedAccount: eg.selfAccount);
@@ -472,7 +472,7 @@ void main () {
472472

473473
testWidgets('while loading, choose a different account', (tester) async {
474474
final SemanticsHandle semanticsHandle = tester.ensureSemantics();
475-
final loc = await ZulipLocalizations.delegate.load(const Locale('en'));
475+
final zulipLocalizations = await ZulipLocalizations.delegate.load(const Locale('en'));
476476
try {
477477
testBinding.globalStore.loadPerAccountDuration = loadPerAccountDuration;
478478
await prepare(tester);
@@ -485,7 +485,7 @@ void main () {
485485
// While the second account is still loading, we should be on a loading page.
486486
await tester.pump(loadPerAccountDuration);
487487
checkOnLoadingPage();
488-
expect(find.bySemanticsLabel(loc.loading), findsOneWidget);
488+
expect(find.bySemanticsLabel(zulipLocalizations.loading), findsOneWidget);
489489

490490
await tester.pump(loadPerAccountDuration);
491491
// The second load finished.
@@ -497,7 +497,7 @@ void main () {
497497

498498
testWidgets('while loading, choosing an account disallows going back', (tester) async {
499499
final SemanticsHandle semanticsHandle = tester.ensureSemantics();
500-
final loc = await ZulipLocalizations.delegate.load(const Locale('en'));
500+
final zulipLocalizations = await ZulipLocalizations.delegate.load(const Locale('en'));
501501
try {
502502
testBinding.globalStore.loadPerAccountDuration = loadPerAccountDuration;
503503
await prepare(tester);
@@ -512,7 +512,7 @@ void main () {
512512
.isNotNull().isFirst.isTrue();
513513

514514
// Semantics check: ensure loading label present and first route is our account route.
515-
expect(find.bySemanticsLabel(loc.loading), findsOneWidget);
515+
expect(find.bySemanticsLabel(zulipLocalizations.loading), findsOneWidget);
516516

517517
await tester.pump(loadPerAccountDuration); // wait for loadPerAccount
518518
checkOnHomePage(tester, expectedAccount: eg.otherAccount);
@@ -523,7 +523,7 @@ void main () {
523523

524524
testWidgets('while loading, go to nested levels of ChooseAccountPage', (tester) async {
525525
final SemanticsHandle semanticsHandle = tester.ensureSemantics();
526-
final loc = await ZulipLocalizations.delegate.load(const Locale('en'));
526+
final zulipLocalizations = await ZulipLocalizations.delegate.load(const Locale('en'));
527527
try {
528528
testBinding.globalStore.loadPerAccountDuration = loadPerAccountDuration;
529529
final thirdAccount = eg.account(user: eg.thirdUser);
@@ -545,7 +545,7 @@ void main () {
545545
..accountId.equals(eg.otherAccount.id);
546546

547547
// Semantics: loading label should appear during the account switch.
548-
expect(find.bySemanticsLabel(loc.loading), findsOneWidget);
548+
expect(find.bySemanticsLabel(zulipLocalizations.loading), findsOneWidget);
549549

550550
await tester.pump(kTryAnotherAccountWaitPeriod);
551551

‎test/widgets/message_list_test.dart‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -842,9 +842,9 @@ void main() {
842842

843843
// Semantics check: ensure loading semantics label is not present
844844
final SemanticsHandle semanticsHandle = tester.ensureSemantics();
845-
final loc = await ZulipLocalizations.delegate.load(const Locale('en'));
845+
final zulipLocalizations = await ZulipLocalizations.delegate.load(const Locale('en'));
846846
try {
847-
expect(find.bySemanticsLabel(loc.loading), findsNothing);
847+
expect(find.bySemanticsLabel(zulipLocalizations.loading), findsNothing);
848848
} finally {
849849
semanticsHandle.dispose();
850850
}
@@ -882,8 +882,8 @@ void main() {
882882
check(tester.getRect(find.text('message 9')))
883883
.bottom.isGreaterThan(loadingIndicatorRect.top - 36); // TODO(#1569) where's this space going?
884884
//Semantics check: ensure the loading indicator exposes the expected label
885-
final loc = await ZulipLocalizations.delegate.load(const Locale('en'));
886-
expect(find.bySemanticsLabel(loc.loading), findsOneWidget);
885+
final zulipLocalizations = await ZulipLocalizations.delegate.load(const Locale('en'));
886+
expect(find.bySemanticsLabel(zulipLocalizations.loading), findsOneWidget);
887887
await tester.pumpAndSettle();
888888
} finally {
889889
semanticsHandle.dispose();

‎test/widgets/topic_list_test.dart‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ void main() {
117117
);
118118
// Enable semantics for this test so we can assert the accessibility label.
119119
final SemanticsHandle semanticsHandle = tester.ensureSemantics();
120-
final loc = await ZulipLocalizations.delegate.load(const Locale('en'));
120+
final zulipLocalizations = await ZulipLocalizations.delegate.load(const Locale('en'));
121121
try {
122122
await tester.pumpWidget(TestZulipApp(
123123
accountId: eg.selfAccount.id,
@@ -126,13 +126,13 @@ void main() {
126126
// Visual indicator present
127127
check(find.byType(CircularProgressIndicator)).findsOne();
128128
// Semantics label should be discoverable while loading.
129-
expect(find.bySemanticsLabel(loc.loading), findsOneWidget);
129+
expect(find.bySemanticsLabel(zulipLocalizations.loading), findsOneWidget);
130130

131131
await tester.pump(Duration(seconds: 1));
132132
// Visual indicator gone after load completes
133133
check(find.byType(CircularProgressIndicator)).findsNothing();
134134
// Semantics label should also be gone.
135-
expect(find.bySemanticsLabel(loc.loading), findsNothing);
135+
expect(find.bySemanticsLabel(zulipLocalizations.loading), findsNothing);
136136
} finally {
137137
semanticsHandle.dispose();
138138
}

0 commit comments

Comments
 (0)