feat: add no relay indicator and heads-up notification in chat list #471
Conversation
WalkthroughAdds relay-connectivity detection and UI: ChatListScreen disables Add New Chat and shows a sticky heads-up banner when no relay types are connected. Introduces WnStickyHeadsUp component, an allRelayTypesConnectionProvider, a new asset path Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant CLS as ChatListScreen
participant RSP as RelayStatusProvider
participant UI as UI
U->>CLS: Open Chat List
CLS->>RSP: Request allRelayTypesConnectionProvider
RSP-->>CLS: true / false
alt at least one relay type connected each
CLS->>UI: Enable "Add New Chat" (icAddNewChat)
else any relay type missing
CLS->>UI: Disable "Add New Chat" (icOffChat)
CLS->>UI: Show WnStickyHeadsUp("No Relays Connected")
end
sequenceDiagram
participant U as User
participant HeadsUp as WnStickyHeadsUp
participant Nav as Navigator
U->>HeadsUp: Tap "Connect Relays" action
HeadsUp->>Nav: Navigate to Routes.settingsNetwork
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Poem
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
CHANGELOG.md (1)
11-11: Consider using past tense for changelog entries.Changelog entries typically use past tense to describe completed changes. Consider rephrasing to "Added global no relay indicator" for consistency with common changelog conventions.
-- Global No relay indicator +- Added global no relay indicatorlib/ui/core/ui/wn_heads_up.dart (2)
6-19: Add documentation for the public widget API.Consider adding documentation comments to describe the widget's purpose and parameters, especially since this is a reusable UI component that other developers will use.
+/// A sticky heads-up notification banner that displays at the top of the screen. +/// +/// Shows an icon, title, subtitle, and optional action widget. +/// The banner type determines the color scheme and default icon. class WnStickyHeadsUp extends StatelessWidget { const WnStickyHeadsUp({ super.key, required this.title, required this.subtitle, this.type = WnHeadingType.error, this.icon, this.action, }); + + /// The main heading text displayed prominently. final String title; + + /// The descriptive text displayed below the title. final String subtitle; + + /// Optional custom icon. If null, uses the default icon for the type. final IconData? icon; + + /// Optional action widget displayed below the subtitle. final Widget? action; + + /// The type of heads-up, affecting color scheme and default icon. final WnHeadingType type;
76-102: Consider using a more specific enum name.The enum name
WnHeadingTypemight be confusing as it's not about headings but about notification types. Consider renaming toWnHeadsUpTypeorWnNotificationTypefor clarity.-enum WnHeadingType { +enum WnHeadsUpType { error, warning, info; Color color(BuildContext context) { switch (this) { - case WnHeadingType.error: + case WnHeadsUpType.error: return context.colors.destructive; - case WnHeadingType.warning: + case WnHeadsUpType.warning: return context.colors.warning; - case WnHeadingType.info: + case WnHeadsUpType.info: return context.colors.info; } } IconData get icon { switch (this) { - case WnHeadingType.error: + case WnHeadsUpType.error: return CarbonIcons.error_filled; - case WnHeadingType.warning: + case WnHeadsUpType.warning: return CarbonIcons.warning_filled; - case WnHeadingType.info: + case WnHeadsUpType.info: return CarbonIcons.information; } } }Also update the widget class:
- this.type = WnHeadingType.error, + this.type = WnHeadsUpType.error,- final WnHeadingType type; + final WnHeadsUpType type;lib/ui/contact_list/chat_list_screen.dart (1)
273-280: Consider extracting icon size to a constant.The hardcoded dimensions
21.ware used multiple times. Consider extracting to a constant for maintainability.+ static const double _chatIconSize = 21.0; + icon: Image.asset( noRelayConnected ? AssetsPaths.icOffChat : AssetsPaths.icAddNewChat, - width: 21.w, - height: 21.w, + width: _chatIconSize.w, + height: _chatIconSize.w, color: context.colors.primaryForeground.withValues( alpha: noRelayConnected ? 0.5 : 1.0, ), ),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
assets/pngs/ic_add_chat.pngis excluded by!**/*.pngassets/pngs/ic_off_chat.pngis excluded by!**/*.png
📒 Files selected for processing (4)
CHANGELOG.md(1 hunks)lib/ui/contact_list/chat_list_screen.dart(5 hunks)lib/ui/core/themes/assets.dart(1 hunks)lib/ui/core/ui/wn_heads_up.dart(1 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/core/themes/assets.dartlib/ui/contact_list/chat_list_screen.dartlib/ui/core/ui/wn_heads_up.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 (1)
📚 Learning: 2025-08-08T13:39:00.467Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.467Z
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:
lib/ui/contact_list/chat_list_screen.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 (4)
lib/ui/core/themes/assets.dart (1)
62-62: LGTM! Asset path follows established naming conventions.The new asset path for the offline chat icon is properly declared following the existing pattern and naming conventions.
lib/ui/contact_list/chat_list_screen.dart (3)
225-231: LGTM! Relay connectivity check is implemented correctly.The logic properly checks if all relays are disconnected by ensuring none have a
connectedstatus, which aligns with the PR objective to show the indicator only when ALL relays are disconnected.
264-272: LGTM! Add chat button correctly disabled when offline.The button is properly disabled when no relays are connected, preventing users from attempting to create chats while offline. Good UX decision.
337-356: LGTM! Heads-up notification properly integrated.The no-relay indicator is well-implemented with clear messaging and a direct action to resolve the issue. The fade-in animation provides a smooth user experience.
|
BTW I haven't tried it locally yet, but while. try it I left those tiny comments 😉 |
Yea, those icons dont exist in carbon icon |
Wait... realized that the Pr has an issue, the issue has a figma and there it is an icon with a name that actually matches carbon icon name... Apparently it is called chatt--off |
|
I found another issue when trying it locally, the message seems to not be completely sticky, when scrolling it goes down a bit and a white space is shown. Also the loading messages spinner shows above it instead of below. Here is a short video if the issue that I am getting when trying it locally. no-relays-issue.mov |
but the icon does not exist in the carbon icon class, did you check ? |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
lib/ui/contact_list/chat_list_screen.dart (1)
338-357: Consider addressing the UI stickiness issue reported in testingBased on the PR comments, the heads-up component has positioning issues when scrolling (shifts down with white space appearing) and the loading spinner appears above it instead of below. These issues should be resolved to ensure proper sticky behavior.
The heads-up notification implementation looks correct, but there are reported issues with its sticky behavior and interaction with the loading spinner. Please verify:
- The heads-up banner remains truly sticky at the top when scrolling
- The loading spinner appears below the heads-up, not above it
- No white space appears above the heads-up when scrolling
Consider wrapping the heads-up in a
SliverPersistentHeaderor adjusting the sliver ordering to ensure proper sticky behavior.
🧹 Nitpick comments (4)
lib/config/providers/relay_status_provider.dart (2)
152-165: Optimize relay connectivity checks to avoid redundant async operationsThe
_hasConnectedRelayOfType()method converts eachRelayUrlto a string using an async operation. This could be optimized by batching the conversions or using a more efficient approach.Consider optimizing the relay URL conversions:
Future<bool> _hasConnectedRelayOfType(List<RelayUrl> relayUrls) async { if (relayUrls.isEmpty) { return false; } - for (final relayUrl in relayUrls) { - final url = await stringFromRelayUrl(relayUrl: relayUrl); - if (isRelayConnected(url)) { - return true; - } - } - - return false; + // Convert all URLs in parallel for better performance + final urlFutures = relayUrls.map((relayUrl) => stringFromRelayUrl(relayUrl: relayUrl)); + final urls = await Future.wait(urlFutures); + + return urls.any((url) => isRelayConnected(url)); }
174-180: Add error handling for the FutureProviderThe
allRelayTypesConnectionProviderdoesn't handle potential errors fromareAllRelayTypesConnected(). While the method has internal error handling, it's good practice to handle errors at the provider level as well.Consider adding explicit error handling:
// Provider for checking if all relay types have at least one connected relay final allRelayTypesConnectionProvider = FutureProvider<bool>((ref) async { // Watch the relay status provider to trigger rebuilds when statuses change ref.watch(relayStatusProvider); - final notifier = ref.read(relayStatusProvider.notifier); - return await notifier.areAllRelayTypesConnected(); + try { + final notifier = ref.read(relayStatusProvider.notifier); + return await notifier.areAllRelayTypesConnected(); + } catch (e) { + // Log the error for debugging + Logger('allRelayTypesConnectionProvider').warning('Failed to check relay connections: $e'); + // Return false as a safe default when there's an error + return false; + } });lib/ui/contact_list/chat_list_screen.dart (2)
224-229: Simplify error handling in relay connection statusThe error case is correctly handled by returning
true(treating errors as disconnected), which is a safe default. However, the underscore placeholder for the second parameter should be named for clarity.final allRelayTypesConnectionAsync = ref.watch(allRelayTypesConnectionProvider); final notAllRelayTypesConnected = allRelayTypesConnectionAsync.when( data: (allConnected) => !allConnected, loading: () => true, - error: (_, _) => true, + error: (error, stackTrace) => true, );
263-271: Extract complex conditional logic for better readabilityThe onPressed logic for the add chat button contains nested conditionals that could be simplified for better readability.
Consider extracting the handler to a method:
+ void _handleAddChatPressed() { + if (_searchFocusNode.hasFocus) { + _searchFocusNode.unfocus(); + } + NewChatBottomSheet.show(context); + } + // In the build method: IconButton( - onPressed: - notAllRelayTypesConnected - ? null - : () { - if (_searchFocusNode.hasFocus) { - _searchFocusNode.unfocus(); - } - NewChatBottomSheet.show(context); - }, + onPressed: notAllRelayTypesConnected ? null : _handleAddChatPressed,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/config/providers/relay_status_provider.dart(2 hunks)lib/ui/contact_list/chat_list_screen.dart(5 hunks)
🧰 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/config/providers/relay_status_provider.dartlib/ui/contact_list/chat_list_screen.dart
🧠 Learnings (3)
📚 Learning: 2025-08-08T13:39:00.467Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.467Z
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:
lib/config/providers/relay_status_provider.dart
📚 Learning: 2025-08-08T13:39:00.467Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.467Z
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/config/providers/relay_status_provider.dart
📚 Learning: 2025-08-08T13:39:00.467Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.467Z
Learning: Applies to **/*.dart : Use flutter_rust_bridge to access core functionality of the app.
Applied to files:
lib/config/providers/relay_status_provider.dart
… update heads-up notification styling
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
lib/ui/contact_list/chat_list_screen.dart (3)
264-282: Consider using Carbon Icons as suggested in PR comments.According to the PR discussion, there's uncertainty about whether the icon should come from Carbon Icons. The reviewer found a matching "chatt--off" icon in the Figma that appears to correspond to a Carbon icon.
#!/bin/bash # Check if Carbon Icons package has an offline/disabled chat icon rg -i "chat.*off|off.*chat" --type dart#!/bin/bash # Check the assets configuration for the new icon cat pubspec.yaml | grep -A 5 -B 5 "icOffChat\|ic_off_chat"#!/bin/bash # Look for Carbon Icons usage patterns in the codebase rg "CarbonIcons\." --type dart -A 2 | head -30
289-292: Address the reported UI stacking issue with the loading spinner.According to the PR comments, the loading spinner (lines 300-344) appears above the heads-up component instead of below it. The current implementation adds a
SliverToBoxAdapterwith a fixed height of 100.h whennotAllRelayTypesConnectedis true, but this doesn't properly handle the stacking order with the loading spinner.Consider restructuring the sliver order to ensure proper visual hierarchy:
- if (notAllRelayTypesConnected) - SliverToBoxAdapter( - child: SizedBox(height: 100.h), - ), - if (chatItems.isEmpty) const SliverFillRemaining( hasScrollBody: false, child: _EmptyGroupList(), ) else ...[ + if (notAllRelayTypesConnected) + SliverToBoxAdapter( + child: SizedBox(height: 100.h), + ), if (_isRefreshing) SliverToBoxAdapter(
405-427: Fix the sticky positioning issue reported in PR comments.According to the PR comments, the heads-up message is not fully sticky - when scrolling, it shifts down and creates white space. The current implementation uses a fixed position (
top: 64.h + kToolbarHeight) which doesn't account for scroll offset.Consider using a more robust sticky positioning approach:
- if (notAllRelayTypesConnected) - Positioned( - top: 64.h + kToolbarHeight, - left: 0, - right: 0, - child: - WnStickyHeadsUp( + if (notAllRelayTypesConnected) + Positioned( + top: MediaQuery.of(context).padding.top + kToolbarHeight, + left: 0, + right: 0, + child: Material( + elevation: 4, + child: WnStickyHeadsUp(Alternatively, consider integrating the heads-up as a pinned sliver to ensure it stays properly positioned during scrolling.
🧹 Nitpick comments (1)
lib/ui/contact_list/chat_list_screen.dart (1)
100-100: Ensure the relay status refresh completes successfully.Adding relay status refresh to the parallel data loading is good, but ensure error handling is in place if the refresh fails.
Consider wrapping the Future.wait in a try-catch to handle potential failures gracefully:
+ try { await Future.wait([ ref.read(welcomesProvider.notifier).loadWelcomes(), ref.read(groupsProvider.notifier).loadGroups(), ref.read(profileProvider.notifier).fetchProfileData(), ref.read(relayStatusProvider.notifier).refreshStatuses(), ]); + } catch (e) { + // Log error but don't prevent the screen from loading + debugPrint('Error loading data: $e'); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
lib/config/providers/relay_status_provider.dart(2 hunks)lib/ui/contact_list/chat_list_screen.dart(8 hunks)lib/ui/core/ui/wn_heads_up.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/ui/core/ui/wn_heads_up.dart
- lib/config/providers/relay_status_provider.dart
🧰 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/ui/contact_list/chat_list_screen.dart
🔇 Additional comments (1)
lib/ui/contact_list/chat_list_screen.dart (1)
225-230: allRelayTypesConnectionProvider usage is correct and safeI verified that
allRelayTypesConnectionProvideris defined as aFutureProvider<bool>which returnstruewhen every relay type has at least one connected relay andfalseotherwise. In the UI code you’re inverting that result to derivenotAllRelayTypesConnected, and treatingloadingorerrorcases astrue(i.e. “not all connected”) is a safe fallback.No changes needed here.
Oh you are absolutely right! It doesn't work 🙈 . Sorry about that. |
|
In case you agree, I'm leaving a message in the group so that we are all in the same page, because I had misunderstood that we were doing the opposite thing from another PR |
josefinalliende
left a comment
josefinalliende
left a comment
There was a problem hiding this comment.
✅ Tried it locally and now warning works perfect: it is shown when a single kind is missing, and also it is now sticky alwayw 🎉
Description
Add Global No Relay Indicator
Implements a global indicator to alert users when no relays are
connected, addressing issue #350.
Changes
UI Components:
(lib/ui/core/ui/wn_heads_up.dart) - a reusable heads-up
notification component with support for error, warning, and info
states
(lib/ui/contact_list/chat_list_screen.dart) to display the no-relay
indicator at the top when all relays are disconnected
Visual Updates:
Functionality:
no relays are connected
list when disconnected
navigates to network settings
status
Behavior:
Test Plan
Type of Change
Checklist
just precommitto ensure that formatting and linting are correctCHANGELOG.mdfile with your changesSummary by CodeRabbit