From 6bc8bcc606a404202e1e8ef201c9466171109e04 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 6 Sep 2024 16:37:13 -0700 Subject: [PATCH] login: Support logging out of an account Fixes: #463 --- assets/l10n/app_en.arb | 4 ++++ lib/widgets/app.dart | 53 ++++++++++++++++++++++++++++++++++++++++++ lib/widgets/page.dart | 1 + lib/widgets/store.dart | 24 +++++++++++++++++-- 4 files changed, 80 insertions(+), 2 deletions(-) diff --git a/assets/l10n/app_en.arb b/assets/l10n/app_en.arb index b8af6e155f..f5473deeb3 100644 --- a/assets/l10n/app_en.arb +++ b/assets/l10n/app_en.arb @@ -19,6 +19,10 @@ "@chooseAccountPageTitle": { "description": "Title for ChooseAccountPage" }, + "chooseAccountPageLogOutButton": "Log out", + "@chooseAccountPageLogOutButton": { + "description": "Label for the 'Log out' button for an account on the choose-account page" + }, "chooseAccountButtonAddAnAccount": "Add an account", "@chooseAccountButtonAddAnAccount": { "description": "Label for ChooseAccountPage button to add an account" diff --git a/lib/widgets/app.dart b/lib/widgets/app.dart index dcc320c2dd..830732ae4c 100644 --- a/lib/widgets/app.dart +++ b/lib/widgets/app.dart @@ -8,6 +8,8 @@ import 'package:flutter_gen/gen_l10n/zulip_localizations.dart'; import '../log.dart'; import '../model/localizations.dart'; import '../model/narrow.dart'; +import '../model/store.dart'; +import '../notifications/receive.dart'; import 'about_zulip.dart'; import 'app_bar.dart'; import 'dialog.dart'; @@ -221,15 +223,64 @@ class ChooseAccountPage extends StatelessWidget { required Widget title, Widget? subtitle, }) { + final designVariables = DesignVariables.of(context); + final zulipLocalizations = ZulipLocalizations.of(context); return Card( clipBehavior: Clip.hardEdge, child: ListTile( title: title, subtitle: subtitle, + trailing: PopupMenuButton( + iconColor: designVariables.icon, + itemBuilder: (context) => [ + PopupMenuItem( + value: AccountItemOverflowMenuItem.logOut, + child: Text(zulipLocalizations.chooseAccountPageLogOutButton)), + ], + onSelected: (item) async { + switch (item) { + case AccountItemOverflowMenuItem.logOut: { + unawaited(_logOutAccount(context, accountId)); + } + } + }), + // The default trailing padding with M3 is 24px. Decrease by 12 because + // PopupMenuButton adds 12px padding on all sides of the "…" icon. + contentPadding: const EdgeInsetsDirectional.only(start: 16, end: 12), onTap: () => Navigator.push(context, HomePage.buildRoute(accountId: accountId)))); } + Future _logOutAccount(BuildContext context, int accountId) async { + final globalStore = GlobalStoreWidget.of(context); + + final account = globalStore.getAccount(accountId); + if (account == null) return; // TODO(log) + + // Unawaited, to not block removing the account on this request. + unawaited(_unregisterToken(globalStore, account)); + + await globalStore.removeAccount(accountId); + } + + Future _unregisterToken(GlobalStore globalStore, Account account) async { + // TODO(#322) use actual acked push token; until #322, this is just null. + final token = account.ackedPushToken + // Try the current token as a fallback; maybe the server has registered + // it and we just haven't recorded that fact in the client. + ?? NotificationService.instance.token.value; + if (token == null) return; + + final connection = globalStore.apiConnectionFromAccount(account); + try { + await NotificationService.unregisterToken(connection, token: token); + } catch (e) { + // TODO retry? handle failures? + } finally { + connection.close(); + } + } + @override Widget build(BuildContext context) { final zulipLocalizations = ZulipLocalizations.of(context); @@ -286,6 +337,8 @@ class ChooseAccountPageOverflowButton extends StatelessWidget { } } +enum AccountItemOverflowMenuItem { logOut } + class HomePage extends StatelessWidget { const HomePage({super.key}); diff --git a/lib/widgets/page.dart b/lib/widgets/page.dart index a039769d4f..2008c627a1 100644 --- a/lib/widgets/page.dart +++ b/lib/widgets/page.dart @@ -40,6 +40,7 @@ mixin AccountPageRouteMixin on PageRoute { return PerAccountStoreWidget( accountId: accountId, placeholder: const LoadingPlaceholderPage(), + routeToRemoveOnLogout: this, child: super.buildPage(context, animation, secondaryAnimation)); } } diff --git a/lib/widgets/store.dart b/lib/widgets/store.dart index 3e5165d998..ee583d9fd5 100644 --- a/lib/widgets/store.dart +++ b/lib/widgets/store.dart @@ -1,7 +1,9 @@ import 'package:flutter/material.dart'; +import 'package:flutter/scheduler.dart'; import '../model/binding.dart'; import '../model/store.dart'; +import 'page.dart'; /// Provides access to the app's data. /// @@ -112,11 +114,19 @@ class PerAccountStoreWidget extends StatefulWidget { super.key, required this.accountId, this.placeholder = const LoadingPlaceholder(), + this.routeToRemoveOnLogout, required this.child, }); final int accountId; final Widget placeholder; + + /// A per-account [Route] that should be removed on logout. + /// + /// Use this when the widget is a page on a route that should go away + /// when the account is logged out, instead of lingering with [placeholder]. + final AccountPageRouteMixin? routeToRemoveOnLogout; + final Widget child; /// The user's data for the relevant Zulip account for this widget. @@ -195,6 +205,16 @@ class _PerAccountStoreWidgetState extends State { void didChangeDependencies() { super.didChangeDependencies(); final globalStore = GlobalStoreWidget.of(context); + final accountExists = globalStore.getAccount(widget.accountId) != null; + if (!accountExists) { + // logged out + _setStore(null); + if (widget.routeToRemoveOnLogout != null) { + SchedulerBinding.instance.addPostFrameCallback( + (_) => Navigator.of(context).removeRoute(widget.routeToRemoveOnLogout!)); + } + return; + } // If we already have data, get it immediately. This avoids showing one // frame of loading indicator each time we have a new PerAccountStoreWidget. final store = globalStore.perAccountSync(widget.accountId); @@ -212,13 +232,13 @@ class _PerAccountStoreWidgetState extends State { // The account was logged out while its store was loading. // This widget will be showing [placeholder] perpetually, // but that's OK as long as other code will be removing it from the UI - // (for example by removing a per-account route from the nav). + // (usually by using [routeToRemoveOnLogout]). } }(); } } - void _setStore(PerAccountStore store) { + void _setStore(PerAccountStore? store) { if (store != this.store) { setState(() { this.store = store;