From 69e19aad4101250c90c3aab6f284e0ac4e11c439 Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Thu, 25 Jul 2024 16:08:22 -0400 Subject: [PATCH] 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 #465. Signed-off-by: Zixuan James Li --- lib/widgets/app.dart | 10 +++++-- lib/widgets/app_bar.dart | 22 ++++++++++++++ lib/widgets/inbox.dart | 5 +++- lib/widgets/message_list.dart | 5 +++- lib/widgets/profile.dart | 10 +++++-- lib/widgets/recent_dm_conversations.dart | 6 +++- lib/widgets/subscription_list.dart | 5 +++- test/widgets/app_bar_test.dart | 38 ++++++++++++++++++++++++ 8 files changed, 92 insertions(+), 9 deletions(-) create mode 100644 lib/widgets/app_bar.dart create mode 100644 test/widgets/app_bar_test.dart diff --git a/lib/widgets/app.dart b/lib/widgets/app.dart index b201281ba34..c58848a369e 100644 --- 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 new file mode 100644 index 00000000000..1f2884e3cf4 --- /dev/null +++ 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 index 61e218a202c..56a382c6c4a 100644 --- 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 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 index fca5db56642..d898d348b55 100644 --- 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 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 index afadc289b0d..533a5db09e3 100644 --- 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 index 19238d87707..40709b4452f 100644 --- 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 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 index 557f0a25e7c..763400614d2 100644 --- 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 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 new file mode 100644 index 00000000000..3685eb4ae09 --- /dev/null +++ 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; + }); +}