-
Notifications
You must be signed in to change notification settings - Fork 14
fix: adjust welcome screen padding and add border to chat list avatar #655
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
WalkthroughThe welcome screen adjusts bottom safe area behavior and spacing: disables bottom SafeArea inset, increases bottom padding to 54.h, and reduces inter-button spacing to 4.h. The chat list active account avatar now passes showBorder: true to WnAvatar, adding a border. No public APIs changed. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
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: 2
🧹 Nitpick comments (1)
lib/ui/auth_flow/welcome_screen.dart (1)
93-94: Avoid double bottom spacing; rely on SafeArea.minimumIf you adopt
SafeArea.minimum, the explicitPadding(bottom: 54.h)will stack with system insets. Zero it (or remove the wrapper) to keep spacing = max(systemInset, 54.h).- child: Padding( - padding: EdgeInsets.only(bottom: 54.h), + child: Padding( + padding: EdgeInsets.zero,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/ui/auth_flow/welcome_screen.dart(2 hunks)lib/ui/contact_list/widgets/chat_list_active_account_avatar.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/auth_flow/welcome_screen.dartlib/ui/contact_list/widgets/chat_list_active_account_avatar.dart
🧠 Learnings (1)
📓 Common learnings
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.
⏰ 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/auth_flow/welcome_screen.dart (1)
102-102: 4.h inter‑button gap change looks good; confirm a11y with designThe spec calls for 4px and this matches. Just confirm it doesn’t visually merge the buttons on small screens or impair focus outlines.
| ), | ||
| bottomNavigationBar: SafeArea( | ||
| top: false, | ||
| bottom: false, |
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.
🧩 Analysis chain
Re-enable bottom SafeArea; use SafeArea.minimum to guarantee 54px while protecting against home indicator
Disabling the bottom inset risks overlap with the iOS home indicator and Android gesture area. Keep bottom: true and set a minimum to enforce the 54px spec.
- bottom: false,
+ bottom: true,
+ minimum: EdgeInsets.only(bottom: 54.h),Please sanity‑check on devices with large insets (e.g., iPhone 14/15, Pixel 7) to confirm no overlap.
🏁 Script executed:
#!/bin/bash
# Search for the file and relevant SafeArea usage / bottom:false occurrences.
echo "Searching for welcome_screen.dart..."
file=$(find . -type f -name 'welcome_screen.dart' -print | head -n1 || true)
if [ -n "$file" ]; then
echo "Found: $file"
else
echo "welcome_screen.dart not found"
fi
echo
echo "Searching for 'SafeArea' usages and 'bottom: false' across repo..."
rg -n --hidden --no-ignore -S "SafeArea\\s*\\(" || true
rg -n --hidden --no-ignore -S "bottom:\\s*false" || true
if [ -n "$file" ]; then
echo
echo "Showing lines 1-220 of $file (numbered):"
nl -ba "$file" | sed -n '1,220p'
fiLength of output: 4641
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file=./lib/ui/auth_flow/welcome_screen.dart
if [ ! -f "$file" ]; then
echo "File not found: $file"
exit 0
fi
echo "File: $file"
echo "----- showing lines 1-240 -----"
awk '{printf "%4d: %s\n", NR, $0}' "$file" | sed -n '1,240p' || true
echo
echo "----- SafeArea usages with context (files and excerpts) -----"
rg -n -C2 "SafeArea\\s*\\(" || true
echo
echo "----- SafeArea(bottom: false) occurrences (with context) -----"
rg -n -C2 "SafeArea\\s*\\([^)]*bottom:\\s*false" || true
echo
echo "----- Files where bottomNavigationBar: SafeArea is used -----"
rg -n "bottomNavigationBar:\\s*SafeArea" || trueLength of output: 13437
Re-enable bottom SafeArea; use SafeArea.minimum to guarantee 54px while protecting against home indicator
Set bottom: true and add a minimum so the SafeArea enforces 54.h but still respects larger device insets.
Location: lib/ui/auth_flow/welcome_screen.dart — bottomNavigationBar SafeArea (lines ~89–94).
- bottom: false,
+ bottom: true,
+ minimum: EdgeInsets.only(bottom: 54.h),Sanity-check on devices with large insets (iPhone 14/15, Pixel 7).
📝 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.
| bottom: false, | |
| bottom: true, | |
| minimum: EdgeInsets.only(bottom: 54.h), |
🤖 Prompt for AI Agents
In lib/ui/auth_flow/welcome_screen.dart around lines 89–94, the SafeArea used
for the bottomNavigationBar currently has bottom: false; change it to bottom:
true and add SafeArea.minimum with EdgeInsets.only(bottom: 54.h) (or the
equivalent sized value used in the project) so the widget always reserves at
least 54px while still expanding for larger device insets; update any
imports/usage required to access .h sizing and run a sanity check on devices
with large home indicators (e.g., iPhone 14/15, Pixel 7).
| showBorder: true, | ||
| ), |
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.
🛠️ Refactor suggestion
🧩 Analysis chain
Border should be dark‑mode only per PR description
The change forces a border in all themes. If the intent is “dark mode only,” gate it on theme brightness to avoid a light‑mode regression.
- showBorder: true,
+ showBorder: Theme.of(context).brightness == Brightness.dark,Optionally verify WnAvatar’s showBorder semantics:
🏁 Script executed:
#!/bin/bash
# Locate WnAvatar and inspect showBorder defaults/usages.
fd -a 'wn_avatar.dart'
rg -n -C2 'class\s+WnAvatar|showBorder' $(fd -a 'wn_avatar.dart' | tr '\n' ' ')Length of output: 588
Gate avatar border to dark mode
File: lib/ui/contact_list/widgets/chat_list_active_account_avatar.dart:86-87
WnAvatar.showBorder defaults to false; use a theme check so the border only appears in dark mode.
- showBorder: true,
+ showBorder: Theme.of(context).brightness == Brightness.dark,📝 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.
| showBorder: true, | |
| ), | |
| showBorder: Theme.of(context).brightness == Brightness.dark, | |
| ), |
🤖 Prompt for AI Agents
In lib/ui/contact_list/widgets/chat_list_active_account_avatar.dart around lines
86-87, the WnAvatar currently uses showBorder: true; change this to
conditionally enable the border only in dark mode by using the theme brightness
(e.g. showBorder: Theme.of(context).brightness == Brightness.dark). Ensure this
change is made inside the widget's build method (so context is available) or
otherwise obtain the ThemeData, and remove the hardcoded true.
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.
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.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find and examine the WnAvatar implementation to understand showBorder behavior
fd -t f -e dart -a wn_avatar.dart
# Get the full implementation
cat lib/ui/core/ui/wn_avatar.dart
# Look for any theme-related styling or border logic
rg -n -C5 "showBorder|border|theme|Brightness" lib/ui/core/ui/wn_avatar.dart
Length of output: 3801
@josefinalliende You're absolutely right! Thank you for testing it locally and providing those helpful screenshots.
After examining the WnAvatar implementation, I can see why it looks fine in both modes. The component already has theme-aware border styling built in:
border: Border.all(
color: borderColor ?? context.colors.border,
width: 1.w,
),The context.colors.border automatically uses the appropriate color for each theme - a subtle color in light mode (which is why you can barely detect it) and a more visible color in dark mode. This is exactly the behavior you're seeing in your screenshots.
My original suggestion to add conditional logic based on theme brightness was unnecessary since the theme-aware styling is already handled by the color system. The current implementation with showBorder: true is actually correct as it lets the theme system handle the appropriate border appearance for each mode.
Thanks for catching this and testing it thoroughly! The PR implementation is working as intended. 🐰
ʕ•ᴥ•ʔ
🧠 Learnings used
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.
josefinalliende
left a comment
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.
LGTM! ✅ I think the border in avatar is in both themes, but it looks good for me 👍🏻


Description
Fixes #569 (padding issues on welcome screen) and added border to user avatar in dark mode on the ChatListScreen.
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