-
Notifications
You must be signed in to change notification settings - Fork 14
fix: show loading on switch instead of no accounts #676
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.
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
Conversation
WalkthroughThis 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
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
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
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45–75 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests
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. Comment |
b4fe26c to
f13dfd7
Compare
There was a problem hiding this 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
⛔ Files ignored due to path filters (1)
ios/Podfile.lockis 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.dartlib/ui/settings/general_settings_screen.dart
**/*.md
📄 CodeRabbit inference engine (.cursor/rules/whitenoise.mdc)
**/*.md: NIPs (Nostr Implementation Possibilities) are numbered likeNIP-XXwhereXXare two capitalized hexadecimal digits, e.g.,NIP-01andNIP-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.,07for NIP-07, orC7for 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.mdfor 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 passingprofilesremain.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.
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
Checklist
just precommitto ensure that formatting and linting are correctjust check-flutter-coverageto ensure that flutter coverage rules are passingCHANGELOG.mdfile with your changes (if they affect the user experience)Summary by CodeRabbit
Bug Fixes
Refactor
Documentation