Fix button paddings #671
Fix button paddings #671
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughRefactors the login screen layout and increases keyboard-visible scroll delay to 500ms. Adjusts vertical spacing in multiple UI screens and tightens developer/profile layouts. Adds a guard preventing negative indexing in the new chat bottom sheet. No exported API or core logic signature changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as NewChatBottomSheet
participant Contacts as FilteredContacts
User->>UI: Tap/contact selection
UI->>UI: Compute contactIndex
alt 0 <= contactIndex < Contacts.length
UI->>Contacts: Access filteredContacts[contactIndex]
Contacts-->>UI: Return contact
UI-->>User: Open/start chat
else invalid index
UI-->>User: No-op / prevent access
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/ui/settings/profile/edit_profile_screen.dart (1)
368-371: Prevent RangeError when displayName is emptyIndexing displayName[0] can crash if the string is empty (e.g., no profile name yet).
Apply this diff:
- displayName[0].toUpperCase(), + (displayName.isNotEmpty ? displayName[0] : '?').toUpperCase(),
🧹 Nitpick comments (3)
lib/ui/auth_flow/login_screen.dart (3)
60-69: Replace magic delay with a named constant; consider a metrics-stable triggerA hardcoded 500ms is brittle across devices/OS versions.
Apply this diff to de‑magic the delay:
class _LoginScreenState extends ConsumerState<LoginScreen> with WidgetsBindingObserver { + static const Duration kKeyboardScrollDelay = Duration(milliseconds: 500); @@ - Future.delayed(const Duration(milliseconds: 500), () { + Future.delayed(kKeyboardScrollDelay, () { if (mounted && _scrollController.hasClients) { _scrollController.animateTo( _scrollController.position.maxScrollExtent, duration: const Duration(milliseconds: 300), curve: Curves.easeInOut, ); } });Optional: instead of scrolling to maxScrollExtent, target the field with Scrollable.ensureVisible once viewInsets stabilizes to avoid overscroll.
152-163: Avoid “magic” vertical spacersThe twin Gap(79.5.h) reads like a tuned magic value. Prefer a named constant (e.g., Spacing.xxxLarge) to keep spacing consistent and discoverable.
Would you like a quick pass to introduce a shared spacing constants file and replace these literals across the touched screens?
200-205: Comment nitMinor typos (“bothe”). Clean up for clarity.
Apply this diff:
- // Used .h for bothe to make it square and also go along with the 56.h + // Used .h for both to make it square and to match the 56.h
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
ios/Podfile.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
lib/ui/auth_flow/login_screen.dart(2 hunks)lib/ui/auth_flow/welcome_screen.dart(1 hunks)lib/ui/chat/chat_info/dm_chat_info.dart(2 hunks)lib/ui/contact_list/new_chat_bottom_sheet.dart(1 hunks)lib/ui/settings/developer/developer_settings_screen.dart(1 hunks)lib/ui/settings/profile/edit_profile_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/settings/developer/developer_settings_screen.dartlib/ui/contact_list/new_chat_bottom_sheet.dartlib/ui/auth_flow/welcome_screen.dartlib/ui/auth_flow/login_screen.dartlib/ui/settings/profile/edit_profile_screen.dartlib/ui/chat/chat_info/dm_chat_info.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 (5)
lib/ui/settings/profile/edit_profile_screen.dart (1)
310-311: Spacing tweak after “Discard Changes”: LGTM8.h reads better than 4.h and keeps CTAs visually distinct.
lib/ui/auth_flow/welcome_screen.dart (1)
102-102: Button spacing: LGTMIncreasing to Gap(8.h) improves tap target separation in the bottom bar.
lib/ui/chat/chat_info/dm_chat_info.dart (1)
154-155: Tighter vertical rhythm: LGTMReducing gaps to 8.h between stacked actions (“Search Chat”, “Follow/Unfollow”, “Add to Group”) keeps the panel dense without harming readability.
Also applies to: 173-173
lib/ui/contact_list/new_chat_bottom_sheet.dart (1)
386-396: Fix for intermittent RangeError: good guardThe added check (contactIndex >= 0 && contactIndex < filteredContacts.length) addresses the out-of-range crash during list building.
Consider a small widget test to lock this in (e.g., rapidly toggling showTempContact and filteredContacts lengths while rebuilding the list) to ensure no regressions around section index math.
lib/ui/settings/developer/developer_settings_screen.dart (1)
293-293: Consistent spacing scale: LGTMStandardizing to Gap(8.h) aligns with the PR’s spacing goals.
Also applies to: 299-299
josefinalliende
left a comment
josefinalliende
left a comment
There was a problem hiding this comment.
All the screens touched look good to me!! 👌🏻 I found just two other screens untouched in this PR where I think the spacing between buttos is not consistent yet.
| ); | ||
| }, | ||
| ), | ||
| // Gap(16.h), |
There was a problem hiding this comment.
sug: remove commented code
erskingardner
left a comment
erskingardner
left a comment
There was a problem hiding this comment.
Actually - I saw @josefinalliende 's point about the two screens that were missed. Can you also include a screenshot of the log in screen?
Login screen looks fine to me! |
Description
This PR contains two important improvements:
Code Cleanup:
Removed unnecessary whitespace and formatting inconsistencies
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
New Features
Style
Refactor
Bug Fixes