Skip to content

Commit

Permalink
rework nav wip
Browse files Browse the repository at this point in the history
Signed-off-by: Zixuan James Li <[email protected]>
  • Loading branch information
PIG208 committed Jan 8, 2025
1 parent ab371f7 commit 7a93845
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 70 deletions.
3 changes: 3 additions & 0 deletions lib/model/store.dart
Original file line number Diff line number Diff line change
Expand Up @@ -152,9 +152,12 @@ abstract class GlobalStore extends ChangeNotifier {
assert(_accounts.containsKey(accountId));
final PerAccountStore store;
try {
// await Future<void>.delayed(const Duration(seconds: 2));
// throw AccountNotFoundException();
store = await doLoadPerAccount(accountId);
} catch (e) {
switch (e) {
case AccountNotFoundException():
case ZulipApiException(code: 'INVALID_API_KEY'):
// The API key is invalid and the store can never be loaded
// unless the user retries manually.
Expand Down
36 changes: 35 additions & 1 deletion lib/widgets/app.dart
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,11 @@ class _ZulipAppState extends State<ZulipApp> with WidgetsBindingObserver {
@override
Widget build(BuildContext context) {
final themeData = zulipThemeData(context);
final effectiveNavigatorObservers = [
_EmptyStackNavigatorObserver(),
if (widget.navigatorObservers != null)
...widget.navigatorObservers!,
];
return GlobalStoreWidget(
child: Builder(builder: (context) {
final globalStore = GlobalStoreWidget.of(context);
Expand All @@ -199,7 +204,7 @@ class _ZulipAppState extends State<ZulipApp> with WidgetsBindingObserver {
theme: themeData,

navigatorKey: ZulipApp.navigatorKey,
navigatorObservers: widget.navigatorObservers ?? const [],
navigatorObservers: effectiveNavigatorObservers,
builder: (BuildContext context, Widget? child) {
if (!ZulipApp.ready.value) {
SchedulerBinding.instance.addPostFrameCallback(
Expand Down Expand Up @@ -230,6 +235,35 @@ class _ZulipAppState extends State<ZulipApp> with WidgetsBindingObserver {
}
}

class _EmptyStackNavigatorObserver extends NavigatorObserver {
void _pushIfEmpty() async {
final navigator = await ZulipApp.navigator;
bool isEmptyStack = true;
navigator.popUntil((route) {
isEmptyStack = false;
return true; // never actually pops
});
if (isEmptyStack) {
unawaited(navigator.push(
MaterialWidgetRoute(page: const ChooseAccountPage())));
}
}

@override
void didRemove(Route<void> route, Route<void>? previousRoute) async {
if (previousRoute == null) {
_pushIfEmpty();
}
}

@override
void didPop(Route<void> route, Route<void>? previousRoute) async {
if (previousRoute == null) {
_pushIfEmpty();
}
}
}

class ChooseAccountPage extends StatelessWidget {
const ChooseAccountPage({super.key});

Expand Down
10 changes: 1 addition & 9 deletions lib/widgets/store.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import 'package:flutter/scheduler.dart';

import '../model/binding.dart';
import '../model/store.dart';
import 'app.dart';
import 'page.dart';

/// Provides access to the app's data.
Expand Down Expand Up @@ -231,14 +230,7 @@ class _PerAccountStoreWidgetState extends State<PerAccountStoreWidget> {
SchedulerBinding.instance.addPostFrameCallback(
(_) {
final navigator = Navigator.of(context);
if (widget.routeToRemoveOnLogout!.isFirst) {
// This ensures that we never remove the root navigator level.
navigator.replace(
oldRoute: widget.routeToRemoveOnLogout!,
newRoute: MaterialWidgetRoute(page: const ChooseAccountPage()));
} else {
navigator.removeRoute(widget.routeToRemoveOnLogout!);
}
navigator.removeRoute(widget.routeToRemoveOnLogout!);
});
}
return;
Expand Down
90 changes: 30 additions & 60 deletions test/widgets/store_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import 'package:zulip/api/exception.dart';
import 'package:zulip/model/store.dart';
import 'package:zulip/widgets/app.dart';
import 'package:zulip/widgets/home.dart';
import 'package:zulip/widgets/message_list.dart';
import 'package:zulip/widgets/page.dart';
import 'package:zulip/widgets/store.dart';

Expand All @@ -14,7 +13,6 @@ import '../model/binding.dart';
import '../example_data.dart' as eg;
import '../model/store_checks.dart';
import '../model/test_store.dart';
import '../notifications/display_test.dart';
import '../test_navigation.dart';
import 'dialog_checks.dart';
import 'page_checks.dart';
Expand Down Expand Up @@ -123,14 +121,10 @@ void main() {
.contains('consider MaterialAccountWidgetRoute');
});

testWidgets('PerAccountStoreWidget when log out, no duplicate choose-account page', (tester) async {
Route<void>? newRoute, oldRoute;
final testNavObserver = TestNavigatorObserver()
..onReplaced = (route, prevRoute) {
assert(newRoute == null && oldRoute == null);
newRoute = route!;
oldRoute = prevRoute!;
};
testWidgets('PerAccountStoreWidget when log out, do not push route to non-empty navigator stack', (tester) async {
addTearDown(testBinding.reset);

final testNavObserver = TestNavigatorObserver();
// A route to remove is a root level home route
const loadPerAccountDuration = Duration(seconds: 30);
assert(loadPerAccountDuration > kTryAnotherAccountWaitPeriod);
Expand All @@ -155,77 +149,53 @@ void main() {
await tester.pump(loadPerAccountDuration); // got the error
await tester.pump(TestGlobalStore.removeAccountDuration);

checkErrorDialog(tester,
expectedTitle: 'Could not connect',
expectedMessage:
'Your account at https://chat.example/ could not be authenticated.'
' Please try logging in again or use another account.');
// Ended up with two choose-account page routes on the stack, not good.
check(newRoute).isA<WidgetRoute>().page.isA<ChooseAccountPage>();
check(oldRoute).isA<WidgetRoute>().page.isA<HomePage>();
check(pushedRoutes).single.isA<DialogRoute<void>>();
pushedRoutes.clear();
check(removedRoutes).single.isA<WidgetRoute>().page.isA<HomePage>();
check(testBinding.globalStore.takeDoRemoveAccountCalls())
.single.equals(eg.selfAccount.id);
});

testWidgets('PerAccountStoreWidget when log out, replace root level route', (tester) async {
Route<void>? newRoute, oldRoute;
final testNavObserver = TestNavigatorObserver()
..onReplaced = (route, prevRoute) {
assert(newRoute == null && oldRoute == null);
newRoute = route!;
oldRoute = prevRoute!;
};
// A route to remove is a root level home route
testBinding.globalStore.loadPerAccountException = ZulipApiException(
routeName: '/register', code: 'INVALID_API_KEY', httpStatus: 400,
data: {}, message: '');
await testBinding.globalStore.insertAccount(eg.selfAccount.toCompanion(false));
await tester.pumpWidget(ZulipApp(navigatorObservers: [testNavObserver]));
await tester.pump(); // start to load account
await tester.pump(); // got the error
await tester.pump(TestGlobalStore.removeAccountDuration);

checkErrorDialog(tester,
await tester.tap(find.byWidget(checkErrorDialog(tester,
expectedTitle: 'Could not connect',
expectedMessage:
'Your account at https://chat.example/ could not be authenticated.'
' Please try logging in again or use another account.');
check(newRoute).isA<WidgetRoute>().page.isA<ChooseAccountPage>();
check(oldRoute).isA<WidgetRoute>().page.isA<HomePage>();
check(testBinding.globalStore.takeDoRemoveAccountCalls())
.single.equals(eg.selfAccount.id);
' Please try logging in again or use another account.')));
check(pushedRoutes).isEmpty();
});

testWidgets('PerAccountStoreWidget when log out, remove non-root level route', (tester) async {
final testNavObserver = TestNavigatorObserver();
await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
await testBinding.globalStore.insertAccount(eg.otherAccount.toCompanion(false));
await tester.pumpWidget(ZulipApp(navigatorObservers: [testNavObserver]));
await tester.pump(); // start to load selfAccount
await tester.pump(); // done loading
testWidgets('PerAccountStoreWidget when log out, push route when popping root level route', (tester) async {
addTearDown(testBinding.reset);

final testNavObserver = TestNavigatorObserver();
final pushedRoutes = <Route<void>>[];
final removedRoutes = <Route<void>>[];
testNavObserver.onPushed = (route, prevRoute) => pushedRoutes.add(route);
testNavObserver.onRemoved = (route, prevRoute) => removedRoutes.add(route);
// A route to remove is a root level home route
testBinding.globalStore.loadPerAccountDuration = Duration.zero;
testBinding.globalStore.loadPerAccountException = ZulipApiException(
routeName: '/register', code: 'INVALID_API_KEY', httpStatus: 400,
data: {}, message: '');
await openNotification(tester, eg.otherAccount, eg.streamMessage());
await tester.pump();
await testBinding.globalStore.insertAccount(eg.selfAccount.toCompanion(false));
await tester.pumpWidget(ZulipApp(navigatorObservers: [testNavObserver]));
await tester.pump(); // start to load account
check(pushedRoutes).single.isA<WidgetRoute>().page.isA<HomePage>();
pushedRoutes.clear();
await tester.pump(); // got the error
await tester.pump(TestGlobalStore.removeAccountDuration);

checkErrorDialog(tester,
check(pushedRoutes).single.isA<DialogRoute<void>>();
pushedRoutes.clear();
check(removedRoutes).single.isA<WidgetRoute>().page.isA<HomePage>();
check(testBinding.globalStore.takeDoRemoveAccountCalls())
.single.equals(eg.selfAccount.id);

await tester.tap(find.byWidget(checkErrorDialog(tester,
expectedTitle: 'Could not connect',
expectedMessage:
'Your account at https://chat.example/ could not be authenticated.'
' Please try logging in again or use another account.');
check(testBinding.globalStore.takeDoRemoveAccountCalls())
.single.equals(eg.otherAccount.id);
check(pushedRoutes).deepEquals(<Condition<Object?>>[
(it) => it.isA<WidgetRoute>().page.isA<MessageListPage>(),
(it) => it.isA<DialogRoute<void>>(),
]);
' Please try logging in again or use another account.')));
check(pushedRoutes).single.isA<WidgetRoute>().page.isA<ChooseAccountPage>();
});

testWidgets('PerAccountStoreWidget immediate data after first loaded', (tester) async {
Expand Down

0 comments on commit 7a93845

Please sign in to comment.