Skip to content

Conversation

@codeswot
Copy link
Contributor

@codeswot codeswot commented Sep 22, 2025

Description

Show Error banner directly under the app bar to avoid cutoff and to properly allign spacing

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

  • New Features

    • Adds an in-list heads-up banner on the chat list to alert relay connection issues.
    • Includes an underlined “Connect Relays” action that opens Network settings.
    • Banner appears with a smooth fade-in animation and scrolls with content.
  • Style

    • Replaces the intrusive floating overlay with a cleaner, integrated banner for relay errors.
    • Removes placeholder spacing, providing a more polished and consistent layout.

@codeswot codeswot requested a review from Quwaysim September 22, 2025 13:53
@codeswot codeswot self-assigned this Sep 22, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 22, 2025

Walkthrough

Replaces the relay-error floating overlay with an in-flow WnHeadsUp banner placed via SliverToBoxAdapter within the CustomScrollView. Removes the old Positioned overlay and placeholder spacer, adds an animate().fadeIn() on the banner, and preserves navigation to settingsNetwork via the banner action.

Changes

Cohort / File(s) Summary
Relay error banner refactor
lib/ui/contact_list/chat_list_screen.dart
Removed floating Positioned relay-error overlay and spacer. Added WnHeadsUp banner as a SliverToBoxAdapter within slivers, with underline “Connect Relays” action and fade-in animation. Action continues to navigate to settingsNetwork.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant ChatList as ChatListScreen
  participant Banner as WnHeadsUp (Sliver)
  participant Nav as Navigator

  User->>ChatList: Open screen
  ChatList->>Banner: Render in sliver flow (fadeIn)
  alt Relay error present
    Banner-->>User: Show "Connect Relays" action
    User->>Banner: Tap action
    Banner->>Nav: push(settingsNetwork)
  else No relay error
    Note over ChatList: Banner not shown
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • josefinalliende
  • Quwaysim
  • untreu2

Poem

A banner hops into the scroll, neat and light,
No more floating clouds obscuring sight.
“Connect Relays,” it winks, a subtle cue—
Fade in, glide on, the chats come through.
Thump-thump, says the rabbit, with UI delight.

Pre-merge checks and finishing touches

✅ 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 accurately captures the main change—fixing the error banner so it aligns under the app bar—and therefore reflects the PR objectives, but it contains minor typos/spacing issues ("alllign" and "appbar") that reduce polish and clarity.
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-error-banner-alllign-under-appbar

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1d6bee4 and 44644b4.

⛔ Files ignored due to path filters (1)
  • ios/Podfile.lock is excluded by !**/*.lock
📒 Files selected for processing (1)
  • lib/ui/contact_list/chat_list_screen.dart (1 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/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 (1)
lib/ui/contact_list/chat_list_screen.dart (1)

338-357: LGTM! The error banner positioning fix looks good.

The change successfully moves the error banner from a floating overlay to an in-flow sliver, which will prevent it from being cut off and properly align it under the app bar. The banner retains its functionality with the "Connect Relays" action and includes a nice fade-in animation.


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.

Copy link
Contributor

@josefinalliende josefinalliende left a comment

Choose a reason for hiding this comment

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

How can I reproduce this error @codeswot ? I was trying by removing all the relays from one of the lists, then switched back to chat screen, wait for a long time but the error does not appear, so I can't see it now is aligned or not 😅

@codeswot
Copy link
Contributor Author

How can I reproduce this error @codeswot ? I was trying by removing all the relays from one of the lists, then switched back to chat screen, wait for a long time but the error does not appear, so I can't see it now is aligned or not 😅

On iOS the alignment was fine, you got to test on iOS. You can negate the book flag that shows the error to see the banner and see the alignment. Then for me how I test the connection lost flow is I stop running the docker container completely and wait for 30 sec

@erskingardner
Copy link
Member

How can I reproduce this error @codeswot ? I was trying by removing all the relays from one of the lists, then switched back to chat screen, wait for a long time but the error does not appear, so I can't see it now is aligned or not 😅

@josefinalliende you can just shutdown your local relays if you're running on the simulator and wait 30 seconds.

Copy link
Contributor

@josefinalliende josefinalliende left a comment

Choose a reason for hiding this comment

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

It worked when running with local relays! 🎉

@erskingardner erskingardner merged commit 34cdf96 into master Sep 22, 2025
3 checks passed
@erskingardner erskingardner deleted the fix-error-banner-alllign-under-appbar branch September 22, 2025 19:02
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.

4 participants