Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

notif: Use associated account as initial account; if opened from background #1240

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

notif: Use associated account as initial account; if opened from background #1240

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c961903
notif test [nfc]: Cleanup `TestPlatformDispatcher.defaultRouteNameTes…
rajveermalviya Jan 2, 2025
4d68974
app [nfc]: Pull out `_handleGenerateInitialRoutes`
rajveermalviya Jan 2, 2025
ae30cd3
notif: Use associated account as initial account; if opened from back…
rajveermalviya Dec 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 16 additions & 11 deletions lib/widgets/app.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,21 @@ class _ZulipAppState extends State<ZulipApp> with WidgetsBindingObserver {
return super.didPushRouteInformation(routeInformation);
}

InitialRouteListFactory _handleGenerateInitialRoutes(BuildContext context) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

app [nfc]: Pull out _handleGenerateInitialRoutes

This commit message is misleading 🙂; the change doesn't follow the common pattern of just moving some code out to a helper function for better organization. Putting the globalStore.accounts query inside the callback, instead of outside, is significant and worth mentioning; in fact it might not be NFC. The reason I gave in #1240 (comment) was:

Then in particular we don't have to think about what happens if the firstOrNull account is logged out between a call to _ZulipAppState.build and the time when Flutter decides to call this InitialRouteListFactory callback.

Does that seem right, or am I missing something?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be even clearer if it were split into two commits: one that changes when globalStore.accounts is queried, and one for making the helper function (the pure refactor).

final globalStore = GlobalStoreWidget.of(context);

return (String initialRoute) {
// TODO(#524) choose initial account as last one used
final initialAccountId = globalStore.accounts.firstOrNull?.id;
return [
if (initialAccountId == null)
MaterialWidgetRoute(page: const ChooseAccountPage())
else
HomePage.buildRoute(accountId: initialAccountId),
];
};
}

Future<void> _handleInitialRoute() async {
final initialRouteUrl = Uri.parse(WidgetsBinding.instance.platformDispatcher.defaultRouteName);
if (initialRouteUrl case Uri(scheme: 'zulip', host: 'notification')) {
Expand All @@ -177,9 +192,6 @@ class _ZulipAppState extends State<ZulipApp> with WidgetsBindingObserver {
final themeData = zulipThemeData(context);
return GlobalStoreWidget(
child: Builder(builder: (context) {
final globalStore = GlobalStoreWidget.of(context);
// TODO(#524) choose initial account as last one used
final initialAccountId = globalStore.accounts.firstOrNull?.id;
return MaterialApp(
title: 'Zulip',
localizationsDelegates: ZulipLocalizations.localizationsDelegates,
Expand All @@ -206,14 +218,7 @@ class _ZulipAppState extends State<ZulipApp> with WidgetsBindingObserver {
// like [Navigator.push], never mere names as with [Navigator.pushNamed].
onGenerateRoute: (_) => null,

onGenerateInitialRoutes: (_) {
return [
if (initialAccountId == null)
MaterialWidgetRoute(page: const ChooseAccountPage())
else
HomePage.buildRoute(accountId: initialAccountId),
];
});
onGenerateInitialRoutes: _handleGenerateInitialRoutes(context));
}));
}
}
Expand Down