diff --git a/lib/widgets/app.dart b/lib/widgets/app.dart index e234534f7a..90dddccc75 100644 --- a/lib/widgets/app.dart +++ b/lib/widgets/app.dart @@ -266,8 +266,7 @@ class ChooseAccountPage extends StatelessWidget { // The default trailing padding with M3 is 24px. Decrease by 12 because // IconButton (the "…" button) comes with 12px padding on all sides. contentPadding: const EdgeInsetsDirectional.only(start: 16, end: 12), - onTap: () => Navigator.push(context, - HomePage.buildRoute(accountId: accountId)))); + onTap: () => HomePage.navigate(context, accountId: accountId))); } @override diff --git a/lib/widgets/home.dart b/lib/widgets/home.dart index 29d79ba3d3..d6b47338a9 100644 --- a/lib/widgets/home.dart +++ b/lib/widgets/home.dart @@ -36,6 +36,14 @@ class HomePage extends StatefulWidget { page: const HomePage()); } + /// Navigate to [HomePage], ensuring that its route is at the root level. + static void navigate(BuildContext context, {required int accountId}) { + final navigator = Navigator.of(context); + navigator.popUntil((route) => route.isFirst); + unawaited(navigator.pushReplacement( + HomePage.buildRoute(accountId: accountId))); + } + @override State createState() => _HomePageState(); } diff --git a/lib/widgets/login.dart b/lib/widgets/login.dart index 088650e4f2..ce1cf09438 100644 --- a/lib/widgets/login.dart +++ b/lib/widgets/login.dart @@ -395,10 +395,7 @@ class _LoginPageState extends State { return; } - unawaited(Navigator.of(context).pushAndRemoveUntil( - HomePage.buildRoute(accountId: accountId), - (route) => (route is! _LoginSequenceRoute)), - ); + HomePage.navigate(context, accountId: accountId); } Future _getUserId(String email, String apiKey) async { diff --git a/test/flutter_checks.dart b/test/flutter_checks.dart index 9d81e8ea20..d2b4b6f38d 100644 --- a/test/flutter_checks.dart +++ b/test/flutter_checks.dart @@ -44,6 +44,7 @@ extension IconChecks on Subject { } extension RouteChecks on Subject> { + Subject get isFirst => has((r) => r.isFirst, 'isFirst'); Subject get settings => has((r) => r.settings, 'settings'); } diff --git a/test/widgets/app_test.dart b/test/widgets/app_test.dart index f252937ef9..2b5d29b682 100644 --- a/test/widgets/app_test.dart +++ b/test/widgets/app_test.dart @@ -1,3 +1,5 @@ +import 'dart:async'; + import 'package:checks/checks.dart'; import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; @@ -157,6 +159,45 @@ void main() { ..bottom.isLessThan(2 / 3 * screenHeight); }); + testWidgets('choosing an account clears the navigator stack', (tester) async { + addTearDown(testBinding.reset); + await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); + await testBinding.globalStore.add(eg.otherAccount, eg.initialSnapshot()); + + final pushedRoutes = >[]; + final poppedRoutes = >[]; + final testNavObserver = TestNavigatorObserver(); + testNavObserver.onPushed = (route, prevRoute) => pushedRoutes.add(route); + testNavObserver.onPopped = (route, prevRoute) => poppedRoutes.add(route); + testNavObserver.onReplaced = (route, prevRoute) { + poppedRoutes.add(prevRoute!); + pushedRoutes.add(route!); + }; + await tester.pumpWidget(ZulipApp(navigatorObservers: [testNavObserver])); + await tester.pump(); + + final navigator = await ZulipApp.navigator; + unawaited(navigator.push( + MaterialWidgetRoute(page: const ChooseAccountPage()))); + await tester.pumpAndSettle(); + + check(poppedRoutes).isEmpty(); + check(pushedRoutes).deepEquals(>[ + (it) => it.isA() + ..accountId.equals(eg.selfAccount.id) + ..page.isA(), + (it) => it.isA().page.isA() + ]); + pushedRoutes.clear(); + + await tester.tap(find.text(eg.otherAccount.email)); + await tester.pump(); + check(poppedRoutes).length.equals(2); + check(pushedRoutes).single.isA() + ..accountId.equals(eg.otherAccount.id) + ..page.isA(); + }); + group('log out', () { Future<(Widget, Widget)> prepare(WidgetTester tester, {required Account account}) async { await setupChooseAccountPage(tester, accounts: [account]); diff --git a/test/widgets/home_test.dart b/test/widgets/home_test.dart index dfd217685a..8a40b10f26 100644 --- a/test/widgets/home_test.dart +++ b/test/widgets/home_test.dart @@ -9,6 +9,7 @@ import 'package:zulip/widgets/app_bar.dart'; import 'package:zulip/widgets/home.dart'; import 'package:zulip/widgets/icons.dart'; import 'package:zulip/widgets/inbox.dart'; +import 'package:zulip/widgets/page.dart'; import 'package:zulip/widgets/profile.dart'; import 'package:zulip/widgets/subscription_list.dart'; import 'package:zulip/widgets/theme.dart'; @@ -18,6 +19,7 @@ import '../example_data.dart' as eg; import '../flutter_checks.dart'; import '../model/binding.dart'; import '../model/test_store.dart'; +import 'page_checks.dart'; import 'test_app.dart'; void main () { @@ -213,4 +215,171 @@ void main () { check(find.text(eg.selfUser.fullName)).findsAny(); }); }); + + group('_LoadingPlaceholderPage', () { + void checkOnLoadingPage() { + check(find.byType(CircularProgressIndicator).hitTestable()).findsOne(); + check(find.byType(ChooseAccountPage)).findsNothing(); + check(find.byType(HomePage)).findsNothing(); + } + + void checkOnChooseAccountPage() { + // Ignore the possible loading page in the background. + check(find.byType(CircularProgressIndicator).hitTestable()).findsNothing(); + check(find.byType(ChooseAccountPage)).findsOne(); + check(find.byType(HomePage)).findsNothing(); + } + + ModalRoute? getRouteOf(WidgetTester tester, Finder elementFinder) => + ModalRoute.of(tester.element(elementFinder)); + + void checkOnHomePage(WidgetTester tester, {required Account expectedAccount}) { + check(find.byType(CircularProgressIndicator)).findsNothing(); + check(find.byType(ChooseAccountPage)).findsNothing(); + check(find.byType(HomePage).hitTestable()).findsOne(); + check(getRouteOf(tester, find.byType(HomePage))) + .isA().accountId.equals(expectedAccount.id); + } + + Future prepare(WidgetTester tester) async { + addTearDown(testBinding.reset); + await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); + await testBinding.globalStore.add(eg.otherAccount, eg.initialSnapshot()); + await tester.pumpWidget(const ZulipApp()); + await tester.pump(Duration.zero); // wait for the loading page + checkOnLoadingPage(); + } + + Future tapChooseAccount(WidgetTester tester) async { + await tester.tap(find.text('Try another account')); + await tester.pump(Duration.zero); // tap the button + await tester.pump(const Duration(milliseconds: 250)); // wait for animation + checkOnChooseAccountPage(); + } + + Future chooseAccountWithEmail(WidgetTester tester, String email) async { + await tester.tap(find.text(email)); + await tester.pump(Duration.zero); // tap the button + await tester.pump(const Duration(milliseconds: 350)); // wait for push & pop animations + checkOnLoadingPage(); + } + + testWidgets('smoke', (tester) async { + addTearDown(testBinding.reset); + testBinding.globalStore.loadPerAccountDuration = const Duration(seconds: 1); + await prepare(tester); + await tester.pump(const Duration(seconds: 1)); + checkOnHomePage(tester, expectedAccount: eg.selfAccount); + }); + + testWidgets('while loading, go back from ChooseAccountPage', (tester) async { + testBinding.globalStore.loadPerAccountDuration = const Duration(seconds: 1); + await prepare(tester); + await tapChooseAccount(tester); + + await tester.tap(find.byType(BackButton)); + await tester.pump(Duration.zero); // tap the button + await tester.pump(const Duration(milliseconds: 350)); // wait for pop animation + checkOnLoadingPage(); + + await tester.pump(const Duration(seconds: 1)); + checkOnHomePage(tester, expectedAccount: eg.selfAccount); + }); + + testWidgets('while loading, choose a different account', (tester) async { + testBinding.globalStore.loadPerAccountDuration = const Duration(seconds: 1); + await prepare(tester); + await tapChooseAccount(tester); + + testBinding.globalStore.loadPerAccountDuration = const Duration(seconds: 2); + await chooseAccountWithEmail(tester, eg.otherAccount.email); + + // The second loadAccount is still pending. + await tester.pump(const Duration(seconds: 1)); + checkOnLoadingPage(); + + // The second loadAccount finished. + await tester.pump(const Duration(seconds: 1)); + checkOnHomePage(tester, expectedAccount: eg.otherAccount); + }); + + testWidgets('while loading, choosing an account disallow going back', (tester) async { + testBinding.globalStore.loadPerAccountDuration = const Duration(seconds: 1); + await prepare(tester); + await tapChooseAccount(tester); + + // While still loading, choose a different account. + await chooseAccountWithEmail(tester, eg.otherAccount.email); + + // User cannot go back because the navigator stack + // was cleared after choosing an account. + check(getRouteOf(tester, find.byType(CircularProgressIndicator))) + .isNotNull().isFirst.isTrue(); + + await tester.pump(const Duration(seconds: 1)); // wait for loadAccount + checkOnHomePage(tester, expectedAccount: eg.otherAccount); + }); + + testWidgets('while loading, go to nested levels of ChooseAccountPage', (tester) async { + testBinding.globalStore.loadPerAccountDuration = const Duration(seconds: 5); + final thirdAccount = eg.account(user: eg.thirdUser); + await testBinding.globalStore.add(thirdAccount, eg.initialSnapshot()); + await prepare(tester); + + // While still loading the first account, choose a different account. + await tapChooseAccount(tester); + await chooseAccountWithEmail(tester, eg.otherAccount.email); + // User cannot go back because the navigator stack + // was cleared after choosing an account. + check(getRouteOf(tester, find.byType(CircularProgressIndicator))) + .isA() + ..isFirst.isTrue() + ..accountId.equals(eg.otherAccount.id); + + // While still loading the second account, choose a different account. + await tapChooseAccount(tester); + await chooseAccountWithEmail(tester, thirdAccount.email); + // User cannot go back because the navigator stack + // was cleared after choosing an account. + check(getRouteOf(tester, find.byType(CircularProgressIndicator))) + .isA() + ..isFirst.isTrue() + ..accountId.equals(thirdAccount.id); + + await tester.pump(const Duration(seconds: 5)); // wait for loadAccount + checkOnHomePage(tester, expectedAccount: thirdAccount); + }); + + testWidgets('after finishing loading, go back from ChooseAccountPage', (tester) async { + testBinding.globalStore.loadPerAccountDuration = const Duration(seconds: 1); + await prepare(tester); + await tapChooseAccount(tester); + + // Stall while on ChoooseAccountPage so that the account finished loading. + await tester.pump(const Duration(seconds: 1)); + checkOnChooseAccountPage(); + + await tester.tap(find.byType(BackButton)); + await tester.pump(Duration.zero); // tap the button + await tester.pump(const Duration(milliseconds: 350)); // wait for pop animation + checkOnHomePage(tester, expectedAccount: eg.selfAccount); + }); + + testWidgets('after finishing loading, choose the loaded account', (tester) async { + testBinding.globalStore.loadPerAccountDuration = const Duration(seconds: 5); + await prepare(tester); + await tapChooseAccount(tester); + + // Stall while on ChoooseAccountPage so that the account finished loading. + await tester.pump(const Duration(seconds: 5)); + checkOnChooseAccountPage(); + + // Choosing the already loaded account should result in no loading page. + await tester.tap(find.text(eg.selfAccount.email)); + await tester.pump(Duration.zero); // tap the button + await tester.pump(const Duration(milliseconds: 350)); // wait for push & pop animations + // No additional wait for loadAccount. + checkOnHomePage(tester, expectedAccount: eg.selfAccount); + }); + }); } diff --git a/test/widgets/login_test.dart b/test/widgets/login_test.dart index 676cef9ff5..e25913c0c5 100644 --- a/test/widgets/login_test.dart +++ b/test/widgets/login_test.dart @@ -70,6 +70,7 @@ void main() { group('LoginPage', () { late FakeApiConnection connection; late List> pushedRoutes; + late List> poppedRoutes; void takeStartingRoutes() { final expected = >[ @@ -89,8 +90,14 @@ void main() { zulipFeatureLevel: serverSettings.zulipFeatureLevel); pushedRoutes = []; - final testNavObserver = TestNavigatorObserver() - ..onPushed = (route, prevRoute) => pushedRoutes.add(route); + poppedRoutes = []; + final testNavObserver = TestNavigatorObserver(); + testNavObserver.onPushed = (route, prevRoute) => pushedRoutes.add(route); + testNavObserver.onPopped = (route, prevRoute) => poppedRoutes.add(route); + testNavObserver.onReplaced = (route, prevRoute) { + poppedRoutes.add(prevRoute!); + pushedRoutes.add(route!); + }; await tester.pumpWidget(ZulipApp(navigatorObservers: [testNavObserver])); await tester.pump(); final navigator = await ZulipApp.navigator; @@ -144,6 +151,25 @@ void main() { id: testBinding.globalStore.accounts.single.id)); }); + testWidgets('logging into a different account', (tester) async { + await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); + final serverSettings = eg.serverSettings(); + await prepare(tester, serverSettings); + check(poppedRoutes).isEmpty(); + check(pushedRoutes).deepEquals(>[ + (it) => it.isA().page.isA(), + (it) => it.isA().page.isA(), + ]); + pushedRoutes.clear(); + + await login(tester, eg.otherAccount); + final newAccount = testBinding.globalStore.accounts.singleWhere( + (account) => account != eg.selfAccount); + check(newAccount).equals(eg.otherAccount.copyWith(id: newAccount.id)); + check(poppedRoutes).length.equals(2); + check(pushedRoutes).single.isA().page.isA(); + }); + testWidgets('trims whitespace on username', (tester) async { final serverSettings = eg.serverSettings(); await prepare(tester, serverSettings); @@ -192,7 +218,6 @@ void main() { }); // TODO test validators on the TextFormField widgets - // TODO test navigation, i.e. the call to pushAndRemoveUntil // TODO test _getUserId case // TODO test handling failure in fetchApiKey request // TODO test _inProgress logic