refactor(cat-voices): delete keychain dialog (#3618)

* wip

* wip

* chore: show dialog on proper page

* chore: remove old way of showing confirmation dialog

* chore: cleanup

* fix: format

* chore: review update

* fix: format

* chore: rename dialog to buildDialog

Author: LynxLynxx

catalyst_voices/README.md

@@ -5,26 +5,26 @@
 This repository contains the Catalyst Voices app and packages.
 
 * [Catalyst Voices](#catalyst-voices)
-  * [Requirements](#requirements)
-  * [Recommended VS code plugins](#recommended-vs-code-plugins)
-  * [Platforms](#platforms)
-  * [Getting Started](#getting-started)
-    * [Bootstrapping](#bootstrapping)
-    * [Packages](#packages)
-    * [Environment Type vs Flavor](#environment-type-vs-flavor)
-    * [Environment types](#environment-types)
-      * [Stress Test](#stress-test)
-      * [Debug Performance Flags](#debug-performance-flags)
-    * [Flavor types](#flavor-types)
-    * [Environment variables](#environment-variables)
-      * [Environment config](#environment-config)
+    * [Requirements](#requirements)
+    * [Recommended VS code plugins](#recommended-vs-code-plugins)
+    * [Platforms](#platforms)
+    * [Getting Started](#getting-started)
+        * [Bootstrapping](#bootstrapping)
+        * [Packages](#packages)
+        * [Environment Type vs Flavor](#environment-type-vs-flavor)
+        * [Environment types](#environment-types)
+            * [Stress Test](#stress-test)
+            * [Debug Performance Flags](#debug-performance-flags)
+        * [Flavor types](#flavor-types)
+        * [Environment variables](#environment-variables)
+            * [Environment config](#environment-config)
     * [Code Generation](#code-generation)
-      * [Running Code Generation](#running-code-generation)
-        * [Basic Generation](#basic-generation)
-        * [Local Saving](#local-saving)
-      * [GitHub Token / PAT Setup](#github-token--pat-setup)
-      * [Security Notes](#security-notes)
-  * [Running Tests](#running-tests)
+            *[Running Code Generation](#running-code-generation)
+                * [Basic Generation](#basic-generation)
+                *[Local Saving](#local-saving)
+            * [GitHub Token / PAT Setup](#github-token--pat-setup)
+            * [Security Notes](#security-notes)
+    * [Running Tests](#running-tests)
 
 ## Requirements
 

catalyst_voices/apps/voices/lib/notification/catalyst_messenger.dart

@@ -5,6 +5,7 @@ import 'package:catalyst_voices/notification/banner_close_button.dart';
 import 'package:catalyst_voices/notification/banner_content.dart';
 import 'package:catalyst_voices/notification/catalyst_notification.dart';
 import 'package:catalyst_voices/routes/app_router_factory.dart';
+import 'package:catalyst_voices/widgets/modals/voices_dialog.dart';
 import 'package:catalyst_voices_assets/catalyst_voices_assets.dart';
 import 'package:catalyst_voices_shared/catalyst_voices_shared.dart';
 import 'package:flutter/material.dart';
@@ -37,9 +38,12 @@ class CatalystMessenger extends StatefulWidget {
 }
 
 class CatalystMessengerState extends State<CatalystMessenger> {
-  final _pending = <CatalystNotification>[];
+  final _pendingDialogs = <DialogNotification>[];
+  final _pendingBanners = <BannerNotification>[];
   bool _isShowingBanner = false;
+  bool _isShowingDialog = false;
   BannerNotification? _activeBanner;
+  DialogNotification? _activeDialog;
 
   GoRouter? __router;
 
@@ -50,18 +54,26 @@ class CatalystMessengerState extends State<CatalystMessenger> {
   /// Adds a notification to the queue if it is not already present.
   ///
   /// This method ensures that duplicate notifications are not added to the queue.
-  /// If the notification already exists in the `_pending` queue, it logs a message
-  /// and skips adding it. Otherwise, the notification is added to the queue in a
-  /// sorted order, and the queue is processed to display notifications.
+  /// Notifications are routed to the appropriate queue (dialogs or banners)
+  /// and sorted by priority within each queue.
   void add(CatalystNotification notification) {
-    if (_pending.contains(notification)) {
-      _logger.fine('$notification already in queue, skipping add');
-      return;
-    }
-
     _logger.finest('Adding $notification to queue');
 
-    _addSorted(notification);
+    switch (notification) {
+      case DialogNotification():
+        if (_pendingDialogs.any((n) => n.id == notification.id)) {
+          _logger.fine('$notification already in dialogs queue, skipping add');
+          return;
+        }
+        _addToQueue(notification);
+      case BannerNotification():
+        if (_pendingBanners.any((n) => n.id == notification.id)) {
+          _logger.fine('$notification already in banners queue, skipping add');
+          return;
+        }
+        _addToQueue(notification);
+    }
+
     _processQueue();
   }
 
@@ -71,7 +83,13 @@ class CatalystMessengerState extends State<CatalystMessenger> {
   }
 
   void cancelWhere(CatalystNotificationPredicate test) {
-    _pending.removeWhere(test);
+    _pendingDialogs.removeWhere(test);
+    _pendingBanners.removeWhere(test);
+
+    final activeDialog = _activeDialog;
+    if (activeDialog != null && test(activeDialog)) {
+      _hideCurrentDialog();
+    }
 
     final activeBanner = _activeBanner;
     if (activeBanner != null && test(activeBanner)) {
@@ -87,10 +105,18 @@ class CatalystMessengerState extends State<CatalystMessenger> {
     super.dispose();
   }
 
-  void _addSorted(CatalystNotification notification) {
-    _pending
-      ..add(notification)
-      ..sort();
+  void _addToQueue(CatalystNotification notification) {
+    switch (notification) {
+      case BannerNotification():
+        _pendingBanners
+          ..add(notification)
+          ..sort();
+        break;
+      case DialogNotification():
+        _pendingDialogs
+          ..add(notification)
+          ..sort();
+    }
   }
 
   GoRouter _findRouter() {
@@ -103,16 +129,24 @@ class CatalystMessengerState extends State<CatalystMessenger> {
   void _handleRouterChange() {
     final routerState = _router.state;
 
+    // Handle active dialog
+    final activeDialog = _activeDialog;
+    if (activeDialog != null && !activeDialog.routerPredicate(routerState)) {
+      _logger.finer('Hiding dialog(${activeDialog.id}). Not valid for router state');
+      _addToQueue(activeDialog);
+      _hideCurrentDialog();
+    }
+
     // Handle active banner
     final activeBanner = _activeBanner;
     if (activeBanner != null && !activeBanner.routerPredicate(routerState)) {
       _logger.finer('Hiding banner(${activeBanner.id}). Not valid for router state');
-      _addSorted(activeBanner);
+      _addToQueue(activeBanner);
       _hideCurrentBanner();
     }
 
     // Process queue if there are pending notifications
-    if (_pending.isNotEmpty) {
+    if (_pendingDialogs.isNotEmpty || _pendingBanners.isNotEmpty) {
       _processQueue();
     }
   }
@@ -126,6 +160,15 @@ class CatalystMessengerState extends State<CatalystMessenger> {
     messengerState.removeCurrentMaterialBanner(reason: MaterialBannerClosedReason.hide);
   }
 
+  /// Hiding current dialog will trigger _onDialogCompleted and process queue.
+  void _hideCurrentDialog() {
+    final navigatorContext = _router.routerDelegate.navigatorKey.currentContext;
+    if (navigatorContext == null) {
+      return;
+    }
+    Navigator.of(navigatorContext, rootNavigator: true).pop();
+  }
+
   void _onBannerCompleted() {
     assert(_activeBanner != null, 'Completed banner but active was null');
     final activeBanner = _activeBanner!;
@@ -138,29 +181,56 @@ class CatalystMessengerState extends State<CatalystMessenger> {
     _processQueue();
   }
 
+  void _onDialogCompleted() {
+    assert(_activeDialog != null, 'Completed dialog but active was null');
+    final activeDialog = _activeDialog!;
+
+    _logger.finer('Completed dialog $activeDialog');
+
+    _isShowingDialog = false;
+    _activeDialog = null;
+
+    _processQueue();
+  }
+
   void _processQueue() {
     final routerState = _router.state;
-    final allowed = _pending.where((notification) => notification.routerPredicate(routerState));
 
-    if (allowed.isEmpty) {
-      if (_pending.isNotEmpty) {
-        _logger.finest('Found ${_pending.length} notification but none allow for router state');
+    // Filter notifications that are allowed for current router state
+    final allowedDialogs = _pendingDialogs.where((n) => n.routerPredicate(routerState));
+    final allowedBanners = _pendingBanners.where((n) => n.routerPredicate(routerState));
+
+    if (allowedDialogs.isEmpty && allowedBanners.isEmpty) {
+      final totalPending = _pendingDialogs.length + _pendingBanners.length;
+      if (totalPending > 0) {
+        _logger.finest('Found $totalPending notification(s) but none allow for router state');
       }
       return;
     }
 
-    // Process banners
-    if (!_isShowingBanner) {
-      final banner = allowed.whereType<BannerNotification>().firstOrNull;
-      if (banner != null) {
-        _pending.removeWhere((element) => element.id == banner.id);
-        _activeBanner = banner;
-        _isShowingBanner = true;
+    // Get next notification respecting priority across both queues
+    final nextDialog = allowedDialogs.firstOrNull;
+    final nextBanner = allowedBanners.firstOrNull;
 
-        _logger.finer('Showing banner $banner');
+    // Banners and dialog can be shown at the same time
+    if (!_isShowingDialog && nextDialog != null) {
+      _pendingDialogs.removeWhere((element) => element.id == nextDialog.id);
+      _activeDialog = nextDialog;
+      _isShowingDialog = true;
 
-        unawaited(_showBanner(banner).whenComplete(_onBannerCompleted));
-      }
+      _logger.finer('Showing dialog $nextDialog');
+
+      unawaited(_showDialog(nextDialog).whenComplete(_onDialogCompleted));
+    }
+
+    if (!_isShowingBanner && nextBanner != null) {
+      _pendingBanners.removeWhere((element) => element.id == nextBanner.id);
+      _activeBanner = nextBanner;
+      _isShowingBanner = true;
+
+      _logger.finer('Showing banner $nextBanner');
+
+      unawaited(_showBanner(nextBanner).whenComplete(_onBannerCompleted));
     }
   }
 
@@ -190,4 +260,37 @@ class CatalystMessengerState extends State<CatalystMessenger> {
       },
     );
   }
+
+  Future<void> _showDialog(DialogNotification notification) async {
+    // Wait for all pending frames to complete. Navigation (especially with
+    // redirects) can take multiple frames. By waiting while Flutter has
+    // scheduled frames, we ensure navigation has fully completed.
+    await WidgetsBinding.instance.endOfFrame;
+
+    // Check if the dialog is still active after navigation
+    if (_activeDialog?.id != notification.id) {
+      return;
+    }
+
+    // Verify the dialog is still allowed for the current route
+    if (!notification.routerPredicate(_router.state)) {
+      _logger.finer('Dialog ${notification.id} no longer valid for current route after navigation');
+      return;
+    }
+
+    final navigatorContext = _router.routerDelegate.navigatorKey.currentContext;
+    if (navigatorContext == null || !navigatorContext.mounted) {
+      return;
+    }
+
+    final widget = notification.buildDialog(navigatorContext);
+
+    if (navigatorContext.mounted) {
+      await VoicesDialog.show<void>(
+        context: navigatorContext,
+        routeSettings: RouteSettings(name: '/dialog-${notification.id}'),
+        builder: (context) => widget,
+      );
+    }
+  }
 }

catalyst_voices/apps/voices/lib/notification/catalyst_notification.dart

@@ -6,6 +6,7 @@ import 'package:go_router/go_router.dart';
 
 part 'banner_notification.dart';
 part 'catalyst_notification_text.dart';
+part 'dialog_notification.dart';
 
 bool _alwaysAllowRouterPredicate(GoRouterState state) => true;
 

catalyst_voices/apps/voices/lib/notification/dialog_notification.dart

@@ -0,0 +1,12 @@
+part of 'catalyst_notification.dart';
+
+abstract base class DialogNotification extends CatalystNotification {
+  const DialogNotification({
+    required super.id,
+    super.priority,
+    super.routerPredicate,
+    super.type,
+  });
+
+  Widget buildDialog(BuildContext context);
+}

catalyst_voices/apps/voices/lib/notification/specialized/dialog/keychain_deleted_dialog_notification.dart

@@ -0,0 +1,38 @@
+import 'package:catalyst_voices/notification/catalyst_notification.dart';
+import 'package:catalyst_voices/routes/routing/root_route.dart';
+import 'package:catalyst_voices/widgets/buttons/voices_filled_button.dart';
+import 'package:catalyst_voices/widgets/modals/voices_info_dialog.dart';
+import 'package:catalyst_voices_assets/catalyst_voices_assets.dart';
+import 'package:catalyst_voices_localization/catalyst_voices_localization.dart';
+import 'package:flutter/material.dart';
+
+final class KeychainDeletedDialogNotification extends DialogNotification {
+  KeychainDeletedDialogNotification()
+    : super(
+        id: 'keychainDeleteDialog',
+        routerPredicate: (state) => RootRoute.rootRouteNameOptions.contains(state.name),
+      );
+
+  @override
+  Widget buildDialog(BuildContext context) {
+    return const _KeychainDeletedDialog();
+  }
+}
+
+class _KeychainDeletedDialog extends StatelessWidget {
+  const _KeychainDeletedDialog();
+
+  @override
+  Widget build(BuildContext context) {
+    return VoicesInfoDialog(
+      icon: VoicesAssets.icons.checkCircle.buildIcon(),
+      title: Text(context.l10n.keychainDeletedDialogTitle),
+      message: Text(context.l10n.keychainDeletedDialogSubtitle),
+      action: VoicesFilledButton(
+        key: const Key('KeychainDeletedDialogCloseButton'),
+        onTap: () => Navigator.of(context).pop(),
+        child: Text(context.l10n.close),
+      ),
+    );
+  }
+}