-
Notifications
You must be signed in to change notification settings - Fork 14
patch: updated WnFilledButton to have prefixIcon and suffixIcon inste… #509
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
…ad of misleading and hacky params.
WalkthroughConsolidated WnFilledButton API: renamed title→label, removed .child/.icon ctors, added prefixIcon/suffixIcon/labelTextStyle, and migrated many call sites from Row-based children to label + optional prefix/suffix icons; visual/layout behavior preserved except for a few loading-to-disabled adjustments. Changes
Sequence Diagram(s)(omitted — changes are API/markup refactors without new control-flow interactions) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
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 ignored due to path filters (1)
📒 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 (
|
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
🔭 Outside diff range comments (5)
lib/ui/chat/chat_info/widgets/member_action_buttons.dart (1)
111-118: Don’t use ref.watch in event handlers; use ref.read inside _toggleContactref.watch should be used in build; calling it inside _toggleContact risks assertion failures and unintended rebuild coupling. Compute the contact status with ref.read in the handler.
Update _toggleContact to avoid watch:
Future<void> _toggleContact() async { setState(() => _isAddingContact = true); try { final contactsNotifier = ref.read(contactsProvider.notifier); // Use read here to avoid watch outside build: final contactsState = ref.read(contactsProvider); final contacts = contactsState.contactModels ?? []; final bool isCurrentlyContact = contacts.any( (c) => c.publicKey.toLowerCase() == widget.user.publicKey.toLowerCase(), ); if (isCurrentlyContact) { await contactsNotifier.removeContactByHex(widget.user.publicKey); if (mounted) ref.showSuccessToast('${widget.user.displayName} removed from contacts'); } else { await contactsNotifier.addContactByHex(widget.user.publicKey); if (mounted) ref.showSuccessToast('${widget.user.displayName} added to contacts'); } } catch (e) { if (mounted) ref.showErrorToast('Failed to update contact: $e'); } finally { if (mounted) setState(() => _isAddingContact = false); } }Keep _isContact() with ref.watch for build-time reactivity; use ref.read in actions.
Also applies to: 120-151
lib/ui/settings/general_settings_screen.dart (1)
247-249: Logic bug: multiple accounts check is off by one“Multiple accounts” should be > 1, not > 2. With exactly two accounts, this path won’t execute as intended.
Apply this fix:
- final hasMultipleAccounts = accounts.length > 2; + final hasMultipleAccounts = accounts.length > 1;lib/ui/contact_list/start_chat_bottom_sheet.dart (1)
152-160: Avoid ref.watch inside _isContact when called from actionsref.watch should be confined to build. Here _isContact() is also used from _toggleContact (an event handler). Use ref.read to get a stable snapshot in the handler.
Either:
- Keep _isContact() with ref.watch for build usage only, and in _toggleContact compute using ref.read (preferred), or
- Add a separate helper using ref.read and call that in _toggleContact.
Example adjustment for _toggleContact:
final contactsState = ref.read(contactsProvider); final contacts = contactsState.contactModels ?? []; final bool isCurrentlyContact = contacts.any( (c) => c.publicKey.toLowerCase() == widget.contact.publicKey.toLowerCase(), );Also applies to: 167-171
lib/ui/settings/app_settings/app_settings_screen.dart (1)
37-37: Typo in dialog title"Delete app app data" has a duplicated word.
Apply this diff:
- title: 'Delete app app data', + title: 'Delete app data',lib/ui/core/ui/wn_filled_button.dart (1)
4-14: Preserve WnButton API: forward style and onLongPressCurrently, callers can’t customize style or onLongPress through WnFilledButton. Forward these to avoid breaking the base API.
Apply this diff:
WnFilledButton({ super.key, super.size, super.visualState, + super.style, this.loading = false, this.prefixIcon, this.suffixIcon, this.titleTextStyle, required String title, required super.onPressed, + super.onLongPress, }) : super(
🧹 Nitpick comments (20)
lib/ui/auth_flow/info_screen.dart (2)
124-130: Add explicit size to the suffix icon for visual consistency.In other call sites within this PR, suffix icons are explicitly sized. Do the same here to avoid size discrepancies across devices/themes.
- suffixIcon: SvgPicture.asset( + suffixIcon: SvgPicture.asset( AssetsPaths.icArrowRight, + width: 18.w, + height: 18.w, colorFilter: ColorFilter.mode( context.colors.primaryForeground, BlendMode.srcIn, ), ),
121-123: Consider decoupling "loading" from provider loading state.If you only want the spinner when the user taps the button, pass _isLoading to loading and keep isButtonDisabled for disabling. This avoids showing a spinner due to unrelated provider work.
- loading: isButtonDisabled, + loading: _isLoading,lib/ui/settings/profile/share_profile_qr_scan_screen.dart (1)
114-126: API update looks good; add semantics label for accessibility.The migration to title + suffixIcon is correct. Consider adding a semanticsLabel to improve screen reader support.
- suffixIcon: SvgPicture.asset( + suffixIcon: SvgPicture.asset( AssetsPaths.icQrCode, height: 18.w, width: 18.w, + semanticsLabel: 'QR code', colorFilter: ColorFilter.mode( context.colors.primaryForeground, BlendMode.srcIn, ), ),lib/ui/chat/chat_info/dm_chat_info.dart (3)
198-214: Set both width and height on the suffix icon for consistent sizing.Other screens set both; doing the same avoids layout variance and keeps icons square.
suffixIcon: SvgPicture.asset( AssetsPaths.icSearch, - width: 14.w, + width: 14.w, + height: 14.w, colorFilter: ColorFilter.mode( context.colors.secondaryForeground, BlendMode.srcIn, ), ),
216-232: Good: loading state disables action; add height to icon for consistency.The onPressed gating with isContactLoading is correct. For consistency, specify height as well.
suffixIcon: SvgPicture.asset( isContact ? AssetsPaths.icRemoveUser : AssetsPaths.icAddUser, - width: 14.w, + width: 14.w, + height: 14.w, colorFilter: ColorFilter.mode( isContact ? context.colors.secondaryForeground : context.colors.primaryForeground, BlendMode.srcIn, ), ),
244-256: Also add height to the Add to Group icon.Keeps sizing uniform with other buttons in this screen.
suffixIcon: SvgPicture.asset( AssetsPaths.icAdd, - width: 14.w, + width: 14.w, + height: 14.w, colorFilter: ColorFilter.mode( context.colors.secondaryForeground, BlendMode.srcIn, ), ),lib/ui/settings/profile/share_profile_screen.dart (1)
178-190: Migration looks correct; consider adding a semantics label.The move to title + suffixIcon is consistent with the new API. Optional: add a semanticsLabel for screen readers.
suffixIcon: SvgPicture.asset( AssetsPaths.icScan, height: 18.w, width: 18.w, + semanticsLabel: 'Scan', colorFilter: ColorFilter.mode( context.colors.primaryForeground, BlendMode.srcIn, ), ),lib/ui/chat/chat_info/widgets/member_action_buttons.dart (2)
80-96: Prevent duplicate DM group creation while loadingIf WnFilledButton doesn’t internally disable taps during loading, this can double-trigger group creation. Safer to gate onPressed by loading state.
Apply this diff if WnFilledButton does not disable on loading:
- onPressed: _createOrOpenDirectMessageGroup, + onPressed: _isLoading ? null : _createOrOpenDirectMessageGroup,
156-171: Disable tap while loading and align disabled icon stateYou set loading: _isLoading but still allow onPressed. If the button doesn’t internally guard, taps can queue while loading. Also consider ensuring suffixIcon reflects disabled state (muted color or opacity) if the button doesn’t handle it.
Apply this diff if needed:
- onPressed: _toggleContact, + onPressed: _isLoading ? null : _toggleContact,Optional: localize the user-visible strings per guidelines.
// title: isContact ? context.l10n.removeContact : context.l10n.addContact,lib/ui/settings/general_settings_screen.dart (2)
224-229: Confirm if manual titleTextStyle override is necessary for destructive buttonIf WnFilledButton’s destructive variant already enforces the correct label color/contrast, overriding with titleTextStyle may be redundant and couples this button to typography internals.
If defaults are correct, remove the explicit style to simplify:
- titleTextStyle: WnButtonSize.small.textStyle().copyWith( - color: context.colors.solidNeutralWhite, - ),
333-344: Specify SVG icon size for consistent layoutThis suffixIcon lacks width/height, unlike other usages in this PR. Explicit sizing avoids layout jitter across devices.
Apply this diff:
- suffixIcon: SvgPicture.asset( - AssetsPaths.icArrowsVertical, - colorFilter: ColorFilter.mode( + suffixIcon: SvgPicture.asset( + AssetsPaths.icArrowsVertical, + width: 14.w, + height: 14.w, + colorFilter: ColorFilter.mode( context.colors.primary, BlendMode.srcIn, ), ),lib/ui/contact_list/start_chat_bottom_sheet.dart (1)
261-275: Show progress on Add/Remove Contact and compute isContact once
- Add loading: _isAddingContact for feedback consistency (you already do this for Start Chat).
- Optional: compute _isContact() once to avoid double provider reads in one build.
Apply this minimal UX fix:
WnFilledButton( size: WnButtonSize.small, visualState: WnButtonVisualState.secondary, onPressed: _isAddingContact ? null : _toggleContact, + loading: _isAddingContact, title: _isContact() ? 'Remove Contact' : 'Add Contact',Optional refactor (outside this hunk): compute once at the top of the buttons section and reuse:
final bool isContact = _isContact(); // then use: title: isContact ? ... : ... // and icon: isContact ? AssetsPaths.icRemoveUser : AssetsPaths.icAddUserAlso consider muted icon color when disabled, unless WnFilledButton handles this internally.
lib/ui/contact_list/widgets/profile_ready_card.dart (3)
4-4: Standardize flutter_svg importOther files import from flutter_svg/flutter_svg.dart. Align this file for consistency and IDE symbol resolution.
-import 'package:flutter_svg/svg.dart'; +import 'package:flutter_svg/flutter_svg.dart';
82-94: Localize user-facing stringPer guidelines, prefer AppLocalizations for strings.
// title: context.l10n.shareYourProfile,
97-108: Localize user-facing stringSimilarly, localize “Search For Friends”.
// title: context.l10n.searchForFriends,lib/ui/settings/app_settings/app_settings_screen.dart (2)
51-59: Migration to new WnFilledButton API looks good; minor nit on explicit text colorThe conversion to title/titleTextStyle is correct. Given WnFilledButton sets foregroundColor via visualState, you may be able to omit titleTextStyle here if the visual state already yields white for destructive buttons.
260-276: Clean adoption of suffixIcon; consider centralizing icon sizingUsage aligns with the new API and reads clearly. Consider centralizing the 18.w icon size in the button size spec (e.g., a size.iconSize getter), so icon sizing stays consistent across usages and sizes.
lib/ui/core/ui/wn_filled_button.dart (3)
75-80: Disable callbacks while loading to reflect disabled state and prevent re-tapsIgnoring pointer events works, but setting callbacks to null ensures the button uses disabled styling while loading and guards long-press too.
Apply this diff:
return FilledButton( style: effectiveStyle, - onPressed: onPressed, - onLongPress: onLongPress, - child: !loading ? child : loadingIndicator, + onPressed: loading ? null : onPressed, + onLongPress: loading ? null : onLongPress, + child: loading ? loadingIndicator : child, );
46-46: Use FilledButtonTheme instead of ElevatedButtonThemeYou’re building a FilledButton; merging the FilledButtonTheme is more appropriate than ElevatedButtonTheme and will pick up correct theme overrides.
Apply this diff:
- final theme = Theme.of(context).elevatedButtonTheme; + final theme = Theme.of(context).filledButtonTheme; @@ - .merge(theme.style); + .merge(theme.style);Also applies to: 73-73
68-71: Avoid redundant text color sourcesYou set both ButtonStyle.textStyle.color and an explicit Text style (via titleTextStyle or size.textStyle). Consider relying on visualState.foregroundColor (and not setting a color on the inner Text unless explicitly requested) to avoid conflicts.
📜 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 (2)
assets/svgs/ic_arrows_vertical.svgis excluded by!**/*.svgpubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
lib/ui/auth_flow/info_screen.dart(1 hunks)lib/ui/chat/chat_info/dm_chat_info.dart(3 hunks)lib/ui/chat/chat_info/widgets/group_member_bottom_sheet.dart(2 hunks)lib/ui/chat/chat_info/widgets/member_action_buttons.dart(2 hunks)lib/ui/contact_list/start_chat_bottom_sheet.dart(1 hunks)lib/ui/contact_list/widgets/profile_ready_card.dart(2 hunks)lib/ui/core/ui/wn_filled_button.dart(2 hunks)lib/ui/settings/app_settings/app_settings_screen.dart(4 hunks)lib/ui/settings/general_settings_screen.dart(4 hunks)lib/ui/settings/profile/edit_profile_screen.dart(2 hunks)lib/ui/settings/profile/share_profile_qr_scan_screen.dart(1 hunks)lib/ui/settings/profile/share_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/profile/share_profile_qr_scan_screen.dartlib/ui/chat/chat_info/widgets/member_action_buttons.dartlib/ui/settings/profile/share_profile_screen.dartlib/ui/chat/chat_info/widgets/group_member_bottom_sheet.dartlib/ui/settings/profile/edit_profile_screen.dartlib/ui/contact_list/widgets/profile_ready_card.dartlib/ui/settings/app_settings/app_settings_screen.dartlib/ui/settings/general_settings_screen.dartlib/ui/auth_flow/info_screen.dartlib/ui/contact_list/start_chat_bottom_sheet.dartlib/ui/chat/chat_info/dm_chat_info.dartlib/ui/core/ui/wn_filled_button.dart
🧠 Learnings (4)
📚 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/chat/chat_info/widgets/member_action_buttons.dartlib/ui/settings/app_settings/app_settings_screen.dartlib/ui/settings/general_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 flutter_rust_bridge to access core functionality of the app.
Applied to files:
lib/ui/contact_list/widgets/profile_ready_card.dartlib/ui/settings/app_settings/app_settings_screen.dartlib/ui/settings/general_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 ThemeData to manage themes.
Applied to files:
lib/ui/contact_list/widgets/profile_ready_card.dartlib/ui/settings/app_settings/app_settings_screen.dartlib/ui/settings/general_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/app_settings/app_settings_screen.dartlib/ui/settings/general_settings_screen.dart
🔇 Additional comments (8)
lib/ui/auth_flow/info_screen.dart (1)
120-131: Migration to new WnFilledButton API looks correct.Using title + suffixIcon with loading/null onPressed aligns with the new API and intention of the PR.
lib/ui/settings/profile/edit_profile_screen.dart (2)
270-281: Dialog button migration LGTM.Switch to title-based WnFilledButton with the secondaryWarning visual state and small size is consistent with the new API.
270-281: No remaining WnFilledButton.icon/child usages foundA project-wide search shows only commented-out references in lib/ui/chat/chat_info/group_chat_info.dart; there are no active calls to WnFilledButton.icon(...) or old-style child: parameters in WnFilledButton(...). No further action is needed.
lib/ui/contact_list/start_chat_bottom_sheet.dart (1)
278-291: LGTM on Add to Group button migrationMigration to title + suffixIcon reads clean and consistent with the new API.
lib/ui/chat/chat_info/widgets/group_member_bottom_sheet.dart (2)
269-283: LGTM on “Add to Another Group” button migrationUsage aligns well with the new WnFilledButton API; sizing/tinting is consistent with other buttons.
287-310: LGTM on “Remove From Group” destructive stylingClear label styling via titleTextStyle and destructive icon tint fits the warning context. Nice.
lib/ui/settings/app_settings/app_settings_screen.dart (1)
5-5: New imports are appropriate for SVG asset usageAdding flutter_svg and assets paths is correct given the new suffixIcon usage.
Also applies to: 21-21
lib/ui/core/ui/wn_filled_button.dart (1)
1-83: No active references to removed constructors
A repository-wide search forWnFilledButton.icon/.childonly returned commented-out lines inlib/ui/chat/chat_info/group_chat_info.dart. There are no live usages of the deprecated variants. ✅
| if (prefixIcon != null) ...[ | ||
| prefixIcon, | ||
| size == WnButtonSize.small ? Gap(8.w) : Gap(12.w), | ||
| ], | ||
| FittedBox( | ||
| fit: BoxFit.scaleDown, | ||
| child: Text( | ||
| title, | ||
| style: titleTextStyle ?? size.textStyle(), | ||
| ), | ||
| ), | ||
| if (suffixIcon != null) ...[ | ||
| size == WnButtonSize.small ? Gap(8.w) : Gap(12.w), | ||
| suffixIcon, | ||
| ], | ||
| ], |
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.
Fix null-safety and loosen icon types (SvgPicture? → Widget?)
- prefixIcon/suffixIcon are nullable but are inserted into a List without null assertion — this won’t compile.
- Restricting to SvgPicture is unnecessary; callers may want to pass Icon, Image, or any Widget.
Switch to Widget? and assert non-null at insertion sites.
Apply this diff:
- if (prefixIcon != null) ...[
- prefixIcon,
+ if (prefixIcon != null) ...[
+ prefixIcon!,
size == WnButtonSize.small ? Gap(8.w) : Gap(12.w),
],
@@
- if (suffixIcon != null) ...[
+ if (suffixIcon != null) ...[
size == WnButtonSize.small ? Gap(8.w) : Gap(12.w),
- suffixIcon,
+ suffixIcon!,
],
@@
- final SvgPicture? prefixIcon;
- final SvgPicture? suffixIcon;
+ final Widget? prefixIcon;
+ final Widget? suffixIcon;Also applies to: 40-42
🤖 Prompt for AI Agents
In lib/ui/core/ui/wn_filled_button.dart around lines 20-35 and 40-42,
prefixIcon/suffixIcon are currently typed as SvgPicture? and are being inserted
into a List<Widget> without null-assertion which breaks null-safety and
unnecessarily restricts icon types; change their types to Widget? (allow any
Widget such as Icon or Image), and in the widget tree assert non-null when
expanding into the children list (e.g., use the existing if (prefixIcon != null)
... pattern but insert prefixIcon! and suffixIcon!), keeping the gap logic
intact so the List<Widget> only contains non-null Widgets.
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 (5)
lib/ui/contact_list/widgets/profile_ready_card.dart (2)
85-97: Suffix icon has no explicit size; add dimensions to avoid layout/visual inconsistenciesWithout width/height, SVGs will render at their intrinsic size, which may not match the button spec (especially for small buttons). Recommend setting an explicit size.
Apply:
suffixIcon: SvgPicture.asset( AssetsPaths.icQrCode, + width: 18.w, + height: 18.w, colorFilter: ColorFilter.mode( context.colors.primary, BlendMode.srcIn, ), ),Additionally, consider localizing the title string via AppLocalizations to avoid introducing new hard-coded UI strings.
I can propose the exact localization keys/usages if you share your l10n conventions or ARB keys.
100-111: Set explicit size for the suffix icon and prefer localization for the button titleSame sizing concern as above; specify dimensions for consistency with your design tokens.
Apply:
suffixIcon: SvgPicture.asset( AssetsPaths.icAddUser, + width: 18.w, + height: 18.w, colorFilter: ColorFilter.mode( context.colors.primaryForeground, BlendMode.srcIn, ), ),Also, replace the hard-coded 'Search For Friends' with a localized string.
Happy to draft the l10n changes if you provide the localization keys or preferred naming.
lib/ui/settings/app_settings/app_settings_screen.dart (3)
50-58: Avoid overriding title text color for destructive buttons; let the component handle contrastIf
WnFilledButtonalready applies the correct foreground color fordestructive, overridingtitleTextStyleper call is unnecessary and risks drift across states (pressed/disabled).Apply:
- child: WnFilledButton( + child: WnFilledButton( visualState: WnButtonVisualState.destructive, size: WnButtonSize.small, onPressed: () => Navigator.of(dialogContext).pop(true), - title: 'Delete', - titleTextStyle: WnButtonSize.small.textStyle().copyWith( - color: context.colors.solidNeutralWhite, - ), + title: 'Delete', ),Also consider localizing 'Delete' per your l10n scheme.
263-279: Set button size explicitly to match the large style; avoid manual text color overridesYou’re using
WnButtonSize.large.textStyle()for the title, but the button’ssizeisn’t set. If the component’s default isn’t large, typography and paddings may be inconsistent. Prefer settingsize: WnButtonSize.largeand letting the component manage colors for the destructive variant.Apply:
WnFilledButton( title: 'Delete All Data', - titleTextStyle: WnButtonSize.large.textStyle().copyWith( - color: context.colors.solidNeutralWhite, - ), visualState: WnButtonVisualState.destructive, + size: WnButtonSize.large, onPressed: () => _deleteAllData(context, ref), suffixIcon: SvgPicture.asset( AssetsPaths.icDelete, height: 18.w, width: 18.w, colorFilter: ColorFilter.mode( context.colors.solidNeutralWhite, BlendMode.srcIn, ), ), ),Also, replace the hard-coded 'Delete All Data' with a localized string to comply with your localization guideline.
Confirm that
WnFilledButton’s default foreground fordestructiveis already white; if not, prefer a theme-level fix rather than per-call overrides.
5-5: Unify flutter_svg import paths across the repoPubspec.yaml declares
flutter_svg: ^2.1.0which supports the new entrypoint
import 'package:flutter_svg/svg.dart';However, we still have legacy imports of
import 'package:flutter_svg/flutter_svg.dart';in both code and tests. Please migrate all of them to the new
svg.dartentrypoint for consistency.Affected files (examples):
- test/ui/core/ui/wn_callout_test.dart
- test/ui/contact_list/.../*.dart
- lib/ui/contact_list/chat_list_screen.dart
- lib/ui/contact_list/start_chat_bottom_sheet.dart
- lib/ui/settings/general_settings_screen.dart
- …and any other files importing
package:flutter_svg/flutter_svg.dartEnsuring a single import style will avoid mixed entrypoints and potential build/runtime issues.
📜 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)
lib/ui/chat/chat_info/widgets/group_member_bottom_sheet.dart(2 hunks)lib/ui/contact_list/widgets/profile_ready_card.dart(2 hunks)lib/ui/core/ui/wn_button.dart(1 hunks)lib/ui/settings/app_settings/app_settings_screen.dart(3 hunks)lib/ui/settings/general_settings_screen.dart(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- lib/ui/core/ui/wn_button.dart
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/ui/chat/chat_info/widgets/group_member_bottom_sheet.dart
- lib/ui/settings/general_settings_screen.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/app_settings/app_settings_screen.dartlib/ui/contact_list/widgets/profile_ready_card.dart
🧠 Learnings (2)
📚 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/app_settings/app_settings_screen.dartlib/ui/contact_list/widgets/profile_ready_card.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 flutter_rust_bridge to access core functionality of the app.
Applied to files:
lib/ui/settings/app_settings/app_settings_screen.dartlib/ui/contact_list/widgets/profile_ready_card.dart
| import 'package:flutter_riverpod/flutter_riverpod.dart'; | ||
| import 'package:flutter_screenutil/flutter_screenutil.dart'; | ||
| import 'package:flutter_svg/flutter_svg.dart'; | ||
| import 'package:flutter_svg/svg.dart'; |
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.
💡 Verification agent
🧩 Analysis chain
Confirm flutter_svg import path change is compatible with your installed version
The project-wide switch to package:flutter_svg/svg.dart is fine if you’re on flutter_svg ≥2.x and consistently using the new entrypoint. Please verify the dependency version and that there are no remaining mixed imports.
Run this to verify:
🏁 Script executed:
#!/bin/bash
# Check flutter_svg dependency version
rg -n "flutter_svg:" pubspec.yaml -C2
# Find all import paths and ensure consistency
rg -nP "package:flutter_svg/(flutter_svg|svg)\.dart" -C1Length of output: 13045
Ensure consistent flutter_svg entrypoint across your codebase
- Verified pubspec.yaml uses
flutter_svg: ^2.1.0(≥2.x), so the newsvg.dartentrypoint is supported. - However, there are still numerous imports using the old path
package:flutter_svg/flutter_svg.dartin both lib/ and test/ files.
Please update all of these to the new entrypoint:
import 'package:flutter_svg/svg.dart';You can locate remaining legacy imports with:
rg -l "package:flutter_svg/flutter_svg.dart"After refactoring, rerun the above command to confirm no old imports remain.
🤖 Prompt for AI Agents
In lib/ui/contact_list/widgets/profile_ready_card.dart around line 4, the file
currently imports the old flutter_svg entrypoint; replace any occurrence of
"package:flutter_svg/flutter_svg.dart" with the new entrypoint
"package:flutter_svg/svg.dart" to match the pubspec >=2.x requirement, apply the
same replacement across all lib/ and test/ files (use rg -l
"package:flutter_svg/flutter_svg.dart" to find them), update any affected import
statements in tests and widgets, and re-run the ripgrep command to confirm no
legacy imports remain.
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.
Thanks for working on this one! 🙌
My intuition was that we could have one prop for the SVG icon and another for its position, with a default (probably "right" cause most icons are used as suffix). Buut I also like the current approach, since the prop name makes the position clear.
I really like how this is going, it’s a big improvement that we’re no longer using reversed text and icon props 🚀
lib/ui/auth_flow/info_screen.dart
Outdated
| ), | ||
| ), | ||
| ], | ||
| title: 'Setup Profile', |
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.
[sug] "label" for me is more intuitive for button text than "title"... WDYT?
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.
Not bad, let's go with label.... Will make a push soon.
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.
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.
Amazing. Checked locally every screen that was in files changed list and they look great. Just left some small doubts related to some font size changes in some screens. If @vladimir-krstic is ok with the font sizes, then I give my ✅ !
Can't verify from screenshots but they do look odd. Button text font specs should be: |
|
Thanks @josefinalliende @vladimir-krstic, resolving that now |
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: 0
🔭 Outside diff range comments (1)
lib/ui/chat/chat_info/widgets/group_member_bottom_sheet.dart (1)
260-261: Row does not supportspacing; insert an explicit Gap between childrenFlutter’s
Rowhas nospacingnamed parameter. This will not compile on stable. Use aGapbetween theFlexiblechildren (you already importgap).Apply this diff:
- Row( - spacing: 6.w, - children: [ + Row( + children: [ Flexible( child: SendMessageButton(widget.member), ), + Gap(6.w), Flexible( child: AddToContactButton(widget.member), ), ], ),
♻️ Duplicate comments (2)
lib/ui/settings/network/add_relay_bottom_sheet.dart (1)
8-8: Switch flutter_svg import to the new entrypoint for consistencyUse the updated entrypoint to align with the rest of the repo and flutter_svg ≥2.x guidance.
Apply:
-import 'package:flutter_svg/flutter_svg.dart'; +import 'package:flutter_svg/svg.dart';lib/ui/core/ui/wn_filled_button.dart (1)
16-35: Fix null-safety and allow any widget for icons (SvgPicture? → Widget?)
prefixIcon/suffixIconare nullable fields but inserted into aList<Widget>without!, which violates null-safety. Also, restricting these toSvgPictureis unnecessarily tight; callers may want to passIcon,Image, or anyWidget.Apply this diff:
children: [ - if (prefixIcon != null) ...[ - prefixIcon, + if (prefixIcon != null) ...[ + prefixIcon!, size == WnButtonSize.small ? Gap(8.w) : Gap(12.w), ], FittedBox( fit: BoxFit.scaleDown, child: Text( label, style: titleTextStyle ?? size.textStyle(), ), ), - if (suffixIcon != null) ...[ + if (suffixIcon != null) ...[ size == WnButtonSize.small ? Gap(8.w) : Gap(12.w), - suffixIcon, + suffixIcon!, ], ], ), ); final bool loading; - final SvgPicture? prefixIcon; - final SvgPicture? suffixIcon; + final Widget? prefixIcon; + final Widget? suffixIcon; final TextStyle? titleTextStyle;Also applies to: 39-42
🧹 Nitpick comments (11)
lib/ui/settings/developer/developer_settings_screen.dart (1)
127-127: Localize button labelsConsider moving hard-coded labels ('Clear Cache', 'Reload Contacts') to AppLocalizations for i18n consistency.
Also applies to: 136-136
lib/ui/auth_flow/login_screen.dart (1)
231-231: Localize the 'Login' labelRecommend sourcing this from AppLocalizations to support translations.
lib/ui/chat/invite/chat_invite_screen.dart (1)
74-74: Localize action labelsPlease consider moving 'Decline' and 'Accept' to AppLocalizations to maintain i18n consistency across the app.
Also applies to: 85-85
lib/ui/settings/network/widgets/relay_tile.dart (1)
80-80: Localize dialog button text'Cancel' and 'Remove Relay' should come from AppLocalizations to support translations.
Also applies to: 87-87
lib/ui/chat/chat_management/add_to_group_screen.dart (1)
218-218: Localize CTA textConsider moving 'Add to Group' to AppLocalizations for i18n.
lib/ui/settings/profile/switch_profile_bottom_sheet.dart (1)
104-119: Avoid repeated npub→hex conversions by leveraging the cache_isActiveAccount currently recomputes/awaits hex conversion per tile even though _precomputeProfileHexes already maintains _pubkeyToHex. Use the cache first, and populate it on miss. This reduces rebuild cost and avoids redundant async work.
Apply this diff:
- Future<bool> _isActiveAccount(ContactModel profile) async { - if (_activeAccountHex == null) return false; - - try { - // Convert profile's npub key to hex for comparison - String profileHex = profile.publicKey; - if (profile.publicKey.startsWith('npub1')) { - profileHex = await hexPubkeyFromNpub(npub: profile.publicKey); - } - - return profileHex == _activeAccountHex; - } catch (e) { - // If conversion fails, try direct comparison as fallback - return profile.publicKey == _activeAccountHex; - } - } + Future<bool> _isActiveAccount(ContactModel profile) async { + if (_activeAccountHex == null) return false; + final original = profile.publicKey; + + // Fast-path using the precomputed cache + final cached = _pubkeyToHex[original]; + if (cached != null) { + return cached == _activeAccountHex; + } + + try { + final hex = original.startsWith('npub1') + ? await hexPubkeyFromNpub(npub: original) + : original; + // Cache for subsequent calls + _pubkeyToHex[original] = hex; + return hex == _activeAccountHex; + } catch (_) { + // Fallback to original and cache the result to avoid repeated work + _pubkeyToHex[original] = original; + return original == _activeAccountHex; + } + }lib/ui/contact_list/group_chat_details_sheet.dart (1)
176-210: Speed up keypackage checks by parallelizing I/O_fetchKeyPackage is awaited sequentially for each contact. Given this is all I/O, you can parallelize with Future.wait to reduce overall latency. Consider a bounded concurrency pool if the upstream can’t handle bursts.
Example implementation:
Future<Map<String, List<ContactModel>>> _filterContactsByKeyPackage( List<ContactModel> contacts, ) async { final activeAccountData = await ref.read(activeAccountProvider.notifier).getActiveAccountData(); if (activeAccountData == null) { throw Exception('No active account found'); } final withKey = <ContactModel>[]; final withoutKey = <ContactModel>[]; // Fire requests in parallel final results = await Future.wait( contacts.map((contact) async { try { final pubkey = await publicKeyFromString(publicKeyString: contact.publicKey); final keyPackage = await fetchKeyPackage( pubkey: pubkey, nip65Relays: activeAccountData.nip65Relays, ); return (contact, hasKey: keyPackage != null); } catch (_) { return (contact, hasKey: false); } }), eagerError: false, ); for (final r in results) { if (r.hasKey) { withKey.add(r.$1); } else { withoutKey.add(r.$1); } } return { 'withKeyPackage': withKey, 'withoutKeyPackage': withoutKey, }; }If you need to limit concurrency, you can chunk contacts or use a simple semaphore.
lib/ui/auth_flow/create_profile_screen.dart (1)
61-71: Avoid calling ref.listen inside build to prevent multiple subscriptionsPlacing ref.listen in build risks creating a new listener on every rebuild. Move it to initState (ConsumerState provides ref there) and remove the build-time listener.
Proposed change (initState addition):
@override void initState() { super.initState(); WidgetsBinding.instance.addPostFrameCallback((_) async { // existing metadata-loading logic unchanged… }); // Move this from build to initState ref.listen<AccountState>(accountProvider, (previous, next) { if (next.metadata?.displayName != null && next.metadata!.displayName!.isNotEmpty && _displayNameController.text.isEmpty) { _displayNameController.text = next.metadata!.displayName!; if (mounted) { setState(() { _isLoadingDisplayName = false; }); } } }); }Then remove the ref.listen block from build.
lib/ui/chat/chat_info/widgets/group_member_bottom_sheet.dart (1)
275-289: Suffix icon migration reads well; verify icon sizing consistencyThe trailing icon implementation using
suffixIconis correct. Quick check: ensure the 14.w x 13.h sizing is intentional vs. other small buttons (many use 18x18). If it's by design for this specific button, all good.lib/ui/contact_list/start_chat_bottom_sheet.dart (1)
261-275: Avoid double provider reads and show loading for better UX
_isContact()is called twice (label and icon), each callingref.watch(...). Cache it once per build to avoid redundant provider reads.- Show a spinner when
_isAddingContactis true using theloadingprop (you already disable onPressed).Apply this diff to the button:
WnFilledButton( size: WnButtonSize.small, visualState: WnButtonVisualState.secondary, - onPressed: _isAddingContact ? null : _toggleContact, - label: _isContact() ? 'Remove Contact' : 'Add Contact', + loading: _isAddingContact, + onPressed: _isAddingContact ? null : _toggleContact, + label: isContact ? 'Remove Contact' : 'Add Contact', suffixIcon: SvgPicture.asset( - _isContact() ? AssetsPaths.icRemoveUser : AssetsPaths.icAddUser, + isContact ? AssetsPaths.icRemoveUser : AssetsPaths.icAddUser, width: 18.w, height: 18.w, colorFilter: ColorFilter.mode( context.colors.primary, BlendMode.srcIn, ), ), ),And add this once at the start of
build(or just before rendering the buttons):// At the start of build(), before returning the Column: final bool isContact = _isContact();lib/ui/settings/app_settings/app_settings_screen.dart (1)
5-5: Use a consistent flutter_svg import across the appThis file imports
package:flutter_svg/svg.dartwhile many others usepackage:flutter_svg/flutter_svg.dart. Both paths can work depending on flutter_svg version, but mixing them is brittle and surprises tooling. Prefer a single import path project-wide.If you choose the canonical one used elsewhere in the repo, apply:
-import 'package:flutter_svg/svg.dart'; +import 'package:flutter_svg/flutter_svg.dart';
📜 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 (26)
lib/ui/auth_flow/create_profile_screen.dart(1 hunks)lib/ui/auth_flow/info_screen.dart(1 hunks)lib/ui/auth_flow/login_screen.dart(1 hunks)lib/ui/auth_flow/welcome_screen.dart(1 hunks)lib/ui/chat/chat_info/dm_chat_info.dart(3 hunks)lib/ui/chat/chat_info/widgets/group_member_bottom_sheet.dart(3 hunks)lib/ui/chat/chat_info/widgets/member_action_buttons.dart(2 hunks)lib/ui/chat/chat_management/add_to_group_screen.dart(1 hunks)lib/ui/chat/invite/chat_invite_screen.dart(2 hunks)lib/ui/contact_list/group_chat_details_sheet.dart(1 hunks)lib/ui/contact_list/group_welcome_invitation_sheet.dart(2 hunks)lib/ui/contact_list/new_group_chat_sheet.dart(1 hunks)lib/ui/contact_list/start_chat_bottom_sheet.dart(1 hunks)lib/ui/contact_list/widgets/profile_ready_card.dart(2 hunks)lib/ui/contact_list/widgets/share_invite_button.dart(1 hunks)lib/ui/core/ui/wn_filled_button.dart(2 hunks)lib/ui/settings/app_settings/app_settings_screen.dart(3 hunks)lib/ui/settings/developer/developer_settings_screen.dart(2 hunks)lib/ui/settings/general_settings_screen.dart(2 hunks)lib/ui/settings/network/add_relay_bottom_sheet.dart(1 hunks)lib/ui/settings/network/widgets/relay_tile.dart(1 hunks)lib/ui/settings/profile/connect_profile_bottom_sheet.dart(2 hunks)lib/ui/settings/profile/edit_profile_screen.dart(4 hunks)lib/ui/settings/profile/share_profile_qr_scan_screen.dart(1 hunks)lib/ui/settings/profile/share_profile_screen.dart(1 hunks)lib/ui/settings/profile/switch_profile_bottom_sheet.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- lib/ui/auth_flow/info_screen.dart
- lib/ui/settings/general_settings_screen.dart
- lib/ui/settings/profile/share_profile_qr_scan_screen.dart
- lib/ui/chat/chat_info/widgets/member_action_buttons.dart
- lib/ui/chat/chat_info/dm_chat_info.dart
- lib/ui/settings/profile/share_profile_screen.dart
- lib/ui/settings/profile/edit_profile_screen.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/auth_flow/login_screen.dartlib/ui/contact_list/new_group_chat_sheet.dartlib/ui/settings/network/widgets/relay_tile.dartlib/ui/chat/chat_management/add_to_group_screen.dartlib/ui/contact_list/group_chat_details_sheet.dartlib/ui/settings/developer/developer_settings_screen.dartlib/ui/settings/profile/connect_profile_bottom_sheet.dartlib/ui/auth_flow/create_profile_screen.dartlib/ui/contact_list/group_welcome_invitation_sheet.dartlib/ui/settings/profile/switch_profile_bottom_sheet.dartlib/ui/contact_list/widgets/share_invite_button.dartlib/ui/auth_flow/welcome_screen.dartlib/ui/chat/invite/chat_invite_screen.dartlib/ui/chat/chat_info/widgets/group_member_bottom_sheet.dartlib/ui/contact_list/widgets/profile_ready_card.dartlib/ui/settings/app_settings/app_settings_screen.dartlib/ui/settings/network/add_relay_bottom_sheet.dartlib/ui/core/ui/wn_filled_button.dartlib/ui/contact_list/start_chat_bottom_sheet.dart
🧠 Learnings (2)
📚 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/contact_list/widgets/profile_ready_card.dartlib/ui/settings/app_settings/app_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 flutter_rust_bridge to access core functionality of the app.
Applied to files:
lib/ui/contact_list/widgets/profile_ready_card.dartlib/ui/settings/app_settings/app_settings_screen.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 (28)
lib/ui/settings/developer/developer_settings_screen.dart (2)
126-131: WnFilledButton migration tolabel— looks correctBoth button usages correctly adopt the new
labelAPI. No functional concerns from this change.Also applies to: 135-139
126-139: Deprecated WnFilledButton API Fully RemovedI’ve scanned for all active uses of
WnFilledButton.icon/.childconstructors and anytitle:parameters—no live call sites remain. The only references are commented-out snippets in:
- lib/ui/chat/chat_info/group_chat_info.dart (lines 162, 174)
These can be deleted at your discretion. The migration to
label/prefixIcon/suffixIconis complete.lib/ui/auth_flow/login_screen.dart (1)
229-233: WnFilledButtonlabelmigration — goodThe update to
label: 'Login'aligns with the new API. Loading/disable logic remains intact.lib/ui/chat/invite/chat_invite_screen.dart (1)
73-82: Button API migration looks correctBoth 'Decline' and 'Accept' buttons are updated to
labeland preserve previous behaviors (secondary/destructive/loader/disable). Looks good.Also applies to: 84-118
lib/ui/settings/network/widgets/relay_tile.dart (1)
78-83: Dialog buttons migrated tolabelcorrectlyThe cancel and destructive actions keep their visual states and sizes; migration is accurate.
Also applies to: 85-90
lib/ui/chat/chat_management/add_to_group_screen.dart (1)
217-221: CTA migrated tolabel— looks goodThe bottom action now uses
labelwhile preserving loading/enablement logic. No issues spotted.lib/ui/settings/profile/switch_profile_bottom_sheet.dart (2)
213-215: WnFilledButton(title → label) migration looks goodThe call site correctly uses label and keeps behavior intact.
213-215: All deprecated WnFilledButton APIs have been removedI ran the scan across the Dart codebase and confirmed there are no active usages of:
WnFilledButton(title: …)WnFilledButton.iconWnFilledButton.childThe only occurrences of
WnFilledButton.iconare in commented-out legacy snippets (e.g. inlib/ui/chat/chat_info/group_chat_info.dart), so no further changes are needed here.lib/ui/contact_list/widgets/share_invite_button.dart (1)
55-56: WnFilledButton(title → label) migration looks goodCorrectly maps the dynamic text to label and preserves loading/disabled behavior.
lib/ui/contact_list/group_chat_details_sheet.dart (1)
301-304: WnFilledButton(title → label) migration looks goodLabel text and loading/enablement logic remain consistent with prior behavior.
lib/ui/auth_flow/create_profile_screen.dart (1)
205-218: WnFilledButton(title → label) migration looks goodKeeps the same disabled/loading behavior while adopting the new API.
lib/ui/settings/profile/connect_profile_bottom_sheet.dart (2)
42-64: WnFilledButton(title → label) migration looks goodThe semantics and disabled/loading behavior are preserved; visualState remains secondary.
67-82: WnFilledButton(title → label) migration looks goodReads authProvider.isLoading for loading state, consistent with prior logic.
lib/ui/auth_flow/welcome_screen.dart (1)
96-99: No lingering legacy WnFilledButton APIs found
Scanned the entire codebase—there are no active WnFilledButton.child or .icon constructors, nor any title: named arguments on WnFilledButton. All call sites now correctly use label/prefixIcon/suffixIcon.lib/ui/settings/network/add_relay_bottom_sheet.dart (1)
249-252: WnFilledButton API migration (label) is correctThe call site update from title to label preserves behavior and matches the new API.
lib/ui/contact_list/new_group_chat_sheet.dart (1)
208-209: WnFilledButton label migration LGTMThe change to label: 'Continue' is consistent with the new API, with no behavior changes.
lib/ui/contact_list/group_welcome_invitation_sheet.dart (1)
83-84: Button text param rename is correct and behavior-preservingBoth Decline and Accept now use label as expected. No other logic changes—good.
Also applies to: 93-94
lib/ui/contact_list/widgets/profile_ready_card.dart (3)
4-4: Good move to the new flutter_svg entrypointThanks for switching to package:flutter_svg/svg.dart; this matches the repo-wide migration.
85-97: Clean refactor to label + suffixIcon — clearer API and easier to scanReplacing the old child variant with explicit label and suffixIcon improves readability and reduces misuse risk. ColorFilter choice also matches the secondary visual state.
100-111: Second button migration also looks goodExplicit label + suffixIcon with default visual state is appropriate here; the icon color aligns with a primary button.
lib/ui/chat/chat_info/widgets/group_member_bottom_sheet.dart (3)
161-167: Migration tolabelparam looks correctThe cancel button correctly uses the new
labelAPI with the small secondary visual state. No functional issues spotted.
169-174: Destructive confirm button wiring is soundUsing
loading: _isRemovingplus disablingonPressedwhile removing aligns with the new API and avoids duplicate actions. Good update.
292-316: Secondary warning + custom textStyle is coherentApplying
WnButtonVisualState.secondaryWarningand overriding the label color viatitleTextStylekeeps the text/icon red while retaining the secondary-warning container — consistent with the rest of the design system. Looks good.lib/ui/contact_list/start_chat_bottom_sheet.dart (2)
278-291: LGTM: clean migration tosuffixIcon“Add to Group” correctly adopts the new API with a trailing icon and secondary style. No issues noted.
293-297: Good: loading state and label stay in syncStart Chat button properly toggles both the
loadingindicator and the label. This avoids accidental re-taps during group creation.lib/ui/settings/app_settings/app_settings_screen.dart (2)
41-46: Dialog actions: migration tolabellooks correctCancel/Confirm buttons in the dialog use the new API properly. The small sizes and visual states match expected semantics.
263-279: Destructive CTA migration is correct; style override is appropriate“Delete All Data” correctly uses
labeland a trailing icon. Overriding text color to white viatitleTextStylealigns with the destructive visual state and button theme.lib/ui/core/ui/wn_filled_button.dart (1)
68-71: Confirm precedence of text style vs. visual state color
buildButtonsetstextStyle: size.textStyle().copyWith(color: visualState.foregroundColor(context)), while the labelTextusestitleTextStyle ?? size.textStyle(). TheTextstyle will take precedence over theButtonStyle.textStyle. This is fine, but ensure this is intentional since call sites (e.g., destructive secondary with red label) rely on the override.No changes requested; just a heads-up.
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.
Cool! 😎 So I checked the code of the screenshots that I left earlier and just one sheet needs small change to match the font sie vlad mentioned, the others are ok! I left the code suggestions, just 2 lines.
Co-authored-by: Josefina Alliende <mariajosefinaalliende@gmail.com>
Co-authored-by: Josefina Alliende <mariajosefinaalliende@gmail.com>
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: 2
🔭 Outside diff range comments (1)
lib/ui/contact_list/start_chat_bottom_sheet.dart (1)
162-171: Riverpod misuse: _isContact() uses watch; calling it in an event handler can assert at runtime.
_toggleContactcalls_isContact(), which usesref.watch.watchmust only be used during build. Useref.readin handlers.try { - final contactsNotifier = ref.read(contactsProvider.notifier); - final isCurrentlyContact = _isContact(); + final contactsNotifier = ref.read(contactsProvider.notifier); + final contactsState = ref.read(contactsProvider); + final List<ContactModel> contacts = contactsState.contactModels ?? const <ContactModel>[]; + final bool isCurrentlyContact = contacts.any( + (c) => c.publicKey.toLowerCase() == widget.contact.publicKey.toLowerCase(), + );Optionally, consider removing
_isContact()entirely after cachingisContactinbuildto avoid divergent logic paths.
🧹 Nitpick comments (2)
lib/ui/contact_list/start_chat_bottom_sheet.dart (2)
97-102: Avoid side-effects inside setState.Calling
ref.showErrorToast(...)insidesetStateis unnecessary and can surprise. Do the state updates insetState, then call the toast outside.- setState(() { - _isLoadingKeyPackage = false; - _isLoadingKeyPackageError = true; - ref.showErrorToast('Error loading contact: $e'); - }); + setState(() { + _isLoadingKeyPackage = false; + _isLoadingKeyPackageError = true; + }); + ref.showErrorToast('Error loading contact: $e');
294-297: Optional: rely on loading indicator and keep label stable.Given
loading: _isCreatingGroupalready shows progress, consider keeping the label constant to avoid text jank during state transitions.- label: _isCreatingGroup ? 'Creating Chat...' : 'Start Chat', + label: 'Start Chat',
📜 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 (1)
lib/ui/contact_list/start_chat_bottom_sheet.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/contact_list/start_chat_bottom_sheet.dart
🪛 GitHub Actions: CI
lib/ui/contact_list/start_chat_bottom_sheet.dart
[error] 269-269: avoid_redundant_argument_values: The value of the argument is redundant because it matches the default value.
[error] 285-285: avoid_redundant_argument_values: The value of the argument is redundant because it matches the default value.
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: 0
♻️ Duplicate comments (1)
lib/ui/core/ui/wn_filled_button.dart (1)
20-35: Fix nullable icon insertion and loosen icon types to Widget? (compile-time + API flexibility)
- Fields are nullable but are inserted into a List without null assertion. Because these are instance fields, Dart won't promote them to non-null in the collection-if scope — this can fail to compile.
- Restricting to SvgPicture unnecessarily couples the button to one icon type; callers often need Icon, Image, or any Widget.
Apply the following changes:
- Use Widget? for prefix/suffix to accept any widget.
- Use null assertions when inserting them into children.
children: [ - if (prefixIcon != null) ...[ - prefixIcon, + if (prefixIcon != null) ...[ + prefixIcon!, size == WnButtonSize.small ? Gap(8.w) : Gap(12.w), ], @@ - if (suffixIcon != null) ...[ + if (suffixIcon != null) ...[ size == WnButtonSize.small ? Gap(8.w) : Gap(12.w), - suffixIcon, + suffixIcon!, ], ],- final SvgPicture? prefixIcon; - final SvgPicture? suffixIcon; + final Widget? prefixIcon; + final Widget? suffixIcon; final TextStyle? labelTextStyle;Also applies to: 40-42
🧹 Nitpick comments (3)
lib/ui/chat/chat_info/widgets/group_member_bottom_sheet.dart (1)
275-289: Prefer localized strings and confirm icon a11y intent
- Consider moving hard-coded labels ("Add to Another Group", "Remove From Group") to AppLocalizations to match project standards for i18n.
- If the suffix icons are decorative, keep them silent for screen readers (SvgPicture has semanticsLabel; keep it null or consider wrapping with ExcludeSemantics if needed).
If you want, I can prepare a quick patch wiring these strings into AppLocalizations and confirming semantics behavior for the SVGs.
Also applies to: 292-316
lib/ui/core/ui/wn_filled_button.dart (2)
26-30: Avoid overriding the button’s state-driven text colorThe FilledButton.styleFrom already sets textStyle with the correct state-driven color via visualState. Passing a concrete TextStyle to Text (size.textStyle()) can override that and desync the label color from hover/disabled states.
Let the ButtonStyle drive defaults and only apply labelTextStyle when explicitly provided.
- style: labelTextStyle ?? size.textStyle(), + style: labelTextStyle,Also applies to: 68-70
75-80: Optionally disable callbacks while loading for consistent semanticsYou already gate interactions with ignorePointer: loading and swap the child with a loading indicator. Disabling callbacks too makes semantics/ink ripple behavior consistent with “disabled” Material state.
- return FilledButton( - style: effectiveStyle, - onPressed: onPressed, - onLongPress: onLongPress, - child: !loading ? child : loadingIndicator, - ); + return FilledButton( + style: effectiveStyle, + onPressed: loading ? null : onPressed, + onLongPress: loading ? null : onLongPress, + child: !loading ? child : loadingIndicator, + );
📜 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/chat_info/widgets/group_member_bottom_sheet.dart(3 hunks)lib/ui/core/ui/wn_filled_button.dart(2 hunks)lib/ui/settings/app_settings/app_settings_screen.dart(3 hunks)lib/ui/settings/general_settings_screen.dart(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/ui/settings/general_settings_screen.dart
- lib/ui/settings/app_settings/app_settings_screen.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/chat/chat_info/widgets/group_member_bottom_sheet.dartlib/ui/core/ui/wn_filled_button.dart
🔇 Additional comments (1)
lib/ui/chat/chat_info/widgets/group_member_bottom_sheet.dart (1)
165-166: Migration to label-only API looks correctThe updated WnFilledButton(label: ...) usage for Cancel/Remove actions aligns with the new API and preserves the original behavior.
Also applies to: 173-174
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! Approved!
| key: const ValueKey('buttons'), | ||
| children: [ | ||
| WnFilledButton( | ||
| size: WnButtonSize.large, |
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.
Oopps, so just by leaving no size was ok! Good catch!
…ad of misleading and hacky params.
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: 0
🧹 Nitpick comments (4)
lib/ui/chat/chat_info/dm_chat_info.dart (1)
201-201: Localize user-visible button labelsReplace hard-coded strings ('Search Chat', 'Remove Contact'/'Add Contact', 'Add to Group') with AppLocalizations to comply with guidelines and enable i18n.
If you share your l10n keys (or preferred phrasing), I can generate the exact replacements for these labels.
Also applies to: 220-220, 247-247
lib/ui/chat/chat_info/widgets/group_member_bottom_sheet.dart (3)
275-289: Match suffixIcon tint to the button’s secondary visual stateThe secondary button variant typically uses secondaryForeground for its content color. Here the suffixIcon uses primary, which diverges from similar usage elsewhere (e.g., DMChatInfo). Consider aligning the tint for consistency.
Apply this minimal diff:
suffixIcon: SvgPicture.asset( AssetsPaths.icChatInvite, width: 14.w, height: 13.h, colorFilter: ColorFilter.mode( - context.colors.primary, + context.colors.secondaryForeground, BlendMode.srcIn, ), ),Please confirm the intended spec for secondary buttons’ icon/text color. If the design system expects primary here, disregard this suggestion.
292-316: Avoid manual labelTextStyle override if the variant already encodes warning/destructive stylingIf WnButtonVisualState.secondaryWarning is responsible for destructive color/weight, the explicit labelTextStyle override may be redundant and risks diverging from theme updates.
Proposed simplification:
WnFilledButton( onPressed: () async { final result = await _openRemoveFromGroupDialog(); if (result == true && context.mounted) { Navigator.pop(context); } }, size: WnButtonSize.small, visualState: WnButtonVisualState.secondaryWarning, - label: 'Remove From Group', - labelTextStyle: context.textTheme.bodyMedium?.copyWith( - color: context.colors.destructive, - fontWeight: FontWeight.w600, - fontSize: 14.sp, - ), + label: 'Remove From Group', suffixIcon: SvgPicture.asset( AssetsPaths.icRemoveOutlined, width: 14.w, height: 13.h, colorFilter: ColorFilter.mode( context.colors.destructive, BlendMode.srcIn, ), ), ),If the variant does not set the text color as desired, keeping the override is fine—just confirm it’s intentional.
166-166: Localize button labels in this sheetUse AppLocalizations instead of hard-coded strings ('Cancel', both instances of 'Remove From Group', 'Add to Another Group') to follow the project’s localization guideline.
I can draft the l10n ARB entries and replacements if you provide the desired keys/namespaces.
Also applies to: 174-174, 279-279, 301-301
📜 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 (11)
lib/ui/auth_flow/create_profile_screen.dart(1 hunks)lib/ui/auth_flow/login_screen.dart(1 hunks)lib/ui/chat/chat_info/dm_chat_info.dart(3 hunks)lib/ui/chat/chat_info/widgets/group_member_bottom_sheet.dart(3 hunks)lib/ui/chat/chat_management/add_to_group_screen.dart(1 hunks)lib/ui/chat/invite/chat_invite_screen.dart(2 hunks)lib/ui/contact_list/group_welcome_invitation_sheet.dart(2 hunks)lib/ui/contact_list/start_chat_bottom_sheet.dart(1 hunks)lib/ui/settings/network/add_relay_bottom_sheet.dart(1 hunks)lib/ui/settings/profile/edit_profile_screen.dart(4 hunks)lib/ui/settings/profile/share_profile_screen.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- lib/ui/settings/network/add_relay_bottom_sheet.dart
- lib/ui/chat/invite/chat_invite_screen.dart
- lib/ui/chat/chat_management/add_to_group_screen.dart
- lib/ui/contact_list/group_welcome_invitation_sheet.dart
- lib/ui/auth_flow/create_profile_screen.dart
- lib/ui/settings/profile/share_profile_screen.dart
- lib/ui/auth_flow/login_screen.dart
- lib/ui/settings/profile/edit_profile_screen.dart
- lib/ui/contact_list/start_chat_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/chat/chat_info/widgets/group_member_bottom_sheet.dartlib/ui/chat/chat_info/dm_chat_info.dart
🔇 Additional comments (5)
lib/ui/chat/chat_info/dm_chat_info.dart (3)
198-214: Migration to new WnFilledButton API (Search Chat) looks correctClean swap to label + suffixIcon; tinting matches the secondary variant. onPressed flow preserved.
216-242: Add/Remove Contact button refactor is solid
- Correctly uses loading to disable onPressed while pending.
- Visual state and suffixIcon color vary with isContact as expected.
244-257: Add to Group migration is consistent and correctLabel/suffixIcon usage aligns with the new API; tinting matches the secondary variant.
lib/ui/chat/chat_info/widgets/group_member_bottom_sheet.dart (2)
165-166: Dialog “Cancel” button migration is correctSwitched to label with secondary visual state; handler unchanged.
173-174: Dialog “Remove From Group” migration is correctUses label and preserves loading/disabled state wiring via _isRemoving.
Co-authored-by: Josefina Alliende <mariajosefinaalliende@gmail.com>
Co-authored-by: Josefina Alliende <mariajosefinaalliende@gmail.com>
….com/parres-hq/whitenoise_flutter into 395-add-icon-position-arg-to-buttons
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.
✅ Go go go! 🚀


Description
Fixes #395
This is a QoL refactor for WnFilledButton where we had label and icon params as Widget. This often leads to mistake as sometimes devs pass icon to label and vice-versa
Type of Change
What to expect
There is no .icon or .child variant of the WnFilledButton anymore. For Regular filled buttons, either with prefix or suffix icons, use WnFilledButton at lib/ui/core/ui/wn_filled_button.dart. This changed was guided by the buttons components page on Figma.
For IconButtons, please use CustomIconButton at lib/shared/custom_icon_button.dart. It can be updated to fully support al usage types/variants.
Benefits
Consistency and no/less confusion, I think :D :D . Simply specify what you want, and the position comes up just right.
Checklist
just precommitto ensure that formatting and linting are correctCHANGELOG.mdfile with your changesSummary by CodeRabbit
New Features
Style
Refactor