Skip to content

Conversation

@josefinalliende
Copy link
Contributor

@josefinalliende josefinalliende commented Sep 22, 2025

Description

Fixes #638. No accounts in switcher was back. This is what happens to me when I try to switch accounts ant they are not loaded yet (first attempt fails, second attempt works)

no-accounts-issue.MP4

This PR moves logic of loading accounts with its metadata to the switcher, where is needed. When loading shows a spinner. This way when accounts aren't loaded yet, the user knows that have to wait a bit instead of having to close the sheet and reopen it

In general settings screen only is needed now to load accounts for the logout logic, but just accounts length is enough, there is no need to load the metadata of the accounts on logout.

This is how it works now:

no-accounts-fixes.MP4

Type of Change

  • ✨ New feature (non-breaking change which adds functionality)
  • 🛠️ Bug fix (non-breaking change which fixes an issue)
  • ❌ Breaking change (fix or feature that would cause existing functionality to change)
  • 🧹 Code refactor
  • ✅ Build configuration change
  • 📝 Documentation
  • 🗑️ Chore
  • 🧪 Tests

Checklist

  • Run just precommit to ensure that formatting and linting are correct
  • Run just check-flutter-coverage to ensure that flutter coverage rules are passing
  • Updated the CHANGELOG.md file with your changes (if they affect the user experience)

Summary by CodeRabbit

  • Bug Fixes

    • Fixed account switcher error.
    • Improved sign-out and switch flow with clearer success and error toasts.
  • Refactor

    • Account switcher now loads accounts dynamically instead of using cached data.
    • Shows a loading indicator while fetching accounts and highlights the active account.
    • Enhanced error handling during account retrieval.
  • Documentation

    • Updated changelog to reflect the account switcher fix.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 22, 2025

Walkthrough

This change removes cached account/profile data usage, shifts account retrieval to runtime for the settings screen and the switcher sheet, updates logout handling to query accounts after sign-out, modifies the switcher API (no external profiles parameter), and adds loading/error handling within the bottom sheet. CHANGES.md gains a fixed entry.

Changes

Cohort / File(s) Summary of Changes
Changelog update
CHANGELOG.md
Added “Fixed account switcher error” under Unreleased → Fixed.
General settings: runtime accounts + logout flow
lib/ui/settings/general_settings_screen.dart
Removed local account/profile caching, loading timers, and subscriptions; simplified account switcher invocation (no profiles arg); after logout, fetch accounts via getAccounts to decide showing switcher vs. toast; added try/catch and error toast; kept package info loading; updated callback types.
Switcher bottom sheet: internal loading + API change
lib/ui/settings/profile/switch_profile_bottom_sheet.dart
Removed profiles from constructor and show; loads accounts at runtime via getAccounts; added _isLoadingAccounts, _accountsProfileData, sorting with active-first, and error handling; UI shows loading state then renders loaded accounts; updated tap/active highlighting; replaces precompute flow to operate on loaded data.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor U as User
  participant GS as GeneralSettingsScreen
  participant Auth as AuthService
  participant Acc as AccountsService
  participant Sheet as SwitchProfileBottomSheet

  rect rgb(245,248,255)
    U->>GS: Tap "Sign out"
    GS->>Auth: logout()
    Auth-->>GS: success/failure
  end

  alt logout success
    GS->>Acc: getAccounts()
    Acc-->>GS: accounts list
    alt multiple accounts
      GS->>Sheet: show()
      activate Sheet
      Sheet->>Acc: getAccounts()
      Acc-->>Sheet: accounts + profiles
      Sheet-->>U: Render list (active first)
      deactivate Sheet
    else single account
      GS-->>U: Toast "Signed out"
      GS->>GS: Switch to remaining account
    end
  else logout error
    GS-->>U: Error toast
  end
Loading
sequenceDiagram
  autonumber
  actor U as User
  participant GS as GeneralSettingsScreen
  participant Sheet as SwitchProfileBottomSheet
  participant Acc as AccountsService

  U->>GS: Tap "Switch accounts"
  GS->>Sheet: show()  // no profiles passed
  activate Sheet
  Note over Sheet: Internal loading state
  Sheet->>Acc: getAccounts()
  Acc-->>Sheet: accounts + profiles
  Sheet-->>U: Render accounts (active highlighted)
  deactivate Sheet
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45–75 minutes

Possibly related PRs

Suggested reviewers

  • erskingardner
  • untreu2

Poem

I hop between accounts with glee,
No empty lists shall trouble me.
The cache is gone—now fetch on cue,
A loading sheet, then fresher view.
With one quick thump of fluffy feet,
I switch, I toast—bugs face defeat. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "fix: show loading on switch instead of no accounts" is concise, directly describes the main behavioral change (showing a loading state in the account switcher rather than an empty view) and aligns with the PR objective to fix the no-accounts-in-switcher bug.
Linked Issues Check ✅ Passed The changes implement the approach requested in issue [#638] by moving account-loading into the SwitchProfileBottomSheet, showing a spinner while accounts load, updating logout handling in general_settings_screen to rely only on accounts.length, and adding error handling and active-account sorting; these code-level changes satisfy the linked issue's objective to avoid showing a blank switcher.
Out of Scope Changes Check ✅ Passed All modified files (the switcher component, general settings screen, and CHANGELOG) are directly related to the stated objective of loading accounts inside the switcher and fixing the empty-state bug; the removal of the profiles parameter from the switcher API appears to be a necessary refactor tied to that design change rather than an unrelated modification.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/switch-accounts-empty

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.

@josefinalliende josefinalliende linked an issue Sep 22, 2025 that may be closed by this pull request
@josefinalliende josefinalliende force-pushed the fix/switch-accounts-empty branch from b4fe26c to f13dfd7 Compare September 22, 2025 19:26
@josefinalliende josefinalliende marked this pull request as ready for review September 22, 2025 19:30
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
lib/ui/settings/profile/switch_profile_bottom_sheet.dart (2)

136-143: Normalize the active pubkey to hex (current logic may fail to detect/sort active account).

_activeAccountHex is set from the provider without normalization. Sorting and highlighting can fail when formats differ (npub vs hex).

Apply this diff:

   Future<void> _getActivePubkeyHex() async {
-    final activeAccountPubkey = ref.read(activePubkeyProvider) ?? '';
-    if (activeAccountPubkey.isNotEmpty) {
-      setState(() {
-        _activeAccountHex = activeAccountPubkey;
-      });
-    }
+    final String activeAccountPubkey = ref.read(activePubkeyProvider) ?? '';
+    if (activeAccountPubkey.isEmpty) return;
+    final String? hex = PubkeyFormatter(pubkey: activeAccountPubkey).toHex();
+    setState(() {
+      _activeAccountHex = hex ?? activeAccountPubkey;
+    });
   }

160-175: Refresh accounts after connecting a new profile.

The list stays stale after closing ConnectProfileBottomSheet.

Apply this diff:

   Future<void> _handleConnectAnotherProfile() async {
     setState(() {
       _isConnectProfileSheetOpen = true;
     });
 
     try {
       await ConnectProfileBottomSheet.show(context: context);
     } finally {
-      if (mounted) {
-        setState(() {
-          _isConnectProfileSheetOpen = false;
-        });
-      }
+      if (!mounted) return;
+      setState(() {
+        _isConnectProfileSheetOpen = false;
+      });
+      await _loadAccountsProfileData();
     }
   }
🧹 Nitpick comments (9)
CHANGELOG.md (1)

39-39: Clarify the fix and reference the issue.

Be explicit about the UX change and link the issue for traceability.

Apply this diff:

-- Fixed account switcher error
+- Fix: account switcher showed empty state before accounts loaded; switcher now loads accounts internally and shows a loading spinner (#638)
lib/ui/settings/profile/switch_profile_bottom_sheet.dart (7)

74-88: Sorting depends on normalized keys; ensure consistency.

Since _activeAccountHex is hex-normalized (per above), compare against hex for both sides to guarantee correct ordering.

Apply this diff to rely solely on normalized values:

   void _sortAccountsProfileData() {
     _precomputeProfileHexes();
     if (_activeAccountHex != null) {
       _accountsProfileData.sort((a, b) {
-        final aHex = _pubkeyToHex[a.publicKey] ?? a.publicKey;
-        final bHex = _pubkeyToHex[b.publicKey] ?? b.publicKey;
+        final aHex = _pubkeyToHex[a.publicKey] ?? a.publicKey;
+        final bHex = _pubkeyToHex[b.publicKey] ?? b.publicKey;
         final aIsActive = aHex == _activeAccountHex;
         final bIsActive = bHex == _activeAccountHex;
         if (aIsActive && !bIsActive) return -1;
         if (!aIsActive && bIsActive) return 1;
         return 0;
       });
     }
   }

120-126: Do the sort within the same setState for atomic UI update.

Avoid a rebuild between assignment and sort.

Apply this diff:

-      setState(() {
-        _accountsProfileData = accountsProfileData;
-        _isLoadingAccounts = false;
-      });
-      _sortAccountsProfileData();
+      setState(() {
+        _accountsProfileData = accountsProfileData;
+        _isLoadingAccounts = false;
+        _sortAccountsProfileData();
+      });

145-155: Make active-account check synchronous; no need for Future.

This avoids a FutureBuilder per row.

Apply this diff:

-  Future<bool> _isActiveAccount(ContactModel profile) async {
-    if (_activeAccountHex == null) return false;
-
-    try {
-      final String profileHex = PubkeyFormatter(pubkey: profile.publicKey).toHex() ?? '';
-      return profileHex == _activeAccountHex;
-    } catch (e) {
-      // If conversion fails, try direct comparison as fallback
-      return profile.publicKey == _activeAccountHex;
-    }
-  }
+  bool _isActiveAccountSync(ContactModel profile) {
+    if (_activeAccountHex == null) return false;
+    final String normalized =
+        _pubkeyToHex[profile.publicKey] ??
+        (PubkeyFormatter(pubkey: profile.publicKey).toHex() ?? profile.publicKey);
+    return normalized == _activeAccountHex;
+  }

196-205: Avoid Flexible inside a Column with mainAxisSize.min (risk: unbounded height error).

Constrain height explicitly for the list.

Apply this diff:

-              Flexible(
-                child: ListView.builder(
+              ConstrainedBox(
+                constraints: BoxConstraints(
+                  maxHeight: MediaQuery.of(context).size.height * 0.6,
+                ),
+                child: ListView.builder(

209-236: Remove per-row FutureBuilder; compute isActive synchronously.

This reduces rebuild overhead and jank.

Apply this diff:

-                      child: FutureBuilder<bool>(
-                        future: _isActiveAccount(profile),
-                        builder: (context, snapshot) {
-                          final isActiveAccount = snapshot.data ?? false;
-
-                          return Container(
-                            decoration:
-                                isActiveAccount
-                                    ? BoxDecoration(
-                                      color: context.colors.primary.withValues(alpha: 0.1),
-                                    )
-                                    : null,
-                            padding: EdgeInsets.symmetric(horizontal: 8.w, vertical: 4.h),
-                            child: ContactListTile(
-                              contact: profile,
-                              onTap: () {
-                                if (isActiveAccount && !widget.showSuccessToast) {
-                                  // Just close the sheet if selecting the currently active profile
-                                  Navigator.pop(context);
-                                } else {
-                                  widget.onProfileSelected(profile);
-                                  Navigator.pop(context);
-                                }
-                              },
-                            ),
-                          );
-                        },
-                      ),
+                      child: Builder(builder: (context) {
+                        final bool isActiveAccount = _isActiveAccountSync(profile);
+                        return Container(
+                          decoration: isActiveAccount
+                              ? BoxDecoration(
+                                  color: context.colors.primary.withValues(alpha: 0.1),
+                                )
+                              : null,
+                          padding: EdgeInsets.symmetric(horizontal: 8.w, vertical: 4.h),
+                          child: ContactListTile(
+                            contact: profile,
+                            onTap: () {
+                              if (isActiveAccount && !widget.showSuccessToast) {
+                                Navigator.pop(context);
+                              } else {
+                                widget.onProfileSelected(profile);
+                                Navigator.pop(context);
+                              }
+                            },
+                          ),
+                        );
+                      }),

243-245: Localize user-facing text.

Use AppLocalizations instead of hardcoded strings.

Example:

label: AppLocalizations.of(context)!.connectAnotherProfile,

102-134: Minor style nit: remove blank lines inside functions.

Guidelines say no blank lines within functions. Applies here and in other methods in this file.

lib/ui/settings/general_settings_screen.dart (1)

162-163: Localize toast text.

Prefer AppLocalizations for these strings.

ref.showSuccessToast(AppLocalizations.of(context)!.signedOutSwitchedToOther);
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 34cdf96 and f13dfd7.

⛔ Files ignored due to path filters (1)
  • ios/Podfile.lock is excluded by !**/*.lock
📒 Files selected for processing (3)
  • CHANGELOG.md (1 hunks)
  • lib/ui/settings/general_settings_screen.dart (2 hunks)
  • lib/ui/settings/profile/switch_profile_bottom_sheet.dart (3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.dart

📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)

**/*.dart: Always declare the type of each variable and function (parameters and return value). Avoid using 'any'. Create necessary types.
Don't leave blank lines within a function.
One export per file.
Use PascalCase for classes.
Use camelCase for variables, functions, and methods.
Use underscores_case for file and directory names.
Use UPPERCASE for environment variables. Avoid magic numbers and define constants.
Start each function with a verb.
Use verbs for boolean variables. Example: isLoading, hasError, canDelete, etc.
Use complete words instead of abbreviations and correct spelling, except for standard and well-known abbreviations (API, URL, i, j, err, ctx, req, res, next).
Write short functions with a single purpose. Less than 20 instructions.
Name functions with a verb and something else. If it returns a boolean, use isX or hasX, canX, etc. If it doesn't return anything, use executeX or saveX, etc.
Avoid nesting blocks by early checks and returns, or extraction to utility functions.
Use higher-order functions (map, filter, reduce, etc.) to avoid function nesting. Use arrow functions for simple functions (less than 3 instructions). Use named functions for non-simple functions.
Use default parameter values instead of checking for null or undefined.
Reduce function parameters using RO-RO: use an object to pass multiple parameters and to return results. Declare necessary types for input arguments and output.
Use a single level of abstraction in functions.
Don't abuse primitive types and encapsulate data in composite types.
Avoid data validations in functions and use classes with internal validation.
Prefer immutability for data. Use readonly for data that doesn't change. Use 'as const' for literals that don't change.
Declare interfaces to define contracts.
Write small classes with a single purpose. Less than 200 instructions, less than 10 public methods, less than 10 properties.
Use exceptions to handle errors you don't expect. If you catch an exception, it sh...

Files:

  • lib/ui/settings/profile/switch_profile_bottom_sheet.dart
  • lib/ui/settings/general_settings_screen.dart
**/*.md

📄 CodeRabbit inference engine (.cursor/rules/whitenoise.mdc)

**/*.md: NIPs (Nostr Implementation Possibilities) are numbered like NIP-XX where XX are two capitalized hexadecimal digits, e.g., NIP-01 and NIP-C7.
To read a specific NIP, construct the NIP URL following this template: https://raw.githubusercontent.com/nostr-protocol/nips/refs/heads/master/{nip}.md (replace {nip} in the URL template with the relevant NIP name, e.g., 07 for NIP-07, or C7 for NIP-C7).
To read the definition of a specific kind, construct a URL following this template: https://nostrbook.dev/kinds/{kind}.md (replace {kind} in the template with the kind number, e.g., https://nostrbook.dev/kinds/0.md for kind 0).

Files:

  • CHANGELOG.md
🧠 Learnings (5)
📚 Learning: 2025-09-07T13:10:16.542Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#597
File: lib/config/providers/group_provider.dart:311-314
Timestamp: 2025-09-07T13:10:16.542Z
Learning: In the whitenoise_flutter codebase, the User class used in group_provider.dart (and similar contexts) is presentational only, not the actual user class from the Rust API. There are plans to remove this User class and replace it with UserProfileData, similar to the planned consolidation with ContactModel.

Applied to files:

  • lib/ui/settings/profile/switch_profile_bottom_sheet.dart
📚 Learning: 2025-09-03T20:57:53.202Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#594
File: lib/config/providers/user_profile_data_provider.dart:0-0
Timestamp: 2025-09-03T20:57:53.202Z
Learning: In the whitenoise_flutter codebase, pubkey normalization (npub/hex format handling) is implemented in a pubkey validations extension rather than being imported directly into individual providers like user_profile_data_provider.dart.

Applied to files:

  • lib/ui/settings/profile/switch_profile_bottom_sheet.dart
📚 Learning: 2025-09-07T02:15:31.931Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#594
File: lib/src/rust/api/utils.dart:12-16
Timestamp: 2025-09-07T02:15:31.931Z
Learning: The lingering PublicKey references in lib/config/providers/auth_provider.dart, lib/config/providers/group_provider.dart, and rust_builder/cargokit/build_tool/lib/src/options.dart identified during the utils.dart API cleanup were fixed in PR #597 in the whitenoise_flutter codebase.

Applied to files:

  • lib/ui/settings/profile/switch_profile_bottom_sheet.dart
📚 Learning: 2025-08-12T11:21:53.640Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#455
File: lib/ui/settings/profile/switch_profile_bottom_sheet.dart:0-0
Timestamp: 2025-08-12T11:21:53.640Z
Learning: In the whitenoise_flutter codebase, ContactModel.publicKey can be stored in either npub format (from metadata cache) or hex format (from account storage). The activeAccountProvider may return either format depending on how the account was originally stored, so normalization to hex format is required when comparing with other hex-normalized keys in sorting logic.

Applied to files:

  • lib/ui/settings/profile/switch_profile_bottom_sheet.dart
📚 Learning: 2025-08-08T13:39:00.500Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.500Z
Learning: Applies to **/*.dart : Use AutoRoute to manage routes. Use extras to pass data between pages.

Applied to files:

  • lib/ui/settings/general_settings_screen.dart
🔇 Additional comments (2)
lib/ui/settings/general_settings_screen.dart (2)

86-86: Explicitly typed callback param — good.

Keeps API clear and avoids dynamic types.


154-166: Post-logout account switcher: verified — no callers passing profiles remain.

Repo search shows SwitchProfileBottomSheet.show is implemented at lib/ui/settings/profile/switch_profile_bottom_sheet.dart and the only in-repo call is lib/ui/settings/general_settings_screen.dart; it uses onProfileSelected/isDismissible/showSuccessToast and no profiles: argument was found.

@josefinalliende josefinalliende merged commit c16ac6d into master Sep 22, 2025
2 checks passed
@josefinalliende josefinalliende deleted the fix/switch-accounts-empty branch September 22, 2025 19:57
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.

No accounts in switcher bug 🐞 is back

3 participants