Standardised textfield size #454
Conversation
WalkthroughUI sizing and padding were standardized across multiple screens, notably aligning paste/copy buttons with text field heights. Profile keys screen was renamed and routed accordingly, with small-sized fields introduced via a new FieldSize API. Several icons were converted to SVG assets. Minor error handling and spacing tweaks were added elsewhere. Changes
Sequence Diagram(s)Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 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)
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
lib/ui/settings/profile_keys/profile_keys_screen.dart (1)
66-69: Leak: _publicKeyController is never disposedDispose both controllers to avoid leaks.
@override void dispose() { _privateKeyController.dispose(); + _publicKeyController.dispose(); super.dispose(); }
🧹 Nitpick comments (8)
lib/ui/contact_list/new_chat_bottom_sheet.dart (1)
397-397: Suffix icon padding tweak is fine; consider centralizing spacing tokensUsing EdgeInsets.only(right: 14.w) is consistent with the new spacing. Consider extracting 14.w into a shared spacing constant to avoid magic numbers and ease future tuning.
lib/ui/core/ui/wn_text_form_field.dart (2)
199-204: Nit: fix typo in comment“ConstainedBox” ➜ “ConstrainedBox”.
- // Without using ConstainedBox to enforce the target height. + // Without using ConstrainedBox to enforce the target height.
150-153: Optional: derive vertical padding from targetHeight to avoid magic numbers13.5.h and 19.5.h work, but consider computing from targetHeight to keep padding proportional if sizes change later.
lib/ui/settings/donate/donate_screen.dart (1)
109-111: Consider centralizing 56.h/20.w as design tokensReplace repeated “56.h” and “20.w” with named constants (e.g., AppDimens.iconButtonRegular, AppSpacing.xs) to avoid magic numbers and ease future tuning.
Also applies to: 142-144
lib/ui/contact_list/start_chat_bottom_sheet.dart (1)
95-109: Use the formatted error in the toast (and avoid confusing shadowed names)You format WhitenoiseError to a user-friendly string (
error) but still toast with$e. Use the formatted string for consistency and better UX. Also consider renaming the localerrorvariable toerrorMessageto avoid confusion with the named parametererror:inwhitenoiseErrorToString.Apply this minimal fix:
- String error; + String errorMessage; if (e is WhitenoiseError) { - error = await whitenoiseErrorToString(error: e); + errorMessage = await whitenoiseErrorToString(error: e); } else { - error = e.toString(); + errorMessage = e.toString(); } - _logger.warning('Failed to fetch key package: $error'); + _logger.warning('Failed to fetch key package: $errorMessage'); if (mounted) { setState(() { _isLoadingKeyPackage = false; _isLoadingKeyPackageError = true; - ref.showErrorToast('Error loading contact: $e'); + ref.showErrorToast('Error loading contact: $errorMessage'); }); }lib/ui/auth_flow/login_screen.dart (1)
211-221: Avoid magic numbers; derive from a single source of truthTo keep sizes consistent across screens, consider exposing a constant (e.g., Dimens.inputHeightRegular = 56.0) or a token from WnTextFormField (e.g., FieldSize.regular height) and reuse it here for both Container and CustomIconButton sizes/padding.
lib/ui/settings/network/add_relay_bottom_sheet.dart (1)
191-197: Centralize dimensions to avoid driftPrefer a shared dimension token (e.g., Dimens.inputHeightRegular, Dimens.iconButtonPaddingLg) so future changes propagate consistently across screens.
lib/ui/settings/profile_keys/profile_keys_screen.dart (1)
141-152: Replace magic numbers with shared dimension tokensConsider defining shared constants (e.g., Dimens.iconButtonHeightSm = 44.0, Dimens.iconButtonPaddingSm = 14.0) or exposing sizes via theme to ensure consistent usage across screens.
Also applies to: 227-248
📜 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 (10)
CHANGELOG.md(1 hunks)lib/routing/router_provider.dart(2 hunks)lib/ui/auth_flow/login_screen.dart(1 hunks)lib/ui/chat/widgets/chat_input.dart(1 hunks)lib/ui/contact_list/new_chat_bottom_sheet.dart(1 hunks)lib/ui/contact_list/start_chat_bottom_sheet.dart(2 hunks)lib/ui/core/ui/wn_text_form_field.dart(8 hunks)lib/ui/settings/donate/donate_screen.dart(2 hunks)lib/ui/settings/network/add_relay_bottom_sheet.dart(1 hunks)lib/ui/settings/profile_keys/profile_keys_screen.dart(5 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 '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/chat/widgets/chat_input.dartlib/ui/contact_list/new_chat_bottom_sheet.dartlib/ui/auth_flow/login_screen.dartlib/routing/router_provider.dartlib/ui/settings/network/add_relay_bottom_sheet.dartlib/ui/settings/donate/donate_screen.dartlib/ui/contact_list/start_chat_bottom_sheet.dartlib/ui/settings/profile_keys/profile_keys_screen.dartlib/ui/core/ui/wn_text_form_field.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 (16)
📚 Learning: 2025-08-08T13:39:00.467Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.467Z
Learning: Applies to **/*.dart : Avoid nesting widgets deeply in Flutter. Break down large widgets into smaller, focused widgets. A flatter widget structure improves efficiency, readability, maintainability, and performance.
Applied to files:
lib/ui/chat/widgets/chat_input.dartlib/ui/contact_list/new_chat_bottom_sheet.dartlib/routing/router_provider.dartlib/ui/settings/donate/donate_screen.dartlib/ui/core/ui/wn_text_form_field.dart
📚 Learning: 2025-08-08T13:39:00.467Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.467Z
Learning: Applies to **/*.dart : Utilize const constructors wherever possible to reduce rebuilds.
Applied to files:
lib/ui/contact_list/new_chat_bottom_sheet.dart
📚 Learning: 2025-08-08T13:39:00.467Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.467Z
Learning: Applies to **/*.dart : Use UPPERCASE for environment variables. Avoid magic numbers and define constants.
Applied to files:
lib/ui/auth_flow/login_screen.dartlib/ui/settings/donate/donate_screen.dartlib/ui/core/ui/wn_text_form_field.dart
📚 Learning: 2025-08-08T13:39:00.467Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.467Z
Learning: Applies to **/*.dart : Use AutoRoute to manage routes. Use extras to pass data between pages.
Applied to files:
lib/routing/router_provider.dartlib/ui/contact_list/start_chat_bottom_sheet.dart
📚 Learning: 2025-08-08T13:39:00.467Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.467Z
Learning: Applies to **/*.dart : Use freezed to manage UI states.
Applied to files:
lib/routing/router_provider.dartlib/ui/contact_list/start_chat_bottom_sheet.dartlib/ui/settings/profile_keys/profile_keys_screen.dart
📚 Learning: 2025-08-08T13:39:00.467Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.467Z
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/routing/router_provider.dartlib/ui/contact_list/start_chat_bottom_sheet.dart
📚 Learning: 2025-08-08T13:39:00.467Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.467Z
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/routing/router_provider.dartlib/ui/contact_list/start_chat_bottom_sheet.dart
📚 Learning: 2025-08-08T13:39:00.467Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.467Z
Learning: Applies to **/*_test.dart : Use the standard widget testing for Flutter.
Applied to files:
lib/routing/router_provider.dartlib/ui/settings/profile_keys/profile_keys_screen.dart
📚 Learning: 2025-08-08T13:39:00.467Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.467Z
Learning: Applies to **/*.dart : Use ThemeData to manage themes.
Applied to files:
lib/routing/router_provider.dartlib/ui/settings/profile_keys/profile_keys_screen.dart
📚 Learning: 2025-08-08T13:39:00.467Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.467Z
Learning: Applies to **/*.dart : Don't leave blank lines within a function.
Applied to files:
lib/ui/settings/donate/donate_screen.dart
📚 Learning: 2025-08-08T13:39:00.467Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.467Z
Learning: Applies to **/*.dart : Use flutter_rust_bridge to access core functionality of the app.
Applied to files:
lib/ui/contact_list/start_chat_bottom_sheet.dart
📚 Learning: 2025-08-08T13:39:00.467Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.467Z
Learning: Applies to **/*.dart : Use extensions to manage reusable code.
Applied to files:
lib/ui/contact_list/start_chat_bottom_sheet.dartlib/ui/settings/profile_keys/profile_keys_screen.dart
📚 Learning: 2025-08-08T13:39:00.467Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.467Z
Learning: Applies to **/*.dart : Declare interfaces to define contracts.
Applied to files:
lib/ui/contact_list/start_chat_bottom_sheet.dart
📚 Learning: 2025-08-08T13:39:00.467Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.467Z
Learning: Applies to **/*.dart : Use getIt to manage dependencies. Use singleton for services and repositories. Use factory for use cases. Use lazy singleton for controllers.
Applied to files:
lib/ui/contact_list/start_chat_bottom_sheet.dart
📚 Learning: 2025-08-08T13:39:00.467Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.467Z
Learning: Applies to **/*.dart : Use AppLocalizations to manage translations.
Applied to files:
lib/ui/contact_list/start_chat_bottom_sheet.dart
📚 Learning: 2025-08-08T13:39:00.467Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.467Z
Learning: Applies to **/*.dart : Use constants to manage constants values.
Applied to files:
lib/ui/contact_list/start_chat_bottom_sheet.dartlib/ui/core/ui/wn_text_form_field.dart
🔇 Additional comments (16)
lib/ui/chat/widgets/chat_input.dart (2)
159-159: Spacing tweak looks good and consistent with PR goalsReducing the pre-button gap to 4.w aligns with the standardized tighter spacing elsewhere. No functional impact.
159-159: Double-check minimum tap target in tight layoutsWith a smaller gap, ensure the send button still maintains at least a 48dp tap target across devices and densities.
lib/ui/contact_list/new_chat_bottom_sheet.dart (1)
386-393: Prefix icon padding and sizing change LGTMSwitching to padded prefix and 20.w icon size improves alignment and matches the standardized spacing approach.
lib/ui/core/ui/wn_text_form_field.dart (2)
12-15: New FieldSize enum is clear and aligned with design tokensDistinct small (44.h) and regular (56.h) variants are straightforward and help standardize field heights.
50-51: Adding a size property to WnTextFormField is a solid extension pointDefaulting to regular avoids breaking changes and enables consistent sizing across the app.
Also applies to: 84-85
CHANGELOG.md (1)
25-25: Changelog entry reads well and matches the PR scopeAccurately reflects the standardized textfield and button sizing changes.
lib/ui/settings/donate/donate_screen.dart (2)
105-105: Copy button spacing/size update LGTMReducing gap to 4.w and using 56.h aligns with standardized button sizing.
Also applies to: 109-111
134-134: Second row matches the new spacing and size — consistentGood consistency across both address rows.
Also applies to: 142-144
lib/ui/contact_list/start_chat_bottom_sheet.dart (1)
14-16: Imports align with FRB usage and error formatting utilitiesBringing in
api.dart(for WhitenoiseError) andutils.dart(forwhitenoiseErrorToString) is appropriate here. LGTM.lib/ui/auth_flow/login_screen.dart (1)
211-221: Paste button sizing aligns with field height and tap-target guidanceIncreasing to 56.h and padding to 20.w improves usability and visual alignment with the text field. Good change.
lib/ui/settings/network/add_relay_bottom_sheet.dart (1)
191-197: Standardized spacing and button size: LGTMReducing the gap to 4.w and setting the paste button to 56.h with 20.w padding aligns with the new text field sizing and improves consistency.
lib/routing/router_provider.dart (1)
23-23: Route and import updated to ProfileKeysScreenThe import and route builder correctly reflect the screen rename. No behavioral changes; navigation remains intact.
Also applies to: 175-176
lib/ui/settings/profile_keys/profile_keys_screen.dart (4)
17-22: Rename to ProfileKeysScreen is clear and consistentClass and state names now reflect screen purpose; matches router changes.
34-35: Formatting public key only for display is correctUsing
formatPublicKey()when setting the controller improves readability while leaving the copied value (from provider) untouched. Good separation of display vs. copy value.
141-152: Small field size + compact copy button aligns with new size systemSetting
size: FieldSize.small, reducing gap to 4.w, and using a 44.h icon button keeps the row compact while maintaining a 44dp tap target.
227-248: Consistent sizing applied to private key rowSame compact pattern applied; looks consistent and accessible.
erskingardner
left a comment
erskingardner
left a comment
There was a problem hiding this comment.
Looks good but I'd say to handle the code rabbit suggestions
…eactive error icon
Quwaysim
requested review from
codeswot,
erskingardner,
josefinalliende and
untreu2
josefinalliende
left a comment
josefinalliende
left a comment
There was a problem hiding this comment.
Tested it locally and looks good in profile keys screen and login! Left a few comments
| } catch (e) { | ||
| _logger.warning('Failed to fetch key package: $e'); | ||
| String error; | ||
| if (e is WhitenoiseError) { |
There was a problem hiding this comment.
I think this is key package service responsibility... it already has a handleFetchKeyPackageError method which could be extended.
There was a problem hiding this comment.
Didn't know this exists, I'll check it out. Thanks.
There was a problem hiding this comment.
Did it make sense to move it there?
There was a problem hiding this comment.
Not really. I refactored it and tried to mock the error to make sure I didn't break anything but it's quite tricky. I think I'll just remove it for now and do the refactor if the issue shows up later. It's kinda out of scope for this PR (it's just a side fix) and I don't want to have it delaying this fix. @josefinalliende
There was a problem hiding this comment.
Ok, so this if checking the type of error is going to be removed in this PR because it can’t be actually reproduced?
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
lib/ui/core/ui/wn_text_form_field.dart (2)
111-118: LGTM! Properly reactive suffix icon implementation.The
ValueListenableBuilderensures the suffix icon updates reactively when validation state changes, addressing the previous review concern about reactivity.
139-142: LGTM! Correct multiline detection logic.The multiline detection properly handles all cases (maxLines > 1, minLines > 1, and expands) with appropriate default value handling, fixing the previous issue where null checks would never trigger.
🧹 Nitpick comments (1)
lib/ui/core/ui/wn_text_form_field.dart (1)
156-158: Consider extracting padding calculations for clarity.The vertical padding calculation could be clearer if extracted into named variables, improving maintainability.
- contentPadding: EdgeInsets.symmetric(horizontal: 12.w, vertical: isSmall ? 13.5.h : 19.5.h), + contentPadding: EdgeInsets.symmetric( + horizontal: 12.w, + vertical: isSmall ? 13.5.h : 19.5.h, + ),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/ui/chat/widgets/chat_input.dart(2 hunks)lib/ui/core/ui/wn_text_form_field.dart(9 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/ui/chat/widgets/chat_input.dart
🧰 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/core/ui/wn_text_form_field.dart
🧠 Learnings (1)
📚 Learning: 2025-08-08T13:39:00.467Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.467Z
Learning: Applies to **/*.dart : Use ThemeData to manage themes.
Applied to files:
lib/ui/core/ui/wn_text_form_field.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/core/ui/wn_text_form_field.dart (2)
13-16: LGTM! Clean enum implementation for field sizing.The
FieldSizeenum with clear height documentation provides a maintainable way to standardize text field sizes across the app, directly addressing the PR objective of fixing height mismatches.
205-217: LGTM! Well-documented height enforcement logic.The dual-level height enforcement (decoration constraints + parent ConstrainedBox) with clear explanatory comments ensures consistent sizing for both the field content and its container. This properly addresses the standardization objective.
josefinalliende
left a comment
josefinalliende
left a comment
There was a problem hiding this comment.
Reviewed it again! Left a small question about the borders changes haha... my eyes can tell the difference so I need some help 👁️
| width: 1.w, | ||
| ), | ||
| border: | ||
| isReplying |
There was a problem hiding this comment.
So, I have a question here, cause my eyes are not detecting the difference between master en this branch locally.... 🙈 where is this border difference? I got some screenshots form different cases, in ios emulator.
Replies cases
| Case | master branch |
PR branch |
|---|---|---|
| Reply input unfocused |  |
 |
| Reply input focused without content |  |
 |
| Reply input focused with content |  |
 |
"Normal" messages cases (without replies)
| Case | master branch |
PR branch |
|---|---|---|
| Message input unfocused |  |
 |
| Message input focused without content |  |
 |
| Message input focused with content |  |
 |
There was a problem hiding this comment.
Message input focused with content, that's where the issue is. It's pretty tiny, like a pixel or two but it's pretty noticeable too.
There was a problem hiding this comment.
But there the fix is the distance between the input and button right? What it has to do with the border changes? Or there are no border changes really?
| } catch (e) { | ||
| _logger.warning('Failed to fetch key package: $e'); | ||
| String error; | ||
| if (e is WhitenoiseError) { |
There was a problem hiding this comment.
Did it make sense to move it there?
lib/ui/chat/widgets/chat_input.dart
Outdated
| WnIconButton( | ||
| onPressed: _sendMessage, | ||
| icon: Icons.arrow_upward, | ||
| icon: CarbonIcons.arrow_up, |
There was a problem hiding this comment.
I'm really sorry about this comment, because it was me who made the question initially, but in another PR with @codeswot codeswot we came to the conclusion that whe should not use carbon icons cause we are importing a lot of icons (the whole library) and that using svgs would be actually better. Sorry for the confusion 😵
There was a problem hiding this comment.
Ah but anyway it as still material icons, so I don't think its worth reverting it. It is going to change anyway to a svg with the new issue :)
| } catch (e) { | ||
| _logger.warning('Failed to fetch key package: $e'); | ||
| String error; | ||
| if (e is WhitenoiseError) { |
There was a problem hiding this comment.
Ok, so this if checking the type of error is going to be removed in this PR because it can’t be actually reproduced?
josefinalliende
left a comment
josefinalliende
left a comment
There was a problem hiding this comment.
I left some doubts, but approved anyway ✅
Quwaysim
dismissed stale reviews from josefinalliende and codeswot
via
081e9a3
Quwaysim
dismissed
erskingardner’s stale review
updated PR, concerns addressed
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
lib/ui/settings/profile_keys/profile_keys_screen.dart (1)
65-68: Dispose_publicKeyControllerto prevent a memory leak
_publicKeyControlleris created but never disposed. Add it todispose()alongside_privateKeyController.Apply this diff:
@override void dispose() { _privateKeyController.dispose(); + _publicKeyController.dispose(); super.dispose(); }
🧹 Nitpick comments (2)
lib/ui/contact_list/new_chat_bottom_sheet.dart (1)
398-401: Verify vertical centering and tap target after suffix padding changeSwitching from uniform padding to right-only may shift the scan icon visually. Ensure it remains vertically centered and preserves a ≥48 logical px tap target across sizes. If you notice any misalignment in smaller fields, consider symmetric vertical padding.
Optional tweak if needed:
- padding: EdgeInsets.only(right: 14.w), + padding: EdgeInsets.symmetric(vertical: 12.h, horizontal: 14.w),lib/ui/settings/profile_keys/profile_keys_screen.dart (1)
33-35: Good: display-friendly public key formattingUsing
formatPublicKey()for the shown value improves readability. Sincenpubcan be null (as implied by the?.), consider mirroring the null-guard used for the private key in the copy handler to avoid copying a null or crashing ifClipboardUtilsexpects a non-null String.Apply this diff to
_copyPublicKey():void _copyPublicKey() { - final npub = ref.read(nostrKeysProvider).npub; - ClipboardUtils.copyWithToast( - ref: ref, - textToCopy: npub, - successMessage: 'Public key copied to clipboard', - ); + final String? npub = ref.read(nostrKeysProvider).npub; + if (npub != null && npub.isNotEmpty) { + ClipboardUtils.copyWithToast( + ref: ref, + textToCopy: npub, + successMessage: 'Public key copied to clipboard', + ); + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
CHANGELOG.md(1 hunks)lib/ui/contact_list/new_chat_bottom_sheet.dart(1 hunks)lib/ui/settings/donate/donate_screen.dart(2 hunks)lib/ui/settings/network/add_relay_bottom_sheet.dart(1 hunks)lib/ui/settings/profile_keys/profile_keys_screen.dart(5 hunks)
✅ Files skipped from review due to trivial changes (1)
- CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/ui/settings/network/add_relay_bottom_sheet.dart
🧰 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/donate/donate_screen.dartlib/ui/settings/profile_keys/profile_keys_screen.dartlib/ui/contact_list/new_chat_bottom_sheet.dart
🧠 Learnings (1)
📚 Learning: 2025-08-12T11:21:53.640Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#455
File: lib/ui/settings/profile/switch_profile_bottom_sheet.dart:0-0
Timestamp: 2025-08-12T11:21:53.640Z
Learning: In the whitenoise_flutter codebase, ContactModel.publicKey can be stored in either npub format (from metadata cache) or hex format (from account storage). The activeAccountProvider may return either format depending on how the account was originally stored, so normalization to hex format is required when comparing with other hex-normalized keys in sorting logic.
Applied to files:
lib/ui/settings/profile_keys/profile_keys_screen.dart
🔇 Additional comments (4)
lib/ui/settings/donate/donate_screen.dart (1)
109-115: LGTM: spacing and button sizing now align with standardized 56.h controlsReducing the gap to 4.w and using a height-based size (56.h) for the copy button matches the new textfield height standardization.
Also applies to: 138-148
lib/ui/settings/profile_keys/profile_keys_screen.dart (3)
16-21: Screen rename and state type update look goodRenaming to ProfileKeysScreen and updating the ConsumerState type keep conventions consistent and are non-breaking.
144-145: LGTM: compact public key field and copy buttonUsing
FieldSize.small, a tighter gap, and a 44.h button/padding matches the small variant spec and improves alignment.Also applies to: 147-153
230-231: LGTM: private key small field and compact copy buttonThe small field size with visibility toggle plus the 44.h copy button and reduced gap are consistent with the standardized sizing.
Also applies to: 244-250
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (4)
lib/ui/core/ui/wn_text_form_field.dart (4)
119-126: Type the validator correctly and use a verb-based, private handlerMatch TextFormField’s signature and follow naming guidelines. This also avoids confusion with the widget.validator property.
Apply this diff:
- String? validator(dynamic value) { - final result = widget.validator?.call(value); + String? _validate(String? value) { + final String? result = widget.validator?.call(value); hasError.value = result != null; context.dispatchNotification( WnValidationNotification(hashCode, !hasError.value), ); return result; }And wire it in:
- validator: validator, + validator: _validate,Also applies to: 175-175
173-185: Prevent assertion by nulling min/maxLines when expands is trueTextFormField asserts if expands is true while minLines/maxLines are non-null. Defaults here are 1, so this will trip assertions for any expands usage.
Apply this diff:
maxLength: widget.maxLength, - maxLines: widget.maxLines, - minLines: widget.minLines, + maxLines: widget.expands ? null : widget.maxLines, + minLines: widget.expands ? null : widget.minLines,
186-187: Avoid passing initialValue when controller is providedTextFormField asserts if both controller and initialValue are non-null.
Apply this diff:
- initialValue: widget.initialValue, + initialValue: widget.controller == null ? widget.initialValue : null,
90-101: Dispose the ValueNotifier to avoid leakshasError is a ValueNotifier and should be disposed.
Apply this diff:
@override void initState() { super.initState(); @@ isObscuringText = widget.obscureText ?? widget.type == FieldType.password; } bool isObscuringText = true; void toggleIsObscuringText() { setState(() { isObscuringText = !isObscuringText; }); } + + @override + void dispose() { + hasError.dispose(); + super.dispose(); + }Also applies to: 103-109
🧹 Nitpick comments (3)
lib/ui/core/ui/wn_text_form_field.dart (3)
110-117: Use error color and avoid param shadowing in suffix icon builder
- Show the error icon in a destructive/error color for better UX and consistency with theme.
- Avoid shadowing the hasError field with the builder param name.
Apply this diff:
- Widget get suffixIcon => ValueListenableBuilder<bool>( - valueListenable: hasError, - builder: - (_, hasError, _) => - hasError - ? const Icon(Icons.error) - : (widget.decoration?.suffixIcon ?? const SizedBox.shrink()), - ); + Widget get suffixIcon => ValueListenableBuilder<bool>( + valueListenable: hasError, + builder: (_, hasErr, __) => hasErr + ? Icon(Icons.error, color: context.colors.destructive) + : (widget.decoration?.suffixIcon ?? const SizedBox.shrink()), + );
138-142: Extract heights/paddings to named constants (avoid magic numbers)Optional, but improves readability and consistency with “Use constants to manage constants values.” It also centralizes size adjustments.
You can define design constants and use them with ScreenUtil:
// e.g., at top-level (not const because we apply .h/.w) class _FieldMetrics { static const double baseHeightRegular = 56.0; static const double baseHeightSmall = 44.0; static const double baseHPad = 12.0; static const double baseVPadSmall = 13.5; static const double baseVPadRegular = 19.5; }Then in build:
final double targetHeight = (isSmall ? _FieldMetrics.baseHeightSmall : _FieldMetrics.baseHeightRegular).h; contentPadding: EdgeInsets.symmetric( horizontal: _FieldMetrics.baseHPad.w, vertical: (isSmall ? _FieldMetrics.baseVPadSmall : _FieldMetrics.baseVPadRegular).h, ),Also applies to: 155-157
53-85: Make type non-nullable with a default (optional)FieldType is conceptually non-null here (you provide a default). Consider
final FieldType type;to simplify downstream checks. Given “no breaking changes,” defer if external code passes null.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
lib/ui/chat/widgets/chat_input.dart(4 hunks)lib/ui/core/ui/wn_text_form_field.dart(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/ui/chat/widgets/chat_input.dart
🧰 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/core/ui/wn_text_form_field.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 (3)
lib/ui/core/ui/wn_text_form_field.dart (3)
12-15: Nice: Public FieldSize API aligns with design intentClear, minimal, and directly supports the PR’s objective to standardize regular/small heights.
128-137: Initial validation vs autovalidate modeYou eagerly validate in initState and dispatch a notification, which may contradict AutovalidateMode.onUserInteraction UX. If the initial validity signal is needed for enabling actions (e.g., Save), keep it; otherwise consider gating it behind AutovalidateMode.always.
Would you like this to adhere strictly to onUserInteraction (no initial validation), or do you rely on the initial validity notification for enabling controls?
Also applies to: 119-126
173-202: No WnTextFormField call sets expands: trueI scanned every
WnTextFormFieldinstantiation across the repo—none explicitly useexpands: true. Given the widget’s defaults (expands = false,maxLines = 1,minLines = null), there are no current runtime‐assert risks around expansion.
josefinalliende
left a comment
josefinalliende
left a comment
There was a problem hiding this comment.
✅ LGTM! The only comment I have now is that I had assumed this one would be merged before #487. Since that’s not the case, I think it would be better to replace Icons.arrow_upward and Icons.error with the SVGs from assets
|
Fixing. Next time though, I'll prefer that we treat out of scope issues in a separate PR (probably with separate GH issue too). |
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
lib/ui/settings/profile_keys/profile_keys_screen.dart (1)
65-68: Also dispose the publicKey controller
_publicKeyControlleris created but not disposed, which can leak resources.Apply this diff:
@override void dispose() { _privateKeyController.dispose(); + _publicKeyController.dispose(); super.dispose(); }
🧹 Nitpick comments (6)
lib/ui/chat/widgets/stacked_images.dart (1)
124-132: Center the error SVG for better visual balanceAs-is, the SVG will align top-left within the error container. Centering improves alignment and matches typical placeholders.
Apply this diff:
- child: SvgPicture.asset( - AssetsPaths.icErrorFilled, - colorFilter: ColorFilter.mode( - context.colors.destructive, - BlendMode.srcIn, - ), - width: 24.w, - height: 24.w, - ), + child: Center( + child: SvgPicture.asset( + AssetsPaths.icErrorFilled, + colorFilter: ColorFilter.mode( + context.colors.destructive, + BlendMode.srcIn, + ), + width: 24.w, + height: 24.w, + semanticsLabel: 'Image failed to load', + ), + ),lib/ui/settings/network/widgets/network_section.dart (1)
99-107: Good switch to themed SVG error iconUsing
AssetsPaths.icErrorFilledwith a colorFilter and aligning error text color tocontext.colors.destructiveis consistent and theme-friendly.Optional: consider replacing the hard-coded red background/border (Lines 93-96) with themed values to fully eliminate magic colors:
Example:
decoration: BoxDecoration( color: context.colors.destructive.withValues(alpha: 0.08), borderRadius: BorderRadius.circular(8.r), border: Border.all(color: context.colors.destructive.withValues(alpha: 0.3)), ),Also applies to: 114-114
lib/ui/core/ui/wn_text_form_field.dart (4)
112-119: Tint and size the error SVG; use builder context for theme accessTo match the rest of the PR, tint the error icon with the theme’s destructive color and set a consistent size. Also, use the builder’s
contextforcontext.colors.Apply this diff:
- Widget get suffixIcon => ValueListenableBuilder<bool>( - valueListenable: hasError, - builder: - (_, hasError, _) => - hasError - ? SvgPicture.asset(AssetsPaths.icErrorFilled) - : (widget.decoration?.suffixIcon ?? const SizedBox.shrink()), - ); + Widget get suffixIcon => ValueListenableBuilder<bool>( + valueListenable: hasError, + builder: (context, hasErr, __) => hasErr + ? SvgPicture.asset( + AssetsPaths.icErrorFilled, + width: 20.w, + height: 20.w, + colorFilter: ColorFilter.mode( + context.colors.destructive, + BlendMode.srcIn, + ), + ) + : (widget.decoration?.suffixIcon ?? const SizedBox.shrink()), + );
140-144: Prefer explicit local types for clarity (minor)Follow the codebase guideline to explicitly type locals.
Apply this diff:
- final isSmall = widget.size == FieldSize.small; - final targetHeight = isSmall ? 44.h : 56.h; + final bool isSmall = widget.size == FieldSize.small; + final double targetHeight = isSmall ? 44.h : 56.h;
93-106: Dispose the ValueNotifier to prevent leaks (recommended)
hasErroris aValueNotifier; dispose it indispose()to detach listeners cleanly.Add this method to the State class:
@override void dispose() { hasError.dispose(); super.dispose(); }
175-204: Optional: Safeguard TextFormField against Flutter’s expands assertionNo occurrences of
expands: truewere found in the repo. To future-proof this widget and avoid the built-in Flutter assertion (which requires bothmaxLinesandminLinesto be null whenexpandsis true), you can optionally apply the following change inlib/ui/core/ui/wn_text_form_field.dart:- maxLines: widget.maxLines, - minLines: widget.minLines, + maxLines: widget.expands ? null : widget.maxLines, + minLines: widget.expands ? null : widget.minLines,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
lib/ui/chat/widgets/stacked_images.dart(2 hunks)lib/ui/core/ui/wn_text_form_field.dart(9 hunks)lib/ui/settings/network/widgets/network_section.dart(1 hunks)lib/ui/settings/profile_keys/profile_keys_screen.dart(6 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/chat/widgets/stacked_images.dartlib/ui/settings/network/widgets/network_section.dartlib/ui/settings/profile_keys/profile_keys_screen.dartlib/ui/core/ui/wn_text_form_field.dart
🧠 Learnings (2)
📚 Learning: 2025-08-12T11:21:53.640Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#455
File: lib/ui/settings/profile/switch_profile_bottom_sheet.dart:0-0
Timestamp: 2025-08-12T11:21:53.640Z
Learning: In the whitenoise_flutter codebase, ContactModel.publicKey can be stored in either npub format (from metadata cache) or hex format (from account storage). The activeAccountProvider may return either format depending on how the account was originally stored, so normalization to hex format is required when comparing with other hex-normalized keys in sorting logic.
Applied to files:
lib/ui/settings/profile_keys/profile_keys_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 ThemeData to manage themes.
Applied to files:
lib/ui/core/ui/wn_text_form_field.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 (6)
lib/ui/core/ui/wn_text_form_field.dart (2)
14-17: FieldSize enum and size plumbing look solidThe enum addition and public
sizeparameter (defaulting to regular) read well and align with the standard heights. No issues spotted.Also applies to: 52-52, 86-86
146-146: Height standardization logic is correct and cohesive
- Using
isMultilineto gate both decoration constraints and the outer ConstrainedBox is the right approach.- Prefix/suffix icon constraints tied to the target height ensure visual alignment.
Also applies to: 157-160, 206-218
lib/ui/settings/profile_keys/profile_keys_screen.dart (4)
33-35: Good: displaying formatted npubUsing
formatPublicKey()improves readability in the field. LGTM.
145-145: Adoption of FieldSize.small and aligned button sizing looks correct
- Fields set to
FieldSize.smalland buttons sized to 44 with 14 padding match the standardized small height.- Spacing reduced to 4.w aligns the row without crowding.
Also applies to: 235-235, 151-153, 252-254
199-212: Consistent themed SVG for error stateSwitching to
icErrorFilledwith a theme-based tint is consistent with the rest of the app.
16-21: Class rename verified and router updated
- No occurrences of
NostrKeysScreenor_NostrKeysScreenStateremain.ProfileKeysScreenis correctly referenced in GoRouter (lib/routing/router_provider.dart:175).No further action required.
| import 'package:flutter_svg/svg.dart'; | ||
| import 'package:whitenoise/ui/core/themes/assets.dart'; |
There was a problem hiding this comment.
Use the canonical flutter_svg import to avoid build errors
This file imports package:flutter_svg/svg.dart, while the rest of the codebase uses package:flutter_svg/flutter_svg.dart. The non-canonical path can break depending on flutter_svg version. Standardize to the canonical import.
Apply this diff:
-import 'package:flutter_svg/svg.dart';
+import 'package:flutter_svg/flutter_svg.dart';📝 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.
| import 'package:flutter_svg/svg.dart'; | |
| import 'package:whitenoise/ui/core/themes/assets.dart'; | |
| import 'package:flutter_svg/flutter_svg.dart'; | |
| import 'package:whitenoise/ui/core/themes/assets.dart'; |
🤖 Prompt for AI Agents
In lib/ui/chat/widgets/stacked_images.dart around lines 6 to 7, the file imports
flutter_svg using the non-canonical path `package:flutter_svg/svg.dart`; update
that import to the canonical `package:flutter_svg/flutter_svg.dart` (leave the
other import as-is) so it matches the rest of the codebase and avoids
build/version errors, then run the analyzer/build to verify no import errors
remain.
| import 'package:flutter_svg/svg.dart'; | ||
| import 'package:whitenoise/ui/core/themes/assets.dart'; |
There was a problem hiding this comment.
Use the canonical flutter_svg import to avoid build errors
This file uses package:flutter_svg/svg.dart. Please standardize to package:flutter_svg/flutter_svg.dart like the other files.
Apply this diff:
-import 'package:flutter_svg/svg.dart';
+import 'package:flutter_svg/flutter_svg.dart';📝 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.
| import 'package:flutter_svg/svg.dart'; | |
| import 'package:whitenoise/ui/core/themes/assets.dart'; | |
| import 'package:flutter_svg/flutter_svg.dart'; | |
| import 'package:whitenoise/ui/core/themes/assets.dart'; |
🤖 Prompt for AI Agents
In lib/ui/core/ui/wn_text_form_field.dart around lines 4 to 5, the flutter_svg
import uses the non-canonical path "package:flutter_svg/svg.dart"; change it to
the standard canonical import "package:flutter_svg/flutter_svg.dart" to match
other files and avoid build errors, updating only the import line.
You are absolutely right, my bad 💔 ... thanks for the patience! |
josefinalliende
left a comment
josefinalliende
left a comment
There was a problem hiding this comment.
Re-approved ✅ 🎉 Thanks for the feedback, noted!
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
lib/ui/core/ui/wn_icon_button.dart (1)
31-50: Icon is not centered inside the button. Add centering to avoid top-left bias.Without alignment or a Center widget, the SVG (20) will render at the top-left inside a 52 box.
Apply this minimal fix:
decoration: BoxDecoration( color: backgroundColor ?? context.colors.avatarSurface, border: hasBorder ? Border.all( color: borderColor ?? context.colors.input, ) : null, ), + alignment: Alignment.center, child: SvgPicture.asset( iconPath, height: iconSize.w, width: iconSize.w, colorFilter: ColorFilter.mode( iconColor ?? context.colors.primary, BlendMode.srcIn, ), ),
🧹 Nitpick comments (1)
lib/ui/core/ui/wn_icon_button.dart (1)
42-49: Sizing and accessibility improvements (double-scaling check passed)We searched all
WnIconButtoncall sites for.w/.h-scalediconSizevalues—none were found, so there’s no risk of double-scaling today.Recommended optional refactors:
- Add an optional
tooltipparameter and pass asemanticsLabelintoSvgPicture.assetfor screen-reader support.- Wrap the tappable container in a
Materialand useInk(sharing your decoration) so the ink splash is clipped to the button’s shape.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
assets/svgs/ic_arrow_up.svgis excluded by!**/*.svg
📒 Files selected for processing (3)
lib/ui/chat/widgets/chat_input.dart(5 hunks)lib/ui/core/themes/assets.dart(1 hunks)lib/ui/core/ui/wn_icon_button.dart(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/ui/chat/widgets/chat_input.dart
🧰 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/core/themes/assets.dartlib/ui/core/ui/wn_icon_button.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 (3)
lib/ui/core/themes/assets.dart (1)
72-72: Asset path and pubspec validated
- assets/svgs/ic_arrow_up.svg is present in the repository
- pubspec.yaml declares the entire
assets/svgs/directory underflutter: assets:No further changes needed—this is good to merge.
lib/ui/core/ui/wn_icon_button.dart (2)
3-3: Verified flutter_svg Dependency
pubspec.yaml already declaresflutter_svg: ^2.1.0, so the new import is safe and no build failures will occur.
10-11: All WnIconButton call sites updated to useiconPath; no legacyicon:parameters remain.
No further changes required.
josefinalliende
left a comment
josefinalliende
left a comment
There was a problem hiding this comment.
Oh no 😓 Something os broken now
|
Just remove the last commit and I can approve, I can add a new issue for the last changes so that this PR can be merged |
This reverts commit 65345f7.
Description
Fixes #445
Type of Change
Fixed and standardised textfield sizes. We now have regular and small (consistent with the design).
Checklist
just precommitto ensure that formatting and linting are correctCHANGELOG.mdfile with your changesSummary by CodeRabbit
Bug Fixes
Style
Documentation