Skip to content

Commit d4d4d49

Browse files
committed
app: Maintain that the navigator stack is never empty
Signed-off-by: Zixuan James Li <[email protected]>
1 parent e7a0e49 commit d4d4d49

File tree

2 files changed

+74
-1
lines changed

2 files changed

+74
-1
lines changed

lib/widgets/app.dart

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,13 @@ class _ZulipAppState extends State<ZulipApp> with WidgetsBindingObserver {
199199
theme: themeData,
200200

201201
navigatorKey: ZulipApp.navigatorKey,
202-
navigatorObservers: widget.navigatorObservers ?? const [],
202+
navigatorObservers: [
203+
if (widget.navigatorObservers != null)
204+
...widget.navigatorObservers!,
205+
// This must be the last item to maintain the invariant
206+
// that the navigator stack is always non-empty.
207+
_EmptyStackNavigatorObserver(),
208+
],
203209
builder: (BuildContext context, Widget? child) {
204210
if (!ZulipApp.ready.value) {
205211
SchedulerBinding.instance.addPostFrameCallback(
@@ -230,6 +236,39 @@ class _ZulipAppState extends State<ZulipApp> with WidgetsBindingObserver {
230236
}
231237
}
232238

239+
/// Pushes a route whenever the observed navigator stack becomes empty.
240+
class _EmptyStackNavigatorObserver extends NavigatorObserver {
241+
void _pushRouteIfEmptyStack() async {
242+
final navigator = await ZulipApp.navigator;
243+
bool isEmptyStack = true;
244+
// TODO: find a better way to inspect the navigator stack
245+
navigator.popUntil((route) {
246+
isEmptyStack = false;
247+
return true; // never actually pops
248+
});
249+
if (isEmptyStack) {
250+
unawaited(navigator.push(
251+
MaterialWidgetRoute(page: const ChooseAccountPage())));
252+
}
253+
}
254+
255+
@override
256+
void didRemove(Route<void> route, Route<void>? previousRoute) async {
257+
if (previousRoute == null) {
258+
// The route removed is the bottom-most one.
259+
_pushRouteIfEmptyStack();
260+
}
261+
}
262+
263+
@override
264+
void didPop(Route<void> route, Route<void>? previousRoute) async {
265+
if (previousRoute == null) {
266+
// The route popped is the bottom-most one.
267+
_pushRouteIfEmptyStack();
268+
}
269+
}
270+
}
271+
233272
class ChooseAccountPage extends StatelessWidget {
234273
const ChooseAccountPage({super.key});
235274

test/widgets/app_test.dart

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import 'package:checks/checks.dart';
44
import 'package:flutter/material.dart';
55
import 'package:flutter_test/flutter_test.dart';
66
import 'package:zulip/log.dart';
7+
import 'package:zulip/model/actions.dart';
78
import 'package:zulip/model/database.dart';
89
import 'package:zulip/widgets/app.dart';
910
import 'package:zulip/widgets/home.dart';
@@ -57,6 +58,39 @@ void main() {
5758
});
5859
});
5960

61+
group('_EmptyStackNavigatorObserver', () {
62+
late List<Route<void>> pushedRoutes;
63+
late List<Route<void>> removedRoutes;
64+
65+
Future<void> prepare(WidgetTester tester) async {
66+
addTearDown(testBinding.reset);
67+
68+
pushedRoutes = [];
69+
removedRoutes = [];
70+
final testNavObserver = TestNavigatorObserver();
71+
testNavObserver.onPushed = (route, prevRoute) => pushedRoutes.add(route);
72+
testNavObserver.onRemoved = (route, prevRoute) => removedRoutes.add(route);
73+
74+
await tester.pumpWidget(ZulipApp(navigatorObservers: [testNavObserver]));
75+
await tester.pump(); // start to load account
76+
check(pushedRoutes).single.isA<WidgetRoute>().page.isA<HomePage>();
77+
pushedRoutes.clear();
78+
}
79+
80+
testWidgets('push route when removing last route on stack', (tester) async {
81+
await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
82+
await prepare(tester);
83+
84+
final future = logOutAccount(testBinding.globalStore, eg.selfAccount.id);
85+
await tester.pump(TestGlobalStore.removeAccountDuration);
86+
await future;
87+
check(pushedRoutes).single.isA<WidgetRoute>().page.isA<ChooseAccountPage>();
88+
check(removedRoutes).single.isA<WidgetRoute>().page.isA<HomePage>();
89+
check(testBinding.globalStore.takeDoRemoveAccountCalls())
90+
.single.equals(eg.selfAccount.id);
91+
});
92+
});
93+
6094
group('ChooseAccountPage', () {
6195
Future<void> setupChooseAccountPage(WidgetTester tester, {
6296
required List<Account> accounts,

0 commit comments

Comments
 (0)