Skip to content

feat(us2): phase 4 — secret words backup & notification bell - #53

Merged
grunch merged 4 commits into
mainfrom
004-mostro-p2p-client
Mar 30, 2026
Merged

feat(us2): phase 4 — secret words backup & notification bell#53
grunch merged 4 commits into
mainfrom
004-mostro-p2p-client

Conversation

@grunch

@grunch grunch commented Mar 29, 2026

Copy link
Copy Markdown
Member

backup_reminder_provider.dart:

  • Full SharedPreferences-backed BackupReminderNotifier with showBackupReminder() / confirmBackupComplete(); replaces the in-memory stub from first_run_provider.dart (now re-exported)

notification_bell.dart:

  • AppBar bell widget with two visual states: red dot (backup active, no number) and dark-gold pill badge (unread count after backup)
  • Left-right shake animation via TweenSequence triggered reactively through ref.listen on both backupReminderProvider and unreadNotificationCountProvider

account_screen.dart (/key_management):

  • Secret Words card: masked mnemonic (first 2 + last 2 words visible, middle dots), Show/Hide toggle, calls confirmBackupComplete() on reveal
  • Privacy card: Reputation Mode / Full Privacy Mode radio buttons
  • Generate New User (confirmation dialog), Import User (mnemonic input dialog), Refresh User (confirmation dialog)

notifications_screen.dart (/notifications):

  • Pinned backup reminder card (red border, gavel icon) → /key_management
  • Per-notification cards with type icons, relative timestamps, mark-as-read inline action, long-press context menu
  • Overflow menu: Mark all as read / Clear all
  • Empty state with bell-slash icon

Supporting: NotificationModel, NotificationsNotifier, unreadNotificationCountProvider; app_routes.dart wired to real screens

Summary by CodeRabbit

  • New Features

    • Account screen with Secret Words reveal/hide flow, privacy options, and identity actions
    • Notifications screen with list, read/mark/delete actions, contextual navigation, and pinned backup reminder
    • App-bar notification bell showing backup reminder dot or unread count with attention animation
    • In-app notifications model/provider and unread-count provider
    • Persistent backup reminder provider; routes now show real Account and Notifications screens
    • New gold badge color token for theming
  • Bug Fixes

    • First-run flow now treats storage failures as completed
  • Documentation

    • Task checklist updated to mark related items complete

backup_reminder_provider.dart:
- Full SharedPreferences-backed BackupReminderNotifier with
  showBackupReminder() / confirmBackupComplete(); replaces the
  in-memory stub from first_run_provider.dart (now re-exported)

notification_bell.dart:
- AppBar bell widget with two visual states: red dot (backup active,
  no number) and dark-gold pill badge (unread count after backup)
- Left-right shake animation via TweenSequence triggered reactively
  through ref.listen on both backupReminderProvider and
  unreadNotificationCountProvider

account_screen.dart (/key_management):
- Secret Words card: masked mnemonic (first 2 + last 2 words visible,
  middle dots), Show/Hide toggle, calls confirmBackupComplete() on reveal
- Privacy card: Reputation Mode / Full Privacy Mode radio buttons
- Generate New User (confirmation dialog), Import User (mnemonic
  input dialog), Refresh User (confirmation dialog)

notifications_screen.dart (/notifications):
- Pinned backup reminder card (red border, gavel icon) → /key_management
- Per-notification cards with type icons, relative timestamps,
  mark-as-read inline action, long-press context menu
- Overflow menu: Mark all as read / Clear all
- Empty state with bell-slash icon

Supporting: NotificationModel, NotificationsNotifier,
unreadNotificationCountProvider; app_routes.dart wired to real screens
@coderabbitai

coderabbitai Bot commented Mar 29, 2026

Copy link
Copy Markdown
Contributor

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c88e5c83-3fde-4979-a84b-272361cbe8a9

📥 Commits

Reviewing files that changed from the base of the PR and between 7da91d8 and 72a8bf1.

📒 Files selected for processing (3)
  • lib/core/app_theme.dart
  • lib/features/account/screens/account_screen.dart
  • lib/shared/widgets/notification_bell.dart

Walkthrough

Replaces routing stubs with real Account and Notifications screens; adds a persisted backup reminder provider, in-memory notifications model/provider, notification bell widget with animated badges, moves backup provider to account providers and re-exports it, and implements Account and Notifications UIs and interactions.

Changes

Cohort / File(s) Summary
Routing
lib/core/app_routes.dart
Replaced stub builders to return AccountScreen and NotificationsScreen; added imports.
Account feature
lib/features/account/providers/backup_reminder_provider.dart, lib/features/account/screens/account_screen.dart
Added backupReminderProvider (StateNotifier) persisted via SharedPreferences with load/showBackupReminder/confirmBackupComplete; new AccountScreen with secret-words reveal/mask flow, privacy UI, and identity action dialogs (bridge TODOs).
Notifications models & providers
lib/features/notifications/models/notification_model.dart, lib/features/notifications/providers/notifications_provider.dart
Added NotificationType enum and NotificationModel with copyWith; added NotificationsNotifier and notificationsProvider plus unreadNotificationCountProvider and methods to add/mark/delete notifications.
Notifications UI
lib/features/notifications/screens/notifications_screen.dart
Added NotificationsScreen that watches backupReminderProvider and notificationsProvider, shows pinned backup card, lists notifications with tap/long-press actions, and provides overflow actions (mark all read / clear all).
Shared widget
lib/shared/widgets/notification_bell.dart
Added NotificationBell (animated shake) that reads backupReminderProvider and unreadNotificationCountProvider, shows red dot or gold unread-count pill, updates semantics/tooltip, listens for changes to trigger animation, and navigates to notifications route.
Walkthrough provider refactor
lib/features/walkthrough/providers/first_run_provider.dart
Removed local backup provider, re-exported backupReminderProvider from account providers; changed FirstRunNotifier._load() to treat SharedPreferences failures as completed (AsyncValue.data(true)).
Theme
lib/core/app_theme.dart
Added new AppColors.badgeGold token; updated constructor, copyWith, and lerp; defined light/dark values.
Specs
specs/004-mostro-p2p-client/tasks.md
Marked tasks T025–T028 as completed (task status only).

Sequence Diagrams

sequenceDiagram
    participant User
    participant AccountScreen
    participant BackupReminderNotifier
    participant SharedPreferences

    User->>AccountScreen: Tap "Show Secret Words"
    AccountScreen->>AccountScreen: begin reveal flow (loading)
    AccountScreen->>BackupReminderNotifier: confirmBackupComplete()
    BackupReminderNotifier->>SharedPreferences: set dismissed flag
    BackupReminderNotifier->>BackupReminderNotifier: update state -> false
    BackupReminderNotifier->>AccountScreen: notify state changed
Loading
sequenceDiagram
    participant User
    participant NotificationsScreen
    participant NotificationsNotifier
    participant BackupReminderNotifier
    participant Navigator

    User->>NotificationsScreen: Open screen
    NotificationsScreen->>BackupReminderNotifier: watch state
    NotificationsScreen->>NotificationsNotifier: watch notifications list
    NotificationsScreen->>NotificationsScreen: render pinned backup card & list
    User->>NotificationsScreen: Tap notification
    NotificationsScreen->>NotificationsNotifier: markAsRead(id)
    NotificationsNotifier->>NotificationsNotifier: update notification isRead
    NotificationsScreen->>Navigator: navigate to detail (order/dispute) if present
    User->>NotificationsScreen: Long-press notification
    NotificationsScreen->>NotificationsScreen: show action sheet
    User->>NotificationsScreen: Select "Delete"
    NotificationsScreen->>NotificationsNotifier: delete(id)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

Poem

🐰 I hopped in code with a giggle and grin,

Stubs turned to screens, bright badges begin,
Secret words peek, a gentle reminder fades,
Bells that shimmy for unread parades,
Hop, tap, and tidy — carrots for dev brigade! 🥕

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: implementing backup reminder functionality and a notification bell widget, which are the primary features added in this PR.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 004-mostro-p2p-client

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (6)
lib/shared/widgets/notification_bell.dart (2)

93-93: Add const to _RedDot() instantiation.

_RedDot is a stateless widget with no parameters, so its instantiation can be const for a minor performance improvement.

♻️ Proposed fix
                 child: backupActive
-                    ? _RedDot()
+                    ? const _RedDot()
                     : _CountBadge(
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/shared/widgets/notification_bell.dart` at line 93, The instantiation of
the stateless widget _RedDot should be made const for a small performance
benefit; locate the place where _RedDot() is returned (inside the notification
bell widget branch) and change the constructor call to const _RedDot() so the
widget can be canonicalized and avoid unnecessary rebuild work.

120-124: Remove unused colors parameter from _CountBadge.

The colors parameter is passed but never used — the badge uses a hardcoded Color(0xFFB8860B) instead. Either remove the parameter or use colors for theming consistency.

♻️ Option A: Remove unused parameter
 class _CountBadge extends StatelessWidget {
-  const _CountBadge({required this.count, required this.colors});
+  const _CountBadge({required this.count});

   final int count;
-  final AppColors? colors;

And at the call site:

                     : _CountBadge(
                         count: unreadCount,
-                        colors: colors,
                       ),

Also applies to: 131-135

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/shared/widgets/notification_bell.dart` around lines 120 - 124, The
_CountBadge widget declares an unused AppColors? colors parameter and field;
remove the unused parameter and field from the _CountBadge constructor and class
(references: class _CountBadge, const _CountBadge({required this.count, required
this.colors}); final AppColors? colors;) and update all call sites to stop
passing a colors argument (or alternatively, if theming is desired, replace the
hardcoded Color(0xFFB8860B) inside _CountBadge with the appropriate property
from the provided AppColors and keep the parameter). Ensure constructors and
callers are consistent after the change.
lib/features/walkthrough/providers/first_run_provider.dart (1)

29-32: Error handling does not match the stated fail-safe intent.

The comment says "Fail-safe: treat as completed so the user reaches the home screen," but setting AsyncValue.error(e, st) will cause firstRunAsync.when() in app_routes.dart to return null (no redirect). This works coincidentally because null means "no redirect" and the user lands on / (home).

However, the comment implies the intent is to treat the user as having completed the walkthrough. If that's the case, consider setting AsyncValue.data(true) instead for clarity:

♻️ Proposed fix for clearer fail-safe behavior
     } catch (e, st) {
-      // Fail-safe: treat as completed so the user reaches the home screen.
-      state = AsyncValue.error(e, st);
+      // Fail-safe: treat as completed so the user reaches the home screen
+      // (avoids trapping users in a broken walkthrough loop).
+      state = const AsyncValue.data(true);
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/features/walkthrough/providers/first_run_provider.dart` around lines 29 -
32, The catch block in the first run provider currently sets state =
AsyncValue.error(e, st) which contradicts the fail-safe comment and relies on
incidental routing behavior; update the handler in the provider (the catch in
the function that sets state) to set state = AsyncValue.data(true) so the app
treats the walkthrough as completed (matching the comment and the logic used by
firstRunAsync.when() in app_routes.dart), and adjust the comment to reflect this
explicit intentional fail-safe behavior.
lib/features/notifications/screens/notifications_screen.dart (1)

285-291: Consider guarding against future timestamps.

If notification.timestamp is somehow in the future (e.g., clock skew), diff would be negative and none of the conditions would match, causing an incorrect "0d ago" result.

♻️ Proposed defensive fix
   String _relativeTime(DateTime dt) {
     final diff = DateTime.now().difference(dt);
+    if (diff.isNegative) return 'Just now';
     if (diff.inMinutes < 1) return 'Just now';
     if (diff.inMinutes < 60) return '${diff.inMinutes}m ago';
     if (diff.inHours < 24) return '${diff.inHours}h ago';
     return '${diff.inDays}d ago';
   }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/features/notifications/screens/notifications_screen.dart` around lines
285 - 291, The _relativeTime(DateTime dt) helper doesn't handle future
timestamps (negative Duration) which can produce incorrect output; update
_relativeTime to detect if DateTime.now().difference(dt) is negative
(diff.isNegative) and return a sensible value such as 'Just now' (or clamp to 0m
ago) before the existing minute/hour/day checks so future timestamps are treated
as immediate rather than producing "0d ago".
lib/features/account/screens/account_screen.dart (1)

289-317: TextEditingController in dialog should be disposed.

The TextEditingController created at line 290 is not disposed. While the dialog's short lifecycle makes this low-risk, explicit disposal is a best practice to avoid potential memory leaks.

♻️ Proposed fix using StatefulBuilder
   void _showImportDialog(BuildContext context) {
-    final controller = TextEditingController();
     showDialog<void>(
       context: context,
-      builder: (_) => AlertDialog(
+      builder: (_) => _ImportDialog(),
+    );
+  }
+}
+
+class _ImportDialog extends StatefulWidget {
+  `@override`
+  State<_ImportDialog> createState() => _ImportDialogState();
+}
+
+class _ImportDialogState extends State<_ImportDialog> {
+  final _controller = TextEditingController();
+
+  `@override`
+  void dispose() {
+    _controller.dispose();
+    super.dispose();
+  }
+
+  `@override`
+  Widget build(BuildContext context) {
+    return AlertDialog(
         title: const Text('Import Mnemonic'),
         content: TextField(
-          controller: controller,
+          controller: _controller,
           maxLines: 3,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/features/account/screens/account_screen.dart` around lines 289 - 317, The
TextEditingController created in _showImportDialog is not disposed; create the
controller as you already do but ensure you dispose it after the dialog closes
by attaching a completion handler to showDialog (e.g., showDialog(...).then((_)
=> controller.dispose())), or alternatively instantiate the controller inside
the dialog builder and dispose it when the dialog is popped; reference the
TextEditingController, _showImportDialog and showDialog to locate where to add
the disposal so the controller is properly disposed on dialog dismissal.
lib/features/account/providers/backup_reminder_provider.dart (1)

4-4: Extract 'backupReminderActive' to a constant for consistency.

_kBackupReminderDismissed is defined as a constant, but 'backupReminderActive' is used as an inline string in two places (lines 22 and 29). This inconsistency could lead to typos and maintenance issues.

♻️ Proposed fix
 const _kBackupReminderDismissed = 'backupReminderDismissed';
+const _kBackupReminderActive = 'backupReminderActive';
 
 /// Tracks whether the backup reminder (red dot on notification bell) is active.

Then replace inline strings:

-    final active = prefs.getBool('backupReminderActive') ?? false;
+    final active = prefs.getBool(_kBackupReminderActive) ?? false;
-    await prefs.setBool('backupReminderActive', true);
+    await prefs.setBool(_kBackupReminderActive, true);

Also applies to: 22-22, 29-29

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/features/account/providers/backup_reminder_provider.dart` at line 4, Add
a new constant _kBackupReminderActive = 'backupReminderActive' (to mirror the
existing _kBackupReminderDismissed) and replace the two inline occurrences of
'backupReminderActive' with the new constant; update any code in
BackupReminderProvider (or functions/methods that currently reference the inline
string) to use _kBackupReminderActive to ensure consistency and avoid typos.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@lib/features/account/providers/backup_reminder_provider.dart`:
- Around line 10-13: The provider calls BackupReminderNotifier.load()
asynchronously via cascade and doesn't await it, so showBackupReminder() can
read a stale dismissed value; fix by making loading explicit and awaited: remove
the cascade call, turn load() into a public Future<void> that sets an internal
bool _loaded = true when finished (and returns early if _loaded), then update
showBackupReminder() to await ref.read(backupReminderProvider.notifier).load()
(or call a new ensureLoaded() that awaits load()) before reading dismissed; keep
the provider as StateNotifierProvider and ensure all mutation/read paths use the
awaited load/ensureLoaded to avoid the race.

---

Nitpick comments:
In `@lib/features/account/providers/backup_reminder_provider.dart`:
- Line 4: Add a new constant _kBackupReminderActive = 'backupReminderActive' (to
mirror the existing _kBackupReminderDismissed) and replace the two inline
occurrences of 'backupReminderActive' with the new constant; update any code in
BackupReminderProvider (or functions/methods that currently reference the inline
string) to use _kBackupReminderActive to ensure consistency and avoid typos.

In `@lib/features/account/screens/account_screen.dart`:
- Around line 289-317: The TextEditingController created in _showImportDialog is
not disposed; create the controller as you already do but ensure you dispose it
after the dialog closes by attaching a completion handler to showDialog (e.g.,
showDialog(...).then((_) => controller.dispose())), or alternatively instantiate
the controller inside the dialog builder and dispose it when the dialog is
popped; reference the TextEditingController, _showImportDialog and showDialog to
locate where to add the disposal so the controller is properly disposed on
dialog dismissal.

In `@lib/features/notifications/screens/notifications_screen.dart`:
- Around line 285-291: The _relativeTime(DateTime dt) helper doesn't handle
future timestamps (negative Duration) which can produce incorrect output; update
_relativeTime to detect if DateTime.now().difference(dt) is negative
(diff.isNegative) and return a sensible value such as 'Just now' (or clamp to 0m
ago) before the existing minute/hour/day checks so future timestamps are treated
as immediate rather than producing "0d ago".

In `@lib/features/walkthrough/providers/first_run_provider.dart`:
- Around line 29-32: The catch block in the first run provider currently sets
state = AsyncValue.error(e, st) which contradicts the fail-safe comment and
relies on incidental routing behavior; update the handler in the provider (the
catch in the function that sets state) to set state = AsyncValue.data(true) so
the app treats the walkthrough as completed (matching the comment and the logic
used by firstRunAsync.when() in app_routes.dart), and adjust the comment to
reflect this explicit intentional fail-safe behavior.

In `@lib/shared/widgets/notification_bell.dart`:
- Line 93: The instantiation of the stateless widget _RedDot should be made
const for a small performance benefit; locate the place where _RedDot() is
returned (inside the notification bell widget branch) and change the constructor
call to const _RedDot() so the widget can be canonicalized and avoid unnecessary
rebuild work.
- Around line 120-124: The _CountBadge widget declares an unused AppColors?
colors parameter and field; remove the unused parameter and field from the
_CountBadge constructor and class (references: class _CountBadge, const
_CountBadge({required this.count, required this.colors}); final AppColors?
colors;) and update all call sites to stop passing a colors argument (or
alternatively, if theming is desired, replace the hardcoded Color(0xFFB8860B)
inside _CountBadge with the appropriate property from the provided AppColors and
keep the parameter). Ensure constructors and callers are consistent after the
change.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 180a3024-3f8b-4d52-94a7-0cf143e7526c

📥 Commits

Reviewing files that changed from the base of the PR and between ee6ff80 and f7d31bd.

📒 Files selected for processing (9)
  • lib/core/app_routes.dart
  • lib/features/account/providers/backup_reminder_provider.dart
  • lib/features/account/screens/account_screen.dart
  • lib/features/notifications/models/notification_model.dart
  • lib/features/notifications/providers/notifications_provider.dart
  • lib/features/notifications/screens/notifications_screen.dart
  • lib/features/walkthrough/providers/first_run_provider.dart
  • lib/shared/widgets/notification_bell.dart
  • specs/004-mostro-p2p-client/tasks.md

Comment thread lib/features/account/providers/backup_reminder_provider.dart
backup_reminder_provider.dart:
- add _loaded flag to load(); return early on repeat calls so
  showBackupReminder() awaiting load() cannot cause a double-write race
  where load() overwrites state=true with state=false
- extract _kBackupReminderActive constant; replace both inline
  'backupReminderActive' string literals

account_screen.dart:
- dispose TextEditingController after showDialog closes via .then()
  to prevent a resource leak on dialog dismissal

notifications_screen.dart:
- add explicit diff.isNegative guard in _relativeTime() before the
  < 1 minute check to make future-timestamp handling intent clear

first_run_provider.dart:
- catch block: set AsyncValue.data(true) instead of AsyncValue.error
  to match the fail-safe comment; removes incidental dependency on
  the router's error handler returning null

notification_bell.dart:
- add const _RedDot() constructor so the call site is canonicalized
- remove unused colors parameter and field from _CountBadge; remove
  the unused AppColors import

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (2)
lib/shared/widgets/notification_bell.dart (1)

73-97: Expose bell state to assistive technologies.

Consider adding a dynamic Semantics label (e.g., backup reminder active / N unread notifications) so screen-reader users get the same status conveyed visually by the badge.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/shared/widgets/notification_bell.dart` around lines 73 - 97, Wrap the
icon widget (the child of AnimatedBuilder / Stack) in a Semantics widget that
exposes a dynamic label using the existing state variables: when isActive is
false provide "Notifications, no unread notifications", when isActive is true
and backupActive is true provide "Notifications, backup reminder active",
otherwise provide "Notifications, N unread notifications" where N is
unreadCount; ensure the Semantics includes button:true and onTap semantics
matching the IconButton action (or merges semantics) so screen readers announce
the label when focusing the notification bell (refer to IconButton, isActive,
backupActive, _RedDot, _CountBadge, and unreadCount to build the label).
lib/features/account/screens/account_screen.dart (1)

393-396: Use semantic/focusable controls instead of raw GestureDetector for actions.

The info action and privacy option taps are not ideal for keyboard/focus accessibility. Prefer IconButton/InkWell/RadioListTile (or explicit Semantics) for proper discoverability and interaction.

Also applies to: 419-421

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/features/account/screens/account_screen.dart` around lines 393 - 396,
Replace the raw GestureDetector wrappers used for the info action (the
GestureDetector with onTap: onInfo wrapping Icon(Icons.info_outline)) and the
similar privacy-option GestureDetector (the one around the privacy tap at lines
noted) with focusable, semantic controls such as IconButton or InkWell so they
gain keyboard/focus/semantics support; specifically swap the GestureDetector
around the info icon to an IconButton (forwarding onInfo to onPressed) or wrap
the child with an InkWell and provide a proper Semantics label and focusNode,
and do the same for the privacy tap control (ensure any onTap handlers are moved
to onPressed/onTap of the new widget and add a semantic label/tooltip so screen
readers and keyboard users can discover and activate the actions).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@lib/features/account/screens/account_screen.dart`:
- Around line 36-48: In _loadAndRevealWords(), wrap the async work in a
try/finally so _loadingWords is always reset, and guard any setState calls with
mounted checks to avoid calling setState after disposal; specifically, start by
returning early if _loadingWords is true, then setState to mark loading before
awaiting, perform the await in try, and in the try before calling setState to
reveal words verify if (mounted) to update _wordsVisible/_words, and in finally
do a mounted-guarded setState to set _loadingWords = false so loading never gets
stuck (refer to _loadAndRevealWords, _loadingWords, _wordsVisible, and _words).

---

Nitpick comments:
In `@lib/features/account/screens/account_screen.dart`:
- Around line 393-396: Replace the raw GestureDetector wrappers used for the
info action (the GestureDetector with onTap: onInfo wrapping
Icon(Icons.info_outline)) and the similar privacy-option GestureDetector (the
one around the privacy tap at lines noted) with focusable, semantic controls
such as IconButton or InkWell so they gain keyboard/focus/semantics support;
specifically swap the GestureDetector around the info icon to an IconButton
(forwarding onInfo to onPressed) or wrap the child with an InkWell and provide a
proper Semantics label and focusNode, and do the same for the privacy tap
control (ensure any onTap handlers are moved to onPressed/onTap of the new
widget and add a semantic label/tooltip so screen readers and keyboard users can
discover and activate the actions).

In `@lib/shared/widgets/notification_bell.dart`:
- Around line 73-97: Wrap the icon widget (the child of AnimatedBuilder / Stack)
in a Semantics widget that exposes a dynamic label using the existing state
variables: when isActive is false provide "Notifications, no unread
notifications", when isActive is true and backupActive is true provide
"Notifications, backup reminder active", otherwise provide "Notifications, N
unread notifications" where N is unreadCount; ensure the Semantics includes
button:true and onTap semantics matching the IconButton action (or merges
semantics) so screen readers announce the label when focusing the notification
bell (refer to IconButton, isActive, backupActive, _RedDot, _CountBadge, and
unreadCount to build the label).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8e9a411f-da2e-490e-b924-b305a02c4209

📥 Commits

Reviewing files that changed from the base of the PR and between f7d31bd and 8799e1f.

📒 Files selected for processing (5)
  • lib/features/account/providers/backup_reminder_provider.dart
  • lib/features/account/screens/account_screen.dart
  • lib/features/notifications/screens/notifications_screen.dart
  • lib/features/walkthrough/providers/first_run_provider.dart
  • lib/shared/widgets/notification_bell.dart
🚧 Files skipped from review as they are similar to previous changes (3)
  • lib/features/walkthrough/providers/first_run_provider.dart
  • lib/features/account/providers/backup_reminder_provider.dart
  • lib/features/notifications/screens/notifications_screen.dart

Comment thread lib/features/account/screens/account_screen.dart Outdated
account_screen.dart:
- _loadAndRevealWords: wrap async body in try/finally so _loadingWords
  is always reset even on exception; guard all setState and ref.read
  calls after awaits with mounted checks to prevent use-after-disposal
- _CardHeader info action: replace GestureDetector with IconButton to
  gain keyboard focus, tap target sizing, and accessibility semantics
- _PrivacyOption: replace GestureDetector with InkWell + Semantics
  (label: title, button: true) for ripple feedback and screen reader
  discoverability

notification_bell.dart:
- wrap IconButton in Semantics with dynamic label: "no unread
  notifications" / "backup reminder active" / "N unread"; update
  tooltip to match so hover and screen reader both report current state

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (2)
lib/shared/widgets/notification_bell.dart (2)

69-71: Minor: Remove unnecessary parentheses.

The parentheses around next are redundant.

🧹 Suggested cleanup
     ref.listen<int>(unreadNotificationCountProvider, (prev, next) {
-      if ((next) > (prev ?? 0)) _triggerShake();
+      if (next > (prev ?? 0)) _triggerShake();
     });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/shared/widgets/notification_bell.dart` around lines 69 - 71, The
condition in the ref.listen<int> callback uses unnecessary parentheses around
the variable next; update the listener
(ref.listen<int>(unreadNotificationCountProvider, (prev, next) { ... })) to
remove the redundant parentheses and use if (next > (prev ?? 0))
_triggerShake(); leaving the null-coalescing on prev as-is and keeping
references to unreadNotificationCountProvider, prev/next, and _triggerShake
unchanged.

111-125: Consider extracting hardcoded colors to theme or constants.

Both _RedDot (line 120: 0xFFD84D4D) and _CountBadge (line 139: 0xFFB8860B) use hardcoded color values. If your app has a centralized color palette or theme extension, referencing those would improve maintainability and support theming (e.g., dark mode adjustments).

This is a minor suggestion—the current implementation is functional and the colors are clearly intentional.

Also applies to: 127-153

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/shared/widgets/notification_bell.dart` around lines 111 - 125, Extract
the hardcoded color literals used in _RedDot (0xFFD84D4D) and _CountBadge
(0xFFB8860B) into centralized definitions and reference them instead of inline
values: either add named constants (e.g., AppColors.notificationRed and
AppColors.countBadgeAmber) or provide them via your Theme/ThemeExtension and
read them with Theme.of(context).<CountBadge/_RedDot> Update the BoxDecoration
color usages in both _RedDot and _CountBadge to use the new named color values
so theming (including dark mode) and maintainability are supported.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@lib/features/account/screens/account_screen.dart`:
- Around line 24-25: The _privacyMode boolean in AccountScreen is local and only
updated via setState (e.g., in the onChanged handlers around the Toggle/Switch),
so the setting never persists or affects app-wide behavior; either wire it to
Riverpod (preferred) or disable/label the control as "coming soon." To wire:
create a Riverpod StateProvider<bool> (e.g., privacyModeProvider), replace the
local _privacyMode usage with ref.watch(privacyModeProvider), and change the
onChanged handlers to update via ref.read(privacyModeProvider.notifier).state =
newValue (remove setState). To disable/label: keep the UI control
disabled/readonly and show a "Coming soon (Phase 6)" hint so users can’t toggle
a non-persistent setting (remove or stop using _privacyMode for global
behavior).
- Around line 302-307: The TextField that collects the mnemonic in
account_screen.dart should disable IME features: in the TextField (the widget
using controller in the AccountScreen/build method) set autocorrect: false,
enableSuggestions: false, and enableIMEPersonalizedLearning: false so the
OS/keyboard does not store or suggest the recovery phrase; keep the existing
controller and maxLines settings unchanged.
- Around line 429-433: The Semantics wrapper around InkWell currently only
labels the control as a generic button; update it to expose radio semantics by
adding a boolean selection state (e.g., isSelected) and set Semantics(selected:
isSelected, inMutuallyExclusiveGroup: true) and include onTapHint (e.g.,
onTapHint: isSelected ? 'Selected' : 'Activate') while removing or keeping
button: true as appropriate so screen readers know this is a mutually exclusive
(radio-like) option; update the widget that builds this control (the Semantics +
InkWell block in account_screen.dart that uses title and onTap) to accept/derive
isSelected and pass it into the Semantics node.

---

Nitpick comments:
In `@lib/shared/widgets/notification_bell.dart`:
- Around line 69-71: The condition in the ref.listen<int> callback uses
unnecessary parentheses around the variable next; update the listener
(ref.listen<int>(unreadNotificationCountProvider, (prev, next) { ... })) to
remove the redundant parentheses and use if (next > (prev ?? 0))
_triggerShake(); leaving the null-coalescing on prev as-is and keeping
references to unreadNotificationCountProvider, prev/next, and _triggerShake
unchanged.
- Around line 111-125: Extract the hardcoded color literals used in _RedDot
(0xFFD84D4D) and _CountBadge (0xFFB8860B) into centralized definitions and
reference them instead of inline values: either add named constants (e.g.,
AppColors.notificationRed and AppColors.countBadgeAmber) or provide them via
your Theme/ThemeExtension and read them with
Theme.of(context).<CountBadge/_RedDot> Update the BoxDecoration color usages in
both _RedDot and _CountBadge to use the new named color values so theming
(including dark mode) and maintainability are supported.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2e297676-de65-44b4-a2e5-dac7676ce52b

📥 Commits

Reviewing files that changed from the base of the PR and between 8799e1f and 7da91d8.

📒 Files selected for processing (2)
  • lib/features/account/screens/account_screen.dart
  • lib/shared/widgets/notification_bell.dart

Comment thread lib/features/account/screens/account_screen.dart
Comment thread lib/features/account/screens/account_screen.dart
Comment thread lib/features/account/screens/account_screen.dart
… IME flags

- AppColors: add badgeGold token to lerp, _dark, and _light instances
- notification_bell: use theme destructiveRed/badgeGold instead of hardcoded hex;
  import app_theme.dart; remove redundant parens in unread count listener
- account_screen: disable privacy mode controls with Opacity + "Coming soon" label
  (will be wired in Phase 6); make _PrivacyOption.onTap nullable; update Semantics
  to carry selected/inMutuallyExclusiveGroup/onTapHint instead of button: true;
  add autocorrect/enableSuggestions/enableIMEPersonalizedLearning: false to
  mnemonic import TextField
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant