ui: Extract ZulipAppBar for loading indicator.

Ideally we may have test to exhaustively ensure that all pages specific to a single PerAccountStore use ZulipAppBar. Some pages with `AppBar`s are skipped, such as AboutZulip (no PerAccountStore access) and lightboxes (progress indicator is occupied for other purposes, and the AppBar can be hidden). Fixes zulip#465. Signed-off-by: Zixuan James Li <[email protected]>
PIG208 · Aug 8, 2024 · 69e19aa · 69e19aa
1 parent 391167b
commit 69e19aa
Show file tree

Hide file tree

Showing 8 changed files with 92 additions and 9 deletions.
diff --git a/lib/widgets/app.dart b/lib/widgets/app.dart
@@ -8,6 +8,7 @@ import 'package:flutter_gen/gen_l10n/zulip_localizations.dart';
 import '../model/localizations.dart';
 import '../model/narrow.dart';
 import 'about_zulip.dart';
+import 'app_bar.dart';
 import 'inbox.dart';
 import 'login.dart';
 import 'message_list.dart';
@@ -179,9 +180,10 @@ class ChooseAccountPage extends StatelessWidget {
     assert(!PerAccountStoreWidget.debugExistsOf(context));
     final globalStore = GlobalStoreWidget.of(context);
     return Scaffold(
-      appBar: AppBar(
+      appBar: ZulipAppBar(
         title: Text(zulipLocalizations.chooseAccountPageTitle),
-        actions: const [ChooseAccountPageOverflowButton()]),
+        actions: const [ChooseAccountPageOverflowButton()],
+        isLoading: false),
       body: SafeArea(
         minimum: const EdgeInsets.fromLTRB(8, 0, 8, 8),
         child: Center(
@@ -252,7 +254,9 @@ class HomePage extends StatelessWidget {
     }
 
     return Scaffold(
-      appBar: AppBar(title: const Text("Home")),
+      appBar: ZulipAppBar(
+        title: const Text("Home"),
+        isLoading: store.isLoading),
       body: Center(
         child: Column(mainAxisAlignment: MainAxisAlignment.center, children: [
           DefaultTextStyle.merge(

diff --git a/lib/widgets/app_bar.dart b/lib/widgets/app_bar.dart
@@ -0,0 +1,22 @@
+import 'package:flutter/material.dart';
+
+/// A custom [AppBar] with a loading indicator.
+///
+/// This should be used for most of the pages with access to [PerAccountStore].
+// However, there are some exceptions (add more if necessary):
+// - `lib/widgets/lightbox.dart`
+class ZulipAppBar extends AppBar {
+  ZulipAppBar({
+    super.key,
+    required super.title,
+    super.backgroundColor,
+    super.shape,
+    super.actions,
+    required bool isLoading,
+  }) : super(
+    bottom: PreferredSize(
+      preferredSize: const Size.fromHeight(4.0),
+      child: (isLoading)
+        ? LinearProgressIndicator(backgroundColor: backgroundColor, minHeight: 4.0)
+        : const SizedBox.shrink()));
+}
diff --git a/lib/widgets/inbox.dart b/lib/widgets/inbox.dart
@@ -4,6 +4,7 @@ import '../api/model/model.dart';
 import '../model/narrow.dart';
 import '../model/recent_dm_conversations.dart';
 import '../model/unreads.dart';
+import 'app_bar.dart';
 import 'icons.dart';
 import 'message_list.dart';
 import 'page.dart';
@@ -160,7 +161,9 @@ class _InboxPageState extends State<InboxPage> with PerAccountStoreAwareStateMix
     }
 
     return Scaffold(
-      appBar: AppBar(title: const Text('Inbox')),
+      appBar: ZulipAppBar(
+        title: const Text('Inbox'),
+        isLoading: store.isLoading),
       body: SafeArea(
         // Don't pad the bottom here; we want the list content to do that.
         bottom: false,

diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart
@@ -13,6 +13,7 @@ import '../model/store.dart';
 import '../model/typing_status.dart';
 import 'action_sheet.dart';
 import 'actions.dart';
+import 'app_bar.dart';
 import 'compose_box.dart';
 import 'content.dart';
 import 'dialog.dart';
@@ -256,7 +257,9 @@ class _MessageListPageState extends State<MessageListPage> implements MessageLis
     }
 
     return Scaffold(
-      appBar: AppBar(title: MessageListAppBarTitle(narrow: widget.narrow),
+      appBar: ZulipAppBar(
+        title: MessageListAppBarTitle(narrow: widget.narrow),
+        isLoading: store.isLoading,
         backgroundColor: appBarBackgroundColor,
         shape: removeAppBarBottomBorder
           ? const Border()

diff --git a/lib/widgets/profile.dart b/lib/widgets/profile.dart
@@ -6,6 +6,7 @@ import 'package:flutter_gen/gen_l10n/zulip_localizations.dart';
 import '../api/model/model.dart';
 import '../model/content.dart';
 import '../model/narrow.dart';
+import 'app_bar.dart';
 import 'content.dart';
 import 'message_list.dart';
 import 'page.dart';
@@ -69,7 +70,9 @@ class ProfilePage extends StatelessWidget {
     ];
 
     return Scaffold(
-      appBar: AppBar(title: Text(user.fullName)),
+      appBar: ZulipAppBar(
+        title: Text(user.fullName),
+        isLoading: store.isLoading),
       body: SingleChildScrollView(
         child: Center(
           child: ConstrainedBox(
@@ -87,8 +90,11 @@ class _ProfileErrorPage extends StatelessWidget {
 
   @override
   Widget build(BuildContext context) {
+    final store = PerAccountStoreWidget.of(context);
     return Scaffold(
-      appBar: AppBar(title: const Text('Error')),
+      appBar: ZulipAppBar(
+        title: const Text('Error'),
+        isLoading: store.isLoading),
       body: const SingleChildScrollView(
         child: Padding(
           padding: EdgeInsets.symmetric(horizontal: 16, vertical: 32),

diff --git a/lib/widgets/recent_dm_conversations.dart b/lib/widgets/recent_dm_conversations.dart
@@ -4,6 +4,7 @@ import 'package:flutter_gen/gen_l10n/zulip_localizations.dart';
 import '../model/narrow.dart';
 import '../model/recent_dm_conversations.dart';
 import '../model/unreads.dart';
+import 'app_bar.dart';
 import 'content.dart';
 import 'icons.dart';
 import 'message_list.dart';
@@ -55,10 +56,13 @@ class _RecentDmConversationsPageState extends State<RecentDmConversationsPage> w
 
   @override
   Widget build(BuildContext context) {
+    final store = PerAccountStoreWidget.of(context);
     final zulipLocalizations = ZulipLocalizations.of(context);
     final sorted = model!.sorted;
     return Scaffold(
-      appBar: AppBar(title: Text(zulipLocalizations.recentDmConversationsPageTitle)),
+      appBar: ZulipAppBar(
+        title: Text(zulipLocalizations.recentDmConversationsPageTitle),
+        isLoading: store.isLoading),
       body: SafeArea(
         // Don't pad the bottom here; we want the list content to do that.
         bottom: false,

diff --git a/lib/widgets/subscription_list.dart b/lib/widgets/subscription_list.dart
@@ -3,6 +3,7 @@ import 'package:flutter/material.dart';
 import '../api/model/model.dart';
 import '../model/narrow.dart';
 import '../model/unreads.dart';
+import 'app_bar.dart';
 import 'icons.dart';
 import 'message_list.dart';
 import 'page.dart';
@@ -89,7 +90,9 @@ class _SubscriptionListPageState extends State<SubscriptionListPage> with PerAcc
     _sortSubs(unpinned);
 
     return Scaffold(
-      appBar: AppBar(title: const Text("Channels")),
+      appBar: ZulipAppBar(
+        title: const Text("Channels"),
+        isLoading: store.isLoading),
       body: SafeArea(
         // Don't pad the bottom here; we want the list content to do that.
         bottom: false,

diff --git a/test/widgets/app_bar_test.dart b/test/widgets/app_bar_test.dart
@@ -0,0 +1,38 @@
+import 'package:checks/checks.dart';
+import 'package:flutter/material.dart';
+import 'package:flutter_test/flutter_test.dart';
+import 'package:zulip/widgets/app_bar.dart';
+import 'package:zulip/widgets/profile.dart';
+
+import '../example_data.dart' as eg;
+import '../model/binding.dart';
+import '../model/test_store.dart';
+import 'test_app.dart';
+
+void main() {
+  TestZulipBinding.ensureInitialized();
+
+  testWidgets('show progress indicator when loading', (tester) async {
+    addTearDown(testBinding.reset);
+    await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
+
+    final store = await testBinding.globalStore.perAccount(eg.selfAccount.id);
+    await store.addUser(eg.selfUser);
+
+    await tester.pumpWidget(TestZulipApp(accountId: eg.selfAccount.id,
+      child: ProfilePage(userId: eg.selfUser.userId)));
+
+    final finder = find.descendant(
+      of: find.byType(ZulipAppBar),
+      matching: find.byType(LinearProgressIndicator));
+
+    await tester.pumpAndSettle();
+    final rectBefore = tester.getRect(find.byType(ZulipAppBar));
+    check(finder.evaluate()).isEmpty();
+    store.isLoading = true;
+
+    await tester.pump();
+    check(tester.getRect(find.byType(ZulipAppBar))).equals(rectBefore);
+    check(finder.evaluate()).single;
+  });
+}