Skip to content

Conversation

@erskingardner
Copy link
Member

@erskingardner erskingardner commented Sep 22, 2025

Before After
CleanShot 2025-09-22 at 22 06 35 CleanShot 2025-09-22 at 22 05 25

Summary by CodeRabbit

  • New Features

    • Developer Settings: persistent Key Package Management header with Publish/Inspect/Delete action buttons and per-action loading states.
  • Refactor

    • Developer Settings redesigned with a custom app bar, scrollable layout, and clearer presentation of key packages including bordered item containers and integrated confirmation flows.
  • Style

    • New Chat and Group Chat lists: removed list padding for a tighter layout; increased icon/text sizes, adjusted spacing and chevron size for improved readability.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 22, 2025

Walkthrough

List 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

Cohort / File(s) Summary of changes
Contact list bottom sheets
lib/ui/contact_list/new_chat_bottom_sheet.dart, lib/ui/contact_list/new_group_chat_sheet.dart
Set ListView padding to EdgeInsets.zero. In NewChatTile: increased icon and chevron sizes, expanded horizontal gap, raised text font size and removed bold weight.
Developer settings UI restructure
lib/ui/settings/developer/developer_settings_screen.dart
Replaced inline header with WnAppBar (uses context.pop() on leading chevron); converted body into a padded, scrollable layout with a Key Package Management section — added Publish/Inspect/Delete buttons (with loading states), conditional “Key Packages (N)” header, item list or empty placeholder, per-item delete confirmation, and styling/layout adjustments.

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
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • codeswot
  • untreu2
  • josefinalliende

Poem

I nudge the lists to snugly flow,
Icons grown so they better show.
WnAppBar crowns the dev-screen bright,
Keys line up, prepared to flight.
Publish, peek, or sweep—hop light! 🥕🐇

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 and concisely summarizes the primary UI changes in the changeset—repairing the developer settings header and adjusting padding on the "start new chat" sheets—which directly maps to the developer_settings_screen header update and the ListView padding changes in the new chat sheet files; it is clear, relevant, and easy for teammates scanning history to understand.
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-dev-header-and-padding

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.

@erskingardner erskingardner force-pushed the fix-dev-header-and-padding branch from a2689d3 to 1c1d4a6 Compare September 22, 2025 20:11
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.

✅ LGTM!

@erskingardner erskingardner merged commit 8bb0cd8 into master Sep 22, 2025
1 of 2 checks passed
@erskingardner erskingardner deleted the fix-dev-header-and-padding branch September 22, 2025 20:17
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 numbers

The 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 entry

This 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 strings

Move “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

📥 Commits

Reviewing files that changed from the base of the PR and between c16ac6d and 1c1d4a6.

📒 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.dart
  • lib/ui/contact_list/new_group_chat_sheet.dart
  • lib/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 insets

Zero 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 padding

Setting padding to EdgeInsets.zero matches the new group sheet and keeps spacing predictable with the explicit item paddings/gaps.

Comment on lines +358 to +376
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),
),
);
},
),
),
),
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

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.

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.

3 participants