-
Notifications
You must be signed in to change notification settings - Fork 14
Adjust bottom padding and gaps across multiple UI screens #723
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
Adjust bottom padding and gaps across multiple UI screens #723
Conversation
WalkthroughMultiple UI screens and bottom-sheet padding and spacing were adjusted: SafeArea bottom insets re-enabled, several vertical gaps reduced, WnBottomSheet’s padding helper renamed and made keyboard-aware (returns 16.h when keyboard visible, else safeAreaBottom+16.h), and new-group-chat sheet layout and paste/npub normalization reflowed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant OS as System (keyboard)
participant Sheet as WnBottomSheet
participant Safe as SafeArea
Note over User,OS: Open sheet or focus input
User->>Sheet: open()/focusInput()
OS->>Sheet: keyboard visibility change
alt keyboard visible
Sheet->>Sheet: _calculateSafeAreaPadding(context) -> 16.h
Sheet->>Safe: apply bottom padding = 16.h
else keyboard hidden
Sheet->>Safe: query safeAreaBottom
Sheet->>Sheet: _calculateSafeAreaPadding(context) -> safeAreaBottom + 16.h
Sheet->>Safe: apply bottom padding = safeAreaBottom + 16.h
end
sequenceDiagram
autonumber
participant User
participant UI as NewGroupChatSheet
participant Clipboard
participant State
User->>UI: type into search
UI->>UI: _onSearchChanged(input)
alt input starts with "npub" and contains whitespace
UI->>UI: normalize (strip whitespace) & update controller/caret
end
User->>UI: tap paste
UI->>Clipboard: readClipboard()
Clipboard-->>UI: pasteText
UI->>UI: normalize pasted npub (if present) and update search controller
UI->>State: initState seeds _selectedUserProfiles from widget.preSelectedUserProfiles
User->>UI: tap Continue (with selection)
UI->>UI: Navigator.pop()
UI->>State: optionally show GroupChatDetailsSheet using parent context
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
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 |
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.
Tested in my iPhone and left some small comments related to cases then keyboard is open in some screens. Not sure if they make sense, would like to hear @vladimir-krstic 🧑🎨 🎨 opinion 😅
Haven't tested in android, I need to buy one cause the one I had is useless 😞
| Expanded( | ||
| child: Padding( | ||
| padding: EdgeInsets.symmetric(vertical: 24.h), | ||
| padding: EdgeInsets.only(top: 24.h), |
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.
I couldn't find a design for that screen of Edit Profile in Figma with the keyboard open 🤔 so not 100% sure if master was ok, but I personally like it a bit more. @vladimir-krstic ho much spacing should be 1) between the About you text field and the save button when the keyboard is open 2) between the save button and the keyboard?
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.
Yes, I noticed this textfield touching the floating button before eventually auto-scrolling up.
With the space there, it looks awkward to me. The textfield looks abruptly cut off. Maybe I should have added a fade there like I did in login? (I'd prefer to do that in a different PR if we're doing it).
For the space between the floating buttons and the keyboard, I used 8.h.
Let's hear @vladimir-krstic's opinion on both.
Your other comments on other screens are similar so I'm not going to respond directly to them @josefinalliende. We'll decide here. Thank you.
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.
I think the fade is a good idea, lets wait for vlad's opinion.
| label: 'shared.save'.tr(), | ||
| ), | ||
| Gap(36.h), | ||
| Gap(8.h), |
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.
| padding: EdgeInsets.only( | ||
| left: 24.w, | ||
| right: 24.w, | ||
| top: 16.h, |
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.
The transition in master when the keyboard open felt a bit smoother due to this top spacing. Here some videos
Master behaviour (there is always a space that then grows)
master-behaviour-create-profile.MP4
This branch behaviour (the is no space for a moment then it grows)
this.branch.MP4
|
Sorry, I've missed this one. Adding fade under the button definitely would make it much better. And bottom spacing, one between button and keyboard should be something like it is now, maybe a bit more, I guess you have it at 12px here, 16px looks a bit better in designs. Like this: https://www.figma.com/design/dGJqv6m6pchdcwxJEwxxzB/White-Noise---Design-System?node-id=3722-45745&t=xKFCdBrkii6PK8Vs-1 |
thanks @vladimir-krstic !! So @Quwaysim maybe the fade can be a separate issue for another PR as you said. And in this PR just change the spacing from 12px to 16px WDYT? |
…button' of https://github.com/parres-hq/whitenoise_flutter into 720-os-3-button-navigation-partially-covers-start-chat-button
…button' of https://github.com/parres-hq/whitenoise_flutter into 720-os-3-button-navigation-partially-covers-start-chat-button
a9e4e32 to
370c55c
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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
CHANGELOG.md (1)
38-41: Duplicate “### Security” heading.There are two consecutive “### Security” sections; remove one.
Apply this diff:
-### Security - -### Security +### Security
🧹 Nitpick comments (4)
lib/ui/core/ui/wn_bottom_sheet.dart (2)
119-127: Use a named constant and update spacing to 16px as per design feedback.Replace magic
8.hwith a constant (16px requested in PR comments) to keep spacing consistent and adjustable.Apply this diff:
/// Calculate bottom padding for safe area - /// - When keyboard is open: 8.h padding - /// - When keyboard is closed: device safe area (home indicator) + 8.h padding + /// - When keyboard is open: 16.h padding + /// - When keyboard is closed: device safe area (home indicator) + 16.h padding + static const double _kExtraBottomPadding = 16.0; static double _calculateSafeAreaPadding(BuildContext context) { - final keyboardHeight = MediaQuery.viewInsetsOf(context).bottom; - final safeAreaBottom = MediaQuery.viewPaddingOf(context).bottom; - - return keyboardHeight > 0 ? 8.h : safeAreaBottom + 8.h; + final double extra = _kExtraBottomPadding.h; + final double keyboardHeight = MediaQuery.viewInsetsOf(context).bottom; + final double safeAreaBottom = MediaQuery.viewPaddingOf(context).bottom; + return keyboardHeight > 0 ? extra : safeAreaBottom + extra; }If 16px is not final, centralizing it still helps future tweaks.
41-45: Avoid double overlay tint (barrier + custom faded scrim).You’re drawing an animated scrim in
buildTransitionsand also returning a non‑transparentbarrierColor, which yields a darker-than-intended background.Option A (keep custom fade): make the modal barrier transparent.
- Color get barrierColor => Colors.black.withValues(alpha: 0.3); + Color get barrierColor => Colors.transparent;Option B (use default barrier fade): remove the custom faded
Containerand rely onbarrierColor. Pick one for consistent visuals.Also applies to: 82-89
lib/ui/user_profile_list/new_group_chat_sheet.dart (1)
233-239: Use design-system button for consistency.Prefer
WnFilledButton(or the project’s standard) overElevatedButtonin bottom sheets for visual consistency.- ElevatedButton( - onPressed: () { + WnFilledButton( + onPressed: () { // Navigate back - contacts should be loaded by new_chat_bottom_sheet Navigator.of(context).pop(); }, - child: Text('shared.goBack'.tr()), + label: 'shared.goBack'.tr(), ),CHANGELOG.md (1)
36-36: Reference the PR for traceability.Link the fix to this PR.
-- Fixed start chat button cut off (and other bottom sheets). +- Fixed start chat button cut off (and other bottom sheets). [#723]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
CHANGELOG.md(1 hunks)lib/ui/auth_flow/create_profile_screen.dart(1 hunks)lib/ui/chat/chat_info/edit_group_screen.dart(1 hunks)lib/ui/core/ui/wn_bottom_sheet.dart(2 hunks)lib/ui/settings/profile/edit_profile_screen.dart(2 hunks)lib/ui/settings/profile/switch_profile_bottom_sheet.dart(0 hunks)lib/ui/user_profile_list/group_chat_details_sheet.dart(0 hunks)lib/ui/user_profile_list/new_group_chat_sheet.dart(1 hunks)
💤 Files with no reviewable changes (2)
- lib/ui/settings/profile/switch_profile_bottom_sheet.dart
- lib/ui/user_profile_list/group_chat_details_sheet.dart
🚧 Files skipped from review as they are similar to previous changes (3)
- lib/ui/chat/chat_info/edit_group_screen.dart
- lib/ui/settings/profile/edit_profile_screen.dart
- lib/ui/auth_flow/create_profile_screen.dart
🧰 Additional context used
📓 Path-based instructions (3)
**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
**/*.dart: Always declare the type of each variable and function (parameters and return value)
Avoid using dynamic and Object without justification
Create necessary types instead of overusing primitives or dynamic
One export per file
Use PascalCase for classes
Use camelCase for variables, functions, and methods
Avoid magic numbers and define constants
Start each function name with a verb
Use verbs for boolean variables (e.g., isLoading, hasError, canDelete)
Write short functions with a single purpose (under ~20 instructions)
Name functions with a verb plus context; for booleans use isX/hasX/canX; for void use executeX/saveX
Avoid deep nesting via early returns and extraction to utility functions
Use higher-order functions (map, where/filter, reduce) to avoid nesting
Use arrow functions for simple functions (under ~3 statements); use named functions otherwise
Use default parameter values instead of null checks
Reduce function parameters using RO-RO: pass/return parameter objects with declared types
Maintain a single level of abstraction within functions
Encapsulate data in composite types; avoid overusing primitives
Prefer validating data within classes rather than in functions
Prefer immutability; use final for runtime constants and const for compile-time constants
Use const constructors and const literals where possible
Follow SOLID principles
Prefer composition over inheritance
Declare interfaces (abstract classes) to define contracts
Write small classes with a single purpose (under ~200 instructions, <10 public methods, <10 properties)
Use exceptions for unexpected errors
Only catch exceptions to fix expected problems or add context; otherwise use a global handler
Files:
lib/ui/core/ui/wn_bottom_sheet.dartlib/ui/user_profile_list/new_group_chat_sheet.dart
lib/**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
lib/**/*.dart: Use flutter_rust_bridge to access core app functionality
Use Riverpod for state management; prefer StreamProviders for Rust API streams; use keepAlive if needed
Use freezed to model/manage UI states
Controllers should expose methods as inputs and update UI state that drives the UI
Use AutoRoute for navigation and use extras to pass data between pages
Use Dart extensions to manage reusable code
Use ThemeData to manage themes
Use AppLocalizations for translations
Use constants to manage constant values
Avoid deeply nested widget trees; aim for a flatter widget structure for performance and readability
Break down large widgets into smaller, focused, reusable components
Keep the widget tree shallow to simplify state management and data flow
Utilize const constructors and const widgets wherever possible to reduce rebuilds
Files:
lib/ui/core/ui/wn_bottom_sheet.dartlib/ui/user_profile_list/new_group_chat_sheet.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-09-16T07:24:07.489Z
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.
Applied to files:
CHANGELOG.md
🪛 GitHub Actions: CI
lib/ui/core/ui/wn_bottom_sheet.dart
[error] 128-128: Flutter analyze reported dead_code in lib/ui/core/ui/wn_bottom_sheet.dart:128:5. (dead_code).
🪛 LanguageTool
CHANGELOG.md
[style] ~36-~36: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ty in auth screens (iOS and Android). - Fixed start chat button cut off (and other bo...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
⏰ 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
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: 1
🧹 Nitpick comments (1)
lib/ui/user_profile_list/new_group_chat_sheet.dart (1)
210-244: Consider using WnButton for UI consistency.The error state implementation is solid with proper loading, error display, and recovery action. However, the "Go back" button uses
ElevatedButtonwhile the rest of the app uses customWn*Buttoncomponents.Consider this change for consistency:
- ElevatedButton( - onPressed: () { - // Navigate back - contacts should be loaded by new_chat_bottom_sheet - Navigator.of(context).pop(); - }, - child: Text('shared.goBack'.tr()), - ), + WnFilledButton( + onPressed: () { + // Navigate back - contacts should be loaded by new_chat_bottom_sheet + Navigator.of(context).pop(); + }, + label: 'shared.goBack'.tr(), + ),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/ui/user_profile_list/new_group_chat_sheet.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 dynamic and Object without justification
Create necessary types instead of overusing primitives or dynamic
One export per file
Use PascalCase for classes
Use camelCase for variables, functions, and methods
Avoid magic numbers and define constants
Start each function name with a verb
Use verbs for boolean variables (e.g., isLoading, hasError, canDelete)
Write short functions with a single purpose (under ~20 instructions)
Name functions with a verb plus context; for booleans use isX/hasX/canX; for void use executeX/saveX
Avoid deep nesting via early returns and extraction to utility functions
Use higher-order functions (map, where/filter, reduce) to avoid nesting
Use arrow functions for simple functions (under ~3 statements); use named functions otherwise
Use default parameter values instead of null checks
Reduce function parameters using RO-RO: pass/return parameter objects with declared types
Maintain a single level of abstraction within functions
Encapsulate data in composite types; avoid overusing primitives
Prefer validating data within classes rather than in functions
Prefer immutability; use final for runtime constants and const for compile-time constants
Use const constructors and const literals where possible
Follow SOLID principles
Prefer composition over inheritance
Declare interfaces (abstract classes) to define contracts
Write small classes with a single purpose (under ~200 instructions, <10 public methods, <10 properties)
Use exceptions for unexpected errors
Only catch exceptions to fix expected problems or add context; otherwise use a global handler
Files:
lib/ui/user_profile_list/new_group_chat_sheet.dart
lib/**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
lib/**/*.dart: Use flutter_rust_bridge to access core app functionality
Use Riverpod for state management; prefer StreamProviders for Rust API streams; use keepAlive if needed
Use freezed to model/manage UI states
Controllers should expose methods as inputs and update UI state that drives the UI
Use AutoRoute for navigation and use extras to pass data between pages
Use Dart extensions to manage reusable code
Use ThemeData to manage themes
Use AppLocalizations for translations
Use constants to manage constant values
Avoid deeply nested widget trees; aim for a flatter widget structure for performance and readability
Break down large widgets into smaller, focused, reusable components
Keep the widget tree shallow to simplify state management and data flow
Utilize const constructors and const widgets wherever possible to reduce rebuilds
Files:
lib/ui/user_profile_list/new_group_chat_sheet.dart
🧠 Learnings (1)
📚 Learning: 2025-09-15T20:51:15.316Z
Learnt from: Quwaysim
PR: parres-hq/whitenoise_flutter#639
File: lib/ui/auth_flow/login_screen.dart:140-165
Timestamp: 2025-09-15T20:51:15.316Z
Learning: In the whitenoise_flutter codebase, prefer simple context.pop() for back actions rather than more complex Navigator.maybePop() with fallback logic, as indicated by the maintainer Quwaysim.
Applied to files:
lib/ui/user_profile_list/new_group_chat_sheet.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 (2)
lib/ui/user_profile_list/new_group_chat_sheet.dart (2)
174-209: UI restructuring looks clean.The consolidation of the search field and paste button into a single row simplifies the layout and maintains proper functionality.
245-262: ****Safe-area padding is already properly applied. The
WnBottomSheet.show()method hasuseSafeArea = trueas its default parameter (line 142 in wn_bottom_sheet.dart), which automatically applies the_calculateSafeAreaPaddinghelper to the entire sheet content (line 189). Sincenew_group_chat_sheet.dartcallsWnBottomSheet.show()without explicitly overriding this parameter (lines 35-45), the safe-area padding is enabled by default and applies to all content within the sheet, including the continue button at lines 245-262. No additional padding implementation is needed in this file.Likely an incorrect or invalid review comment.
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
…s-start-chat-button



Description
Fixes: #720. Bottom Sheet Safe Area Padding for 3-Button Navigation on Android and GrapheneOS.
Problem
Bottom sheets were not properly accounting for device safe areas (home indicator on iOS, 3-button navigation on Android), causing content to be partially covered or cut off at the bottom.
Issues:
Solution
1. Simplified Bottom Sheet Padding Calculation
2. Cleaned Up Individual Screens
Removed manual safe area workarounds from:
SafeArea(bottom: false)wrapperPadding(bottom: 16.h)Gap(16.h)Gap(28.h)SafeArea(bottom: false)and manual paddingSafeArea(bottom: false)and manualGap(36.h)Behavior
Keyboard Closed:
Keyboard Open:
Testing
✅ Verified on devices with:
Benefits
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
UI/UX Improvements
Documentation