-
Notifications
You must be signed in to change notification settings - Fork 13
Fix header on developer settings, fix padding on start new chat sheets #677
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
WalkthroughList padding removed in new chat/group sheets and small UI adjustments to NewChatTile (icons, gaps, text size). Developer Settings replaced its header with WnAppBar, converted to a scrollable layout, and added a structured “Key Package Management” block with Publish/Inspect/Delete actions and conditional key-packages listing. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant DevSettings as DeveloperSettingsScreen
participant KeySvc as KeyPackageService
participant Dialog as ConfirmationDialog
User->>DevSettings: Tap Publish / Inspect / Delete
alt Inspect
DevSettings->>KeySvc: fetchKeyPackages()
KeySvc-->>DevSettings: packages[]
DevSettings-->>User: show packages or "No key packages found"
else Publish
DevSettings->>KeySvc: publishKeyPackages()
KeySvc-->>DevSettings: result
DevSettings-->>User: show status / refresh
else Delete (item)
DevSettings->>Dialog: show confirmation
Dialog-->>DevSettings: confirmed / cancelled
opt confirmed
DevSettings->>KeySvc: deletePackage(id)
KeySvc-->>DevSettings: result
DevSettings-->>User: update list / show message
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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. Comment |
a2689d3 to
1c1d4a6
Compare
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!
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 (3)
lib/ui/contact_list/new_chat_bottom_sheet.dart (1)
521-522: Tile sizing tweaks: ensure 48dp min touch target and avoid magic numbersThe visual changes read well. To guarantee accessibility across densities, enforce a 48dp min height for the row and consider centralizing the 12/16/20 sizes into constants.
Example patch to enforce the touch target (outside the changed lines, wrapping the Row):
- child: Row( + child: ConstrainedBox( + constraints: BoxConstraints(minHeight: 48.h), + child: Row( children: [ ... - ], - ), + ], + ), + ),Also applies to: 524-524, 529-529, 532-537
lib/ui/settings/developer/developer_settings_screen.dart (2)
237-260: Back nav uses context.pop(); verify with Navigator.push entryThis screen is opened via Navigator.push in show(). Ensure context.pop() resolves to the same Navigator; otherwise, prefer Navigator.of(context).pop() here or migrate show() to go_router for consistency.
282-307: Localize newly added stringsMove “Key Package Management”, “Publish new key package”, “Inspect relay key packages”, and “Delete all key packages from relays” to AppLocalizations to follow the project’s i18n guideline.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
lib/ui/contact_list/new_chat_bottom_sheet.dart(2 hunks)lib/ui/contact_list/new_group_chat_sheet.dart(1 hunks)lib/ui/settings/developer/developer_settings_screen.dart(2 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/new_chat_bottom_sheet.dartlib/ui/contact_list/new_group_chat_sheet.dartlib/ui/settings/developer/developer_settings_screen.dart
🧠 Learnings (5)
📚 Learning: 2025-08-08T13:39:00.500Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.500Z
Learning: Applies to **/*.dart : Use flutter_rust_bridge to access core functionality of the app.
Applied to files:
lib/ui/settings/developer/developer_settings_screen.dart
📚 Learning: 2025-08-23T11:02:28.308Z
Learnt from: Quwaysim
PR: parres-hq/whitenoise_flutter#527
File: lib/ui/core/ui/wn_avatar.dart:1-6
Timestamp: 2025-08-23T11:02:28.308Z
Learning: In the whitenoise_flutter codebase, flutter_screenutil is re-exported through lib/ui/core/themes/src/app_theme.dart, so files that import app_theme.dart do not need to directly import flutter_screenutil to use .w, .h, .sp extensions.
Applied to files:
lib/ui/settings/developer/developer_settings_screen.dart
📚 Learning: 2025-08-08T13:39:00.500Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.500Z
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/ui/settings/developer/developer_settings_screen.dart
📚 Learning: 2025-08-08T13:39:00.500Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.500Z
Learning: Applies to **/*.dart : Use AutoRoute to manage routes. Use extras to pass data between pages.
Applied to files:
lib/ui/settings/developer/developer_settings_screen.dart
📚 Learning: 2025-08-08T13:39:00.500Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.500Z
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/settings/developer/developer_settings_screen.dart
🔇 Additional comments (2)
lib/ui/contact_list/new_group_chat_sheet.dart (1)
113-113: ListView padding set to zero — looks good; confirm tile insetsZero padding aligns with the other sheet. Please double‑check ContactListTile provides its own horizontal insets so items don’t touch the sheet edges on small devices.
lib/ui/contact_list/new_chat_bottom_sheet.dart (1)
326-326: Consistent list paddingSetting padding to EdgeInsets.zero matches the new group sheet and keeps spacing predictable with the explicit item paddings/gaps.
| Expanded( | ||
| child: RepaintBoundary( | ||
| child: ListView.separated( | ||
| itemCount: _keyPackages.length, | ||
| separatorBuilder: (context, index) => SizedBox(height: 8.h), | ||
| itemBuilder: (context, index) { | ||
| final keyPackage = _keyPackages[index]; | ||
| return RepaintBoundary( | ||
| child: _KeyPackageItem( | ||
| keyPackage: keyPackage, | ||
| index: index, | ||
| isLoading: _isLoading, | ||
| onDelete: () => _deleteKeyPackage(keyPackage.id, index), | ||
| ), | ||
| ); | ||
| }, | ||
| ), | ||
| ), | ||
| ), |
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.
Expanded inside SingleChildScrollView will throw (unbounded height)
Expanded under a Column that’s inside a SingleChildScrollView causes “RenderFlex children have non‑zero flex but incoming height constraints are unbounded.” Remove Expanded and make the inner ListView non‑scrollable + shrinkWrap, or switch the whole body to a CustomScrollView with Slivers.
Apply this fix:
- Expanded(
- child: RepaintBoundary(
- child: ListView.separated(
+ RepaintBoundary(
+ child: ListView.separated(
+ shrinkWrap: true,
+ physics: const NeverScrollableScrollPhysics(),
itemCount: _keyPackages.length,
separatorBuilder: (context, index) => SizedBox(height: 8.h),
itemBuilder: (context, index) {
final keyPackage = _keyPackages[index];
- return RepaintBoundary(
- child: _KeyPackageItem(
+ return RepaintBoundary(
+ child: _KeyPackageItem(
keyPackage: keyPackage,
index: index,
isLoading: _isLoading,
onDelete: () => _deleteKeyPackage(keyPackage.id, index),
),
);
},
),
- ),
- ),
+ ),
+ ),🤖 Prompt for AI Agents
In lib/ui/settings/developer/developer_settings_screen.dart around lines 358 to
376, the Expanded wrapping the ListView is inside a Column that’s wrapped by a
SingleChildScrollView which causes unbounded height errors; remove the Expanded
and make the inner ListView non-scrollable by setting shrinkWrap: true and
physics: NeverScrollableScrollPhysics (or alternatively constrain the height
with a SizedBox); keep the RepaintBoundary and existing itemBuilder but ensure
the ListView is non-scrolling so it can lay out inside the SingleChildScrollView
without flex errors.
Summary by CodeRabbit
New Features
Refactor
Style