Fix scrolling/loading performance of start new chat sheet #656
Conversation
WalkthroughAdds lifetime config to flutter_rust_bridge, adjusts build/release flows (removes precommit steps, adds Dart formatting), updates a Rust dependency revision, introduces cached/synchronous public key formatting in models and tiles, implements debounced search and refactored contact list rendering, and rewrites user filtering to an iterative approach. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant N as NewChatBottomSheet
participant D as Debounce Timer (300ms)
participant P as Profile Fetch
participant S as State/Store
U->>N: Type in search field
N->>D: Reset/start timer
D-->>N: On timeout
alt Input is valid pubkey
N->>P: _getUserProfileDataForPublicKey
P-->>N: Metadata result
end
N->>S: setState(_searchQuery, _tempContact, loading flags)
Note over N,S: List rebuilt via _buildContactsList\n(shows loading/empty/sections)
sequenceDiagram
participant M as ContactModel
participant T as ContactListTile
participant F as PubkeyFormatter
participant X as String Extensions
Note over M: _formattedPublicKey cached\n(optional ctor param)
T->>T: _getFormattedPublicKey()
alt preformatted provided
T-->>T: Use preformatted value
else
T->>F: toNpub(publicKey)
F-->>T: npub string
T->>X: formatPublicKey(npub)
X-->>T: formatted string
opt On error
T-->>T: Fallback publicKey.formatPublicKey()
end
end
T-->>T: Render Text(formattedKey)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/domain/models/contact_model.dart (1)
54-55: Do not store empty publicKey when npub conversion fails
toNpub() ?? ''risks an emptypublicKey(regression noted previously). Fall back to the originalpubkeyto keep IDs non-empty; use the same value to precompute the formatted key.Apply this diff:
- final npub = PubkeyFormatter(pubkey: pubkey).toNpub() ?? ''; + final String? npub = PubkeyFormatter(pubkey: pubkey).toNpub(); @@ - // Pre-compute formatted public key - String formattedKey; + // Decide stored key and pre-compute formatted public key + final String storedKey = (npub != null && npub.isNotEmpty) ? npub : pubkey; + String formattedKey; try { - formattedKey = npub.formatPublicKey(); + formattedKey = storedKey.formatPublicKey(); } catch (e) { formattedKey = pubkey.formatPublicKey(); } @@ return ContactModel( displayName: finalDisplayName, - publicKey: npub, + publicKey: storedKey, imagePath: picture, about: about, website: website, nip05: nip05, lud16: lud16, formattedPublicKey: formattedKey, );Also applies to: 64-71, 72-81
🧹 Nitpick comments (3)
scripts/build.sh (1)
332-335: Guard dart formatting against missing integration_test directory
set -ewill fail ifintegration_test/doesn't exist. Add a directory check to keep builds green across environments.Apply this diff:
-# Format dart files so generated output is formatted -print_step "🔍 Formatting dart files" -dart format lib/ integration_test/ +# Format dart files so generated output is formatted +print_step "🔍 Formatting dart files" +if [[ -d integration_test ]]; then + dart format lib/ integration_test/ +else + dart format lib/ +filib/utils/user_utils.dart (1)
26-52: Typed locals + no intra-function blank lines (repo guideline)Follow the repo rule to declare explicit types and avoid blank lines within functions. Keeps style consistent and reduces analyzer noise.
Apply this diff:
- final query = searchQuery.toLowerCase(); - final results = <User>[]; - - // Use for loop instead of where().toList() for better performance + final String query = searchQuery.toLowerCase(); + final List<User> results = <User>[]; + // Use for loop instead of where().toList() for better performance for (final user in users) { - final displayName = _getDisplayName(user).toLowerCase(); - - // Check display name first (most common match) + final String displayName = _getDisplayName(user).toLowerCase(); + // Check display name first (most common match) if (displayName.contains(query)) { results.add(user); continue; } - - // Only check nip05 if display name doesn't match - final nip05 = user.metadata.nip05?.toLowerCase(); + // Only check nip05 if display name doesn't match + final String? nip05 = user.metadata.nip05?.toLowerCase(); if (nip05 != null && nip05.contains(query)) { results.add(user); continue; } - // Only check pubkey if neither display name nor nip05 match if (user.pubkey.toLowerCase().contains(query)) { results.add(user); } } - return results;lib/ui/contact_list/new_chat_bottom_sheet.dart (1)
365-365: Check for potential null reference.The code checks
_isLoadingUserProfileDatawhenfilteredContacts.isEmpty, but this loading check seems redundant here since it's already handled in theshowTempContactbranch.Consider simplifying this condition:
- _isLoadingUserProfileData - ? const ContactListTileLoading() - : Center( + Center( child: Text(
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
android/app/src/main/jniLibs/arm64-v8a/librust_lib_whitenoise.sois excluded by!**/*.soandroid/app/src/main/jniLibs/armeabi-v7a/librust_lib_whitenoise.sois excluded by!**/*.soandroid/app/src/main/jniLibs/x86_64/librust_lib_whitenoise.sois excluded by!**/*.soios/Podfile.lockis excluded by!**/*.lockpubspec.lockis excluded by!**/*.lockrust/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
flutter_rust_bridge.yaml(1 hunks)justfile(0 hunks)lib/domain/models/contact_model.dart(5 hunks)lib/ui/contact_list/new_chat_bottom_sheet.dart(6 hunks)lib/ui/contact_list/widgets/contact_list_tile.dart(3 hunks)lib/utils/user_utils.dart(1 hunks)rust/Cargo.toml(1 hunks)scripts/build.sh(1 hunks)
💤 Files with no reviewable changes (1)
- justfile
🧰 Additional context used
📓 Path-based instructions (1)
**/*.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/utils/user_utils.dartlib/ui/contact_list/new_chat_bottom_sheet.dartlib/domain/models/contact_model.dartlib/ui/contact_list/widgets/contact_list_tile.dart
🧠 Learnings (11)
📓 Common learnings
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.
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 flutter_rust_bridge to access core functionality of the app.
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 the whitenoise rust crate (via flutter_rust_bridge) as the source of all data and only way to trigger changes in our data model.
Learnt from: Quwaysim
PR: parres-hq/whitenoise_flutter#642
File: lib/ui/contact_list/new_chat_bottom_sheet.dart:324-336
Timestamp: 2025-09-16T07:24:07.489Z
Learning: In the whitenoise_flutter project, the user Quwaysim prefers to handle security improvements incrementally across different PRs rather than bundling them all together. Input trimming for search functionality was handled in PR #613, and private key security measures are planned for PR #505.
📚 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 flutter_rust_bridge to access core functionality of the app.
Applied to files:
flutter_rust_bridge.yaml
📚 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 the whitenoise rust crate (via flutter_rust_bridge) as the source of all data and only way to trigger changes in our data model.
Applied to files:
flutter_rust_bridge.yaml
📚 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 Riverpod to manage state. Use StreamProviders to watch for state changes that come from the rust api. See keepAlive if you need to keep the state alive.
Applied to files:
lib/ui/contact_list/new_chat_bottom_sheet.dart
📚 Learning: 2025-08-23T11:02:28.308Z
Learnt from: Quwaysim
PR: parres-hq/whitenoise_flutter#527
File: lib/ui/core/ui/wn_avatar.dart:1-6
Timestamp: 2025-08-23T11:02:28.308Z
Learning: In the whitenoise_flutter codebase, flutter_screenutil is re-exported through lib/ui/core/themes/src/app_theme.dart, so files that import app_theme.dart do not need to directly import flutter_screenutil to use .w, .h, .sp extensions.
Applied to files:
lib/ui/contact_list/new_chat_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 **/*_test.dart : Use the standard widget testing for Flutter.
Applied to files:
lib/ui/contact_list/new_chat_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/domain/models/contact_model.dartlib/ui/contact_list/widgets/contact_list_tile.dart
📚 Learning: 2025-09-07T02:08:09.199Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#594
File: lib/domain/models/contact_model.dart:36-41
Timestamp: 2025-09-07T02:08:09.199Z
Learning: The issue of ContactModel.publicKey potentially being set to an empty string when pubkey.toNpub() conversion fails (causing collision/lookup issues) is being addressed in PR #597 in the whitenoise_flutter codebase.
Applied to files:
lib/domain/models/contact_model.dartlib/ui/contact_list/widgets/contact_list_tile.dart
📚 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/domain/models/contact_model.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/domain/models/contact_model.dart
📚 Learning: 2025-09-04T17:54:09.162Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#594
File: lib/ui/chat/chat_info/dm_chat_info.dart:0-0
Timestamp: 2025-09-04T17:54:09.162Z
Learning: In the whitenoise_flutter codebase, pubkey conversion from hex to npub format is handled using a .toNpub() extension method on String, which eliminates the need for manual format checking or calling npubFromHexPubkey() function directly.
Applied to files:
lib/ui/contact_list/widgets/contact_list_tile.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Flutter CI
🔇 Additional comments (15)
rust/Cargo.toml (1)
26-26: Rev bump for whitenoise looks goodPinning to a specific commit improves reproducibility for the release build paths in this PR.
lib/utils/user_utils.dart (1)
26-52: Straight-line filtering is a good perf winAvoiding
where().toList()and short-circuiting on first match should cut allocations on large lists.lib/domain/models/contact_model.dart (1)
4-4: Import is appropriateUsing the shared
string_extensions.dartfor key formatting centralizes presentation logic.flutter_rust_bridge.yaml (1)
5-5: Lifetimes enabled — confirm flutter_rust_bridge_codegen = 2.11.1enable_lifetime: true is fine; lifetimes are experimental and the build-time generator must match the crate (=2.11.1). Verification here failed because flutter_rust_bridge_codegen was not found in the environment — run locally to confirm:
#!/bin/bash set -e expected="2.11.1" if ! command -v flutter_rust_bridge_codegen >/dev/null 2>&1; then echo "flutter_rust_bridge_codegen: not installed" exit 2 fi installed="$(flutter_rust_bridge_codegen --version | sed -E 's/[^0-9]*([0-9]+\.[0-9]+\.[0-9]+).*/\1/')" echo "FRB codegen installed: $installed (expected: $expected)" test "$installed" = "$expected" || { echo "Mismatch detected"; exit 1; }lib/ui/contact_list/new_chat_bottom_sheet.dart (7)
1-1: Good use ofdart:asyncfor Timer-based debouncing.The addition of the
dart:asyncimport is appropriate for the debouncing implementation.
55-55: Timer field declaration looks good.The nullable Timer field is properly declared for managing debounce state.
70-70: Proper cleanup of the debounce timer.Correctly cancels the timer before other disposals to prevent memory leaks.
96-111: Well-implemented debouncing with proper safeguards.The debouncing implementation is solid with appropriate delay (300ms) and proper lifecycle checks (
mounted). This will effectively reduce the number of filter operations and API calls during typing.
283-408: Excellent refactoring of the contacts list rendering logic.The extraction to
_buildContactsListimproves code organization and maintainability. The method efficiently handles different states (loading, empty, with results) and properly calculates indices for ListView.builder. The use of preformatted public keys for both temp contacts and regular contacts will improve scrolling performance.
350-350: Good optimization with preformatted public keys.Passing the preformatted public key to ContactListTile eliminates redundant formatting calculations during scrolling, which should improve performance as mentioned in the PR objectives.
Also applies to: 393-393
486-490: Clean integration of the new builder method.The simplified Expanded child properly delegates to the new
_buildContactsListmethod while maintaining the error handling flow.lib/ui/contact_list/widgets/contact_list_tile.dart (4)
22-22: Good addition of optional preformatted public key parameter.The new optional parameter follows Dart naming conventions and integrates cleanly with the existing constructor.
Also applies to: 33-33
37-49: Well-implemented synchronous key formatting with proper fallbacks.The
_getFormattedPublicKeymethod efficiently handles the preformatted key case first, then falls back to formatting logic with proper error handling. This synchronous approach eliminates the FutureBuilder overhead and improves scrolling performance.
54-54: Clean replacement of async FutureBuilder with synchronous rendering.Computing the formatted key once in
buildand using it directly removes the loading state complexity and improves performance. The text styling is preserved correctly.Also applies to: 89-96
43-43: Resolved — no change required: PubkeyFormatter handles null/invalid pubkeys. toNpub() returns null for null or invalid inputs (lib/utils/pubkey_formatter.dart — toNpub()) and unit tests cover these cases (test/utils/pubkey_formatter_test.dart).
| String get formattedPublicKey { | ||
| if (_formattedPublicKey != null && _formattedPublicKey.isNotEmpty) { | ||
| return _formattedPublicKey; | ||
| } | ||
|
|
||
| try { | ||
| final npub = PubkeyFormatter(pubkey: publicKey).toNpub() ?? ''; | ||
| return npub.formatPublicKey(); | ||
| } catch (e) { | ||
| return publicKey.formatPublicKey(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Nullability bug: returning a nullable field where String is required
_formattedPublicKey is String?. Returning it directly will fail analysis; use local promotion or a bang.
Apply this diff:
- String get formattedPublicKey {
- if (_formattedPublicKey != null && _formattedPublicKey.isNotEmpty) {
- return _formattedPublicKey;
- }
+ String get formattedPublicKey {
+ final String? cached = _formattedPublicKey;
+ if (cached != null && cached.isNotEmpty) {
+ return cached;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| String get formattedPublicKey { | |
| if (_formattedPublicKey != null && _formattedPublicKey.isNotEmpty) { | |
| return _formattedPublicKey; | |
| } | |
| try { | |
| final npub = PubkeyFormatter(pubkey: publicKey).toNpub() ?? ''; | |
| return npub.formatPublicKey(); | |
| } catch (e) { | |
| return publicKey.formatPublicKey(); | |
| } | |
| } | |
| String get formattedPublicKey { | |
| final String? cached = _formattedPublicKey; | |
| if (cached != null && cached.isNotEmpty) { | |
| return cached; | |
| } | |
| try { | |
| final npub = PubkeyFormatter(pubkey: publicKey).toNpub() ?? ''; | |
| return npub.formatPublicKey(); | |
| } catch (e) { | |
| return publicKey.formatPublicKey(); | |
| } | |
| } |
🤖 Prompt for AI Agents
In lib/domain/models/contact_model.dart around lines 28 to 39, the getter
returns the nullable field _formattedPublicKey (type String?) directly which
fails null-safety analysis; change the early return to return a non-null String
by promoting the variable or using the null-assertion operator. For example,
capture into a local final var (final formatted = _formattedPublicKey;) and if
(formatted != null && formatted.isNotEmpty) return formatted; or keep the
existing null/empty check but return _formattedPublicKey! instead of
_formattedPublicKey; leave the rest of the try/catch logic unchanged.
Also updates whitenoise to the latest
Summary by CodeRabbit
Refactor
Chores