feat(kyc): Message signing #3161
Conversation
…into add/kyc-pre-implementation
Rework UI button widgets (e.g. `UiPrimaryButton`) to bring in line with Flutter layout best practices. These changes will be required for full localisation support since we will need dynamic width buttons.
…t and show result in dialog - Replaced inline result display with a dialog using AlertDialog - Dialog content is now wrapped in a Column with mainAxisSize.min and 32px padding for better presentation - Refactored screen layout to include PageHeader (Back to Wallet) to match Send and Receive screens
…ation and result states - Introduced a complete message signing flow using Bloc architecture - Added MessageSigningBloc, events, and state with Equatable integration - Implemented multi-step UI: - Signing form with address selector, message input, QR, and paste actions - Confirmation step with address and message preview and acknowledgment checkbox - Result card showing signed message and sharing options - Extracted form, confirmation, and result views into dedicated widgets for clarity - Enhanced UI styling with unified card designs and split field containers
…g strings - Applied new UI changes to message signing form - Localized text strings based of feedback
# Conflicts: # assets/translations/en.json # lib/bloc/auth_bloc/auth_bloc.dart # lib/generated/codegen_loader.g.dart # lib/shared/ui/ui_primary_button.dart # lib/shared/widgets/connect_wallet/connect_wallet_button.dart # lib/shared/widgets/disclaimer/disclaimer.dart # lib/views/bitrefill/bitrefill_button.dart # lib/views/bitrefill/bitrefill_button_view.dart # lib/views/dex/simple/form/tables/orders_table/grouped_list_view.dart # lib/views/fiat/fiat_page.dart # lib/views/market_maker_bot/coin_search_dropdown.dart # lib/views/market_maker_bot/coin_selection_and_amount_input.dart # lib/views/wallet/coin_details/coin_details_info/coin_details_common_buttons.dart # lib/views/wallet/coin_details/faucet/faucet_button.dart # lib/views/wallet/coin_details/withdraw_form/pages/failed_page.dart # lib/views/wallet/coin_details/withdraw_form/widgets/send_complete_form/send_complete_form_buttons.dart # lib/views/wallet/coin_details/withdraw_form/widgets/send_confirm_form/send_confirm_buttons.dart # lib/views/wallet/wallet_page/wallet_main/wallet_manage_section.dart # lib/views/wallets_manager/widgets/wallet_list_item.dart # packages/komodo_ui_kit/lib/src/buttons/ui_primary_button.dart # packages/komodo_ui_kit/lib/src/images/coin_icon.dart
- alias mm2 BestOrder in grouped_list_view to disambiguate - pass required isMobile to CoinDetailsCommonButtons
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a full message-signing feature (Bloc, state/events, UI widgets, screen, routing), updates translations, introduces flexible button sizing APIs in UI kit, tweaks various UI components and tooltips, adjusts DEX orders balance sourcing, enhances ticker parsing, and makes minor constructor/keys and const-ness updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant CoinDetails as CoinDetails
participant Screen as MessageSigningScreen
participant Bloc as MessageSigningBloc
participant SDK as KomodoDefiSdk
User->>CoinDetails: Tap "Sign Message"
CoinDetails->>Screen: Open with Coin
Screen->>Bloc: BlocProvider + AddressesRequested(asset)
Bloc->>SDK: fetch pubkeys for asset
SDK-->>Bloc: addresses or error
Bloc-->>Screen: state: ready with addresses
User->>Screen: Select address + Enter message
Screen->>Bloc: InputConfirmed
Bloc-->>Screen: state: confirming
alt User cancels
User->>Screen: Cancel
Screen->>Bloc: ConfirmationCancelled
Bloc-->>Screen: state: ready
else User confirms
User->>Screen: Confirm
Screen->>Bloc: FormSubmitted(message, coinAbbr)
Bloc->>SDK: signMessage(coin, address, message)
alt Success
SDK-->>Bloc: signedMessage
Bloc-->>Screen: state: success (+signedMessage)
Screen-->>User: Show result (copy/share)
else Failure
SDK-->>Bloc: error
Bloc-->>Screen: state: failure (+errorMessage)
Screen-->>User: Show error
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| ), | ||
| ), | ||
| ], | ||
| ), |
There was a problem hiding this comment.
Bug: Share Button Copies, Copy Button Unimplemented
The "Share" button copies the message to the clipboard, while the "Copy" button is unimplemented and intended for share logic. This swaps their functionality, leaving the "Copy" button non-functional.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting
| Row( | ||
| children: [ | ||
| Expanded( | ||
| child: UiSecondaryButton( | ||
| text: 'Share', | ||
| onPressed: () { | ||
| Clipboard.setData(ClipboardData(text: signedMessage)); | ||
| ScaffoldMessenger.of(context).showSnackBar( | ||
| const SnackBar(content: Text('Copied to clipboard')), | ||
| ); | ||
| }, | ||
| ), | ||
| ), | ||
| const SizedBox(width: 12), | ||
| Expanded( | ||
| child: UiPrimaryButton( | ||
| text: 'Copy', | ||
| onPressed: () { | ||
| // TODO: Add share logic | ||
| }, |
There was a problem hiding this comment.
[P1] Copy/share actions miswired in signed message view
The confirmation result shows two buttons labeled “Share” and “Copy”, but the handlers are inverted and incomplete. The Share button copies the message to the clipboard and shows a snackbar, while the Copy button’s onPressed is an empty TODO, so tapping Copy does nothing and there is no way to actually share. Users will be unable to perform the primary action (copying the signed payload) and the labels are misleading.
Useful? React with 👍 / 👎.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Visit the preview URL for this PR (updated for commit 03e303d): https://walletrc--pull-3161-merge-xegsi9cs.web.app (expires Tue, 07 Oct 2025 08:04:29 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: f66a4ff03faa546f12f0ae5a841bd9eff2714dcc |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (1)
lib/views/wallet/coin_details/message_signing/Widgets/message_signed_result.dart (1)
84-107: Critical: Copy and Share button implementations are swapped.The "Share" button copies to clipboard (lines 89-93), while the "Copy" button has a TODO placeholder (line 102) and does nothing. This inverts the expected functionality and leaves users unable to copy the signed message properly.
Apply this diff to fix the button implementations:
Row( children: [ Expanded( child: UiSecondaryButton( - text: 'Share', + text: 'Copy', onPressed: () { Clipboard.setData(ClipboardData(text: signedMessage)); ScaffoldMessenger.of(context).showSnackBar( const SnackBar(content: Text('Copied to clipboard')), ); }, ), ), const SizedBox(width: 12), Expanded( child: UiPrimaryButton( - text: 'Copy', + text: 'Share', onPressed: () { - // TODO: Add share logic + // Add platform share logic if needed, or remove this button }, ), ), ], ),
🧹 Nitpick comments (10)
lib/shared/utils/utils.dart (1)
359-371: Cache the regex patterns to avoid repeated construction.The
suffixPatternandprefixPatternare reconstructed on every call toabbr2Ticker, even when the result is already cached. SincefilteredSuffixesandfilteredPrefixesare const, the patterns should be defined as static or top-level constants.Apply this diff to cache the regex patterns:
+// Regex patterns for ticker suffix/prefix filtering +final _suffixPattern = RegExp(r'-(ERC20|BEP20|QRC20|FTM20|ARB20|HRC20|MVR20|AVX20|HCO20|PLG20|KRC20|SLP|IBC_IRIS|IBC-IRIS|IRIS|segwit|OLD|IBC_NUCLEUSTEST)$'); +final _suffixPatternUnderscore = RegExp(r'_(ERC20|BEP20|QRC20|FTM20|ARB20|HRC20|MVR20|AVX20|HCO20|PLG20|KRC20|SLP|IBC_IRIS|IBC-IRIS|IRIS|segwit|OLD|IBC_NUCLEUSTEST)$'); +final _prefixPattern = RegExp(r'^(NFT)-'); +final _prefixPatternUnderscore = RegExp(r'^(NFT)_'); + String abbr2Ticker(String abbr) { if (_abbr2TickerCache.containsKey(abbr)) return _abbr2TickerCache[abbr]!; if (!abbr.contains('-') && !abbr.contains('_')) return abbr; - const List<String> filteredSuffixes = [ - 'ERC20', - 'BEP20', - 'QRC20', - 'FTM20', - 'ARB20', - 'HRC20', - 'MVR20', - 'AVX20', - 'HCO20', - 'PLG20', - 'KRC20', - 'SLP', - 'IBC_IRIS', - 'IBC-IRIS', - 'IRIS', - 'segwit', - 'OLD', - 'IBC_NUCLEUSTEST', - ]; - - const List<String> filteredPrefixes = ['NFT']; - - // Create regex patterns for both suffixes and prefixes - String suffixPattern = '(${filteredSuffixes.join('|')})'; - String prefixPattern = '(${filteredPrefixes.join('|')})'; - String ticker = abbr // Remove suffixes - .replaceAll(RegExp('-$suffixPattern'), '') - .replaceAll(RegExp('_$suffixPattern'), '') + .replaceAll(_suffixPattern, '') + .replaceAll(_suffixPatternUnderscore, '') // Remove prefixes - .replaceAll(RegExp('^$prefixPattern-'), '') - .replaceAll(RegExp('^${prefixPattern}_'), ''); + .replaceAll(_prefixPattern, '') + .replaceAll(_prefixPatternUnderscore, ''); _abbr2TickerCache[abbr] = ticker; return ticker; }Note: The regex patterns now use
$anchor for suffixes to ensure they only match at the end of the string, which is more precise than the original implementation.lib/views/wallet/coin_details/message_signing/message_signing_screen.dart (1)
76-79: Remove empty initState override.This
initStateoverride does nothing and can be removed to reduce code clutter.- @override - void initState() { - super.initState(); - } -lib/views/wallet/coin_details/message_signing/Widgets/message_signing_confirmation.dart (2)
17-25: Consider removing the theme parameter and using Theme.of(context).Passing
themeas a constructor parameter when it's readily available viaTheme.of(context)adds unnecessary coupling and makes the widget less flexible (it won't automatically adapt to theme changes in the widget tree).const MessageSigningConfirmationCard({ super.key, - required this.theme, required this.message, required this.coinAbbr, required this.understood, required this.onCancel, required this.onUnderstoodChanged, }); @override Widget build(BuildContext context) { + final theme = Theme.of(context); final selected = context.read<MessageSigningBloc>().state.selected;
73-78: UiCheckbox callback could be simplified.The
onChangedcallback wraps the parameter in a lambda that just forwards it. This can be passed directly.UiCheckbox( value: understood, - onChanged: (val) => onUnderstoodChanged(val), + onChanged: onUnderstoodChanged, text: LocaleKeys.messageSigningCheckboxText.tr(), textColor: theme.textTheme.bodyMedium?.color, ),lib/bloc/message_signing/message_signing_event.dart (2)
5-25: Consider making events immutable and equatable.Events currently lack:
constconstructors (where applicable)- Equatable implementation for value equality
- Consistent constructor style (mix of positional and named parameters)
Making events immutable and equatable improves testability and allows for event comparison and deduplication in tests.
+import 'package:equatable/equatable.dart'; import 'package:komodo_defi_types/komodo_defi_types.dart'; -sealed class MessageSigningEvent {} +sealed class MessageSigningEvent extends Equatable { + const MessageSigningEvent(); + + @override + List<Object?> get props => []; +} -class MessageSigningAddressesRequested extends MessageSigningEvent { +class MessageSigningAddressesRequested extends MessageSigningEvent { final Asset asset; - MessageSigningAddressesRequested(this.asset); + const MessageSigningAddressesRequested(this.asset); + + @override + List<Object?> get props => [asset]; } -class MessageSigningAddressSelected extends MessageSigningEvent { +class MessageSigningAddressSelected extends MessageSigningEvent { final PubkeyInfo address; - MessageSigningAddressSelected(this.address); + const MessageSigningAddressSelected(this.address); + + @override + List<Object?> get props => [address]; } -class MessageSigningFormSubmitted extends MessageSigningEvent { +class MessageSigningFormSubmitted extends MessageSigningEvent { final String message; final String coinAbbr; - MessageSigningFormSubmitted({ + const MessageSigningFormSubmitted({ required this.message, required this.coinAbbr, }); + + @override + List<Object?> get props => [message, coinAbbr]; } -class MessageSigningInputConfirmed extends MessageSigningEvent {} -class MessageSigningConfirmationCancelled extends MessageSigningEvent {} +class MessageSigningInputConfirmed extends MessageSigningEvent { + const MessageSigningInputConfirmed(); +} + +class MessageSigningConfirmationCancelled extends MessageSigningEvent { + const MessageSigningConfirmationCancelled(); +}
5-25: Inconsistent constructor parameter style across events.
MessageSigningFormSubmitteduses named parameters whileMessageSigningAddressesRequestedandMessageSigningAddressSelecteduse positional parameters. For consistency and clarity, consider using named parameters for all events with parameters.class MessageSigningAddressesRequested extends MessageSigningEvent { final Asset asset; - MessageSigningAddressesRequested(this.asset); + const MessageSigningAddressesRequested({required this.asset}); } class MessageSigningAddressSelected extends MessageSigningEvent { final PubkeyInfo address; - MessageSigningAddressSelected(this.address); + const MessageSigningAddressSelected({required this.address}); }lib/views/wallet/coin_details/message_signing/Widgets/message_signing_form.dart (4)
140-160: ErrorMessageWidget can use a const constructor.The widget is fully immutable and can be instantiated as const for better performance.
class ErrorMessageWidget extends StatelessWidget { final String errorMessage; const ErrorMessageWidget({ super.key, required this.errorMessage, }); @override Widget build(BuildContext context) { + final theme = Theme.of(context); return Padding( padding: const EdgeInsets.only(top: 16.0), child: Text( errorMessage, - style: Theme.of(context).textTheme.bodyMedium?.copyWith( - color: Theme.of(context).colorScheme.error, + style: theme.textTheme.bodyMedium?.copyWith( + color: theme.colorScheme.error, ), ), ); } }
100-105: Clipboard paste handler could provide user feedback on failure.If clipboard data is unavailable or not text, the paste button silently does nothing. Consider showing a brief message or snackbar to inform the user.
onCopyPressed: () async { final data = await Clipboard.getData('text/plain'); if (data?.text != null) { messageController.text = data!.text!; + } else { + if (context.mounted) { + ScaffoldMessenger.of(context).showSnackBar( + SnackBar( + content: Text(LocaleKeys.clipboardEmpty.tr()), + duration: const Duration(seconds: 2), + ), + ); + } } },
106-117: QR scanner button could benefit from better error handling.If the QR scanner overlay returns an error or the user denies camera permission, consider providing feedback to the user.
trailingIcon: UiPrimaryButton.flexible( onPressed: isSubmitting ? null : () async { - final result = - await QrCodeReaderOverlay.show(context); - if (result != null) { - messageController.text = result; + try { + final result = + await QrCodeReaderOverlay.show(context); + if (result != null) { + messageController.text = result; + } + } catch (e) { + if (context.mounted) { + ScaffoldMessenger.of(context).showSnackBar( + SnackBar( + content: Text( + LocaleKeys.qrScanError.tr(), + ), + ), + ); + } } }, child: const Icon(Icons.qr_code_scanner, size: 16), ),
183-183: Consider using widget.controller.text.length directly instead of late initialization.The
latekeyword with an initializer that depends onwidgetcan be error-prone. Consider initializingcharCountininitStateinstead.class _EnhancedMessageInputState extends State<EnhancedMessageInput> { - late int charCount = widget.controller.text.length; + int charCount = 0; @override void initState() { super.initState(); + charCount = widget.controller.text.length; widget.controller.addListener(_updateCharCount); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
lib/generated/codegen_loader.g.dartis excluded by!**/generated/**
📒 Files selected for processing (27)
assets/translations/en.json(1 hunks)docs/BLOC_NAMING_CONVENTIONS.md(1 hunks)lib/bloc/message_signing/message_signing_bloc.dart(1 hunks)lib/bloc/message_signing/message_signing_event.dart(1 hunks)lib/bloc/message_signing/message_signing_state.dart(1 hunks)lib/shared/utils/utils.dart(1 hunks)lib/views/bitrefill/bitrefill_button.dart(8 hunks)lib/views/common/page_header/page_header.dart(1 hunks)lib/views/dex/simple/form/tables/orders_table/grouped_list_view.dart(3 hunks)lib/views/dex/simple/form/tables/orders_table/orders_table_content.dart(1 hunks)lib/views/nfts/common/widgets/nft_connect_wallet.dart(1 hunks)lib/views/wallet/coin_details/coin_details.dart(2 hunks)lib/views/wallet/coin_details/coin_details_info/coin_details_common_buttons.dart(4 hunks)lib/views/wallet/coin_details/coin_details_info/coin_details_info.dart(1 hunks)lib/views/wallet/coin_details/coin_page_type.dart(1 hunks)lib/views/wallet/coin_details/message_signing/Widgets/message_signed_result.dart(1 hunks)lib/views/wallet/coin_details/message_signing/Widgets/message_signing_confirmation.dart(1 hunks)lib/views/wallet/coin_details/message_signing/Widgets/message_signing_form.dart(1 hunks)lib/views/wallet/coin_details/message_signing/Widgets/message_signing_header.dart(1 hunks)lib/views/wallet/coin_details/message_signing/message_signing_screen.dart(1 hunks)lib/views/wallet/coins_manager/coins_manager_switch_button.dart(1 hunks)lib/views/wallet/wallet_page/wallet_main/wallet_manage_section.dart(1 hunks)lib/views/wallets_manager/widgets/wallets_type_list.dart(1 hunks)packages/komodo_ui_kit/lib/src/buttons/ui_base_button.dart(1 hunks)packages/komodo_ui_kit/lib/src/buttons/ui_border_button.dart(5 hunks)packages/komodo_ui_kit/lib/src/buttons/ui_primary_button.dart(2 hunks)packages/komodo_ui_kit/lib/src/buttons/ui_secondary_button.dart(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-04-01T15:51:37.060Z
Learnt from: takenagain
PR: KomodoPlatform/komodo-wallet#2566
File: lib/bloc/coins_bloc/coins_bloc.dart:10-10
Timestamp: 2025-04-01T15:51:37.060Z
Learning: In the Komodo Wallet project, part files share imports with their parent files. The import for `app_config.dart` in `coins_bloc.dart` is necessary because the part file `coins_state.dart` uses `excludedAssetList` from that package.
Applied to files:
lib/views/dex/simple/form/tables/orders_table/grouped_list_view.dartlib/views/wallet/coin_details/coin_details.dart
🪛 markdownlint-cli2 (0.18.1)
docs/BLOC_NAMING_CONVENTIONS.md
72-72: Heading levels should only increment by one level at a time
Expected: h4; Actual: h5
(MD001, heading-increment)
⏰ 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). (4)
- GitHub Check: Build Mobile (Android)
- GitHub Check: Build Desktop (windows)
- GitHub Check: Build Desktop (linux)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (53)
lib/views/wallets_manager/widgets/wallets_type_list.dart (3)
6-6: LGTM: Modern constructor syntax adopted.The use of
super.keysimplifies the constructor and aligns with current Flutter best practices.
9-13: Good refactor: centralized exclusion list.Extracting the excluded wallet types into a static constant improves maintainability and makes it easy to add or remove types in the future. The addition of
WalletType.metamaskandWalletType.keplrto the exclusions aligns with the PR objectives.
18-18: LGTM: Filter logic updated correctly.The filter now uses the centralized exclusion list, correctly excluding all wallet types in
_excludedWalletTypes. The logic is sound and more maintainable than multiple inequality checks.lib/views/dex/simple/form/tables/orders_table/grouped_list_view.dart (4)
139-139: LGTM! Alias usage clarifies type origin.Using the
mm2.BestOrderalias improves clarity and avoids potential naming conflicts with otherBestOrdertypes in the codebase.
4-6: KomodoDefiSdk registration verified. KomodoDefiSdk is registered in GetIt in lib/services/initializer/app_bootstrapper.dart:41. No changes required.
108-115: Balance aggregation logic is correctThe fold correctly sums balances across grouped items with a safe
BalanceInfo.zero()fallback. Please verify that the externalkdf_rpc.BalanceInfotype provides azero()constructor/factory and implements the+operator.
117-120: Approve sendableBalance usageVerified that
Coin.copyWithincludes adouble? sendableBalanceparameter and that assigningtotalBalance.spendable.toDouble()correctly populates it.lib/views/dex/simple/form/tables/orders_table/orders_table_content.dart (1)
58-58: LGTM! Widget key improves testability.Adding a stable key to
GroupedListView<BestOrder>improves testability and helps Flutter manage the widget tree more predictably during rebuilds.lib/views/nfts/common/widgets/nft_connect_wallet.dart (1)
21-28: LGTM! Correct const removal.Removing
constfrom thePaddingconstructor is appropriate becauseConnectWalletButtonreceives non-const arguments (Size(double.infinity, 40)and runtime enum values). This aligns with proper const usage in Flutter.lib/views/wallet/coin_details/coin_page_type.dart (1)
1-1: LGTM! Clear enum extension.Adding
signMessagetoCoinPageTypeis a clean, additive change that enables routing to the new message-signing flow. The naming is consistent with existing values.lib/views/bitrefill/bitrefill_button.dart (4)
3-3: LGTM! Localization integration.Adding localization support improves the user experience by providing translatable tooltip messages. The usage of
LocaleKeys.zeroBalanceTooltip.tr()on line 139 properly handles the zero-balance case with localized text.Also applies to: 17-17, 139-139
97-99: LGTM! Dynamic key for widget identity.Adding a coin-specific key to
BitrefillInAppWebviewButtonensures proper widget identity when the coin changes. This is good practice for widget lifecycle management and testing.
119-144: LGTM! Improved tooltip logic.The enhanced
_getTooltipMessagemethod now accepts explicit parameters for balance and support status, making the logic clearer and more maintainable. The fallback to custom tooltip (line 124-126) preserves backward compatibility.
147-182: LGTM! Multi-address support implementation.The address selection flow is well-implemented:
- Checks for multiple addresses before showing the selector
- Uses
context.mountedto prevent stale context usage- Updates state with the selected refund address
- Reloads Bitrefill with the new address
The logic correctly handles both single-address and multi-address scenarios.
lib/views/wallet/coin_details/coin_details_info/coin_details_info.dart (1)
261-273: LGTM: Layout adjusted to accommodate new button.Replacing
Paddingwith a full-widthContainerensures the button row inCoinDetailsCommonButtons(which now includes the sign-message button) expands horizontally as intended. The padding is preserved and no logic changes.lib/views/common/page_header/page_header.dart (1)
87-93: LGTM: Removed const to support non-const child.The
Paddingwidget is now non-const becauseConnectWalletButtonis not a const widget. This change is necessary for header flexibility and aligns with the new dynamic UI components (e.g., message-signing flows).lib/views/wallet/coin_details/coin_details.dart (2)
14-14: LGTM: Import added for new message-signing screen.The import for
MessageSigningScreenis correctly added to support the new sign-message flow.
108-112: LGTM: Sign-message case integrated correctly.The new
CoinPageType.signMessagecase correctly instantiatesMessageSigningScreenwith the requiredcoinandonBackButtonPressedcallback. The navigation logic is consistent with other page types.lib/views/wallet/coin_details/coin_details_info/coin_details_common_buttons.dart (4)
441-486: LGTM: Sign-message button implementation is well-structured.The new
CoinDetailsMessageSigningButtonwidget correctly:
- Checks for available addresses and coin suspension status before enabling (lines 457-461, 480-482).
- Uses consistent styling and layout with other coin detail buttons.
- Invokes
selectWidget(CoinPageType.signMessage)to navigate to the signing screen.- Uses localized text via
LocaleKeys.signMessage.tr().
119-137: LGTM: Mobile layout integrates sign-message button correctly.The new button is inserted as a
Flexiblechild in the button row. The conditional layout (Bitrefill, sign-message, and swap buttons) is well-organized and maintains consistent spacing.
184-193: LGTM: Desktop layout integrates sign-message button correctly.The button is placed in a constrained
Container(max width 120px) with consistent margin, matching the layout pattern of other coin detail buttons.
483-483: Locale keysignMessageis defined. Confirmed presence in lib/generated/codegen_loader.g.dart (line 759).lib/views/wallet/coin_details/message_signing/message_signing_screen.dart (4)
81-103: Validation logic is clear and correct.The method properly validates that an address is selected and the message is non-empty before proceeding to confirmation. The user feedback via SnackBar is appropriate.
105-109: Proper resource disposal.The
TextEditingControlleris correctly disposed, preventing memory leaks.
138-149: Good defensive programming for address selection.The fallback to
state.addresses.firstwhenselectedis null ensures the UI gracefully handles edge cases. The additional null check before rendering the result view prevents potential crashes.
34-38: Move context access out of initState to avoid potential errors.Accessing
context.read<KomodoDefiSdk>()ininitStateis unsafe because the widget tree may not be fully built yet, and inherited widgets may not be accessible. This can lead to runtime exceptions.Move the SDK access and asset initialization to
didChangeDependenciesor retrieve the SDK inside thebuildmethod:- @override - void initState() { - super.initState(); - final sdk = context.read<KomodoDefiSdk>(); - asset = widget.coin.toSdkAsset(sdk); - } + @override + void didChangeDependencies() { + super.didChangeDependencies(); + final sdk = context.read<KomodoDefiSdk>(); + asset = widget.coin.toSdkAsset(sdk); + }Likely an incorrect or invalid review comment.
lib/bloc/message_signing/message_signing_bloc.dart (4)
8-18: Bloc structure and event registration are well-organized.The Bloc follows the standard pattern correctly: constructor initializes with an initial state and registers handlers for each event type. The event handler naming is clear and follows conventions.
20-42: Address loading logic is sound.The async event handler correctly:
- Emits loading state before the async operation
- Fetches pubkeys from the SDK
- Auto-selects the first address for better UX
- Handles errors and emits failure state
One minor improvement opportunity: the generic
e.toString()error message could be enhanced with a user-friendly localized fallback, but the current implementation is acceptable.
65-100: Message signing logic handles validation and errors appropriately.The handler:
- Validates the selected address before proceeding
- Emits proper state transitions (submitting → success/failure)
- Provides localized error messages with context
- Clears error messages on retry
51-63: Confirmation flow state transitions are correct.The handlers for confirming and cancelling confirmation properly update the status without side effects. This keeps the state machine simple and predictable.
lib/views/wallet/coin_details/message_signing/Widgets/message_signing_header.dart (1)
6-24: Simple, well-structured header widget.The widget correctly:
- Uses a const constructor for performance
- Delegates to the existing
PageHeadercomponent for consistency- Provides localized back button text
- Offers a clean, minimal API
lib/views/wallet/coin_details/message_signing/Widgets/message_signing_form.dart (1)
162-289: EnhancedMessageInput widget is well-structured.The stateful widget correctly manages its lifecycle:
- Adds listener in
initState- Removes listener in
dispose- Updates character count reactively
The implementation follows Flutter best practices for TextField integration.
lib/bloc/message_signing/message_signing_state.dart (2)
4-12: LGTM!The enum clearly represents the message signing flow lifecycle with semantically appropriate states.
14-35: LGTM!The state class follows best practices: immutable fields, const constructor, sensible defaults in the factory, and proper Equatable integration.
lib/views/wallet/coins_manager/coins_manager_switch_button.dart (1)
18-18: LGTM! Parameter renamed to align with Flutter conventions.The rename from
buttonKeytokeyfollows Flutter's standard naming pattern for widget keys and aligns with the broader UI kit API refactor in this PR.packages/komodo_ui_kit/lib/src/buttons/ui_border_button.dart (6)
24-46: LGTM! Flexible constructor follows good pattern.The
UiBorderButton.flexibleconstructor cleanly enforces size inheritance by settingwidthandheighttonullvia the initializer list. The default padding (16dp horizontal, 8dp vertical) and minimum dimensions (88x36) align with Material Design specifications for outlined buttons.
49-50: Note: Width and height are now nullable.These fields changed from
doubletodouble?to support the flexible constructor. This is a breaking change if external code directly accessed these fields, though typical usage via constructors remains backward compatible.
71-71: LGTM! Constraint logic centralized.Moving from explicit width/height constraints to the
_buildConstraints()method is a clean refactor that supports both fixed and flexible sizing patterns.
94-94: LGTM! Padding logic maintains backward compatibility.The null-coalescing correctly applies custom padding when provided or falls back to the default. The flexible constructor uses 16x8 padding while the regular constructor uses 12x6, preserving existing button appearance.
21-22: LGTM! Padding field enables customization.The optional
paddingfield allows callers to customize button padding while maintaining backward compatibility through nullable default behavior.Also applies to: 62-62
135-165: Approve: no height-only + allowMultiline usage found
No instances ofallowMultilinewith a height-only configuration exist in the codebase, so the_buildConstraintslogic is safe as-is.packages/komodo_ui_kit/lib/src/buttons/ui_base_button.dart (1)
3-13: LGTM! ButtonType enum correctly documents Material Design minimums.The enum covers the primary button types with accurate minimum dimension specifications from Material Design guidelines (text: 64x36, contained/outlined: 88x36, icon: 48x48).
packages/komodo_ui_kit/lib/src/buttons/ui_secondary_button.dart (4)
23-44: LGTM! Well-documented flexible constructor.The
UiSecondaryButton.flexibleconstructor follows the same pattern asUiPrimaryButton.flexible, with appropriate default padding and clear documentation. The Material Design minimum dimensions are correctly preserved.
48-49: LGTM! Nullable sizing fields enable flexible layout.Making
widthandheightnullable, along with the newpaddingandborderRadiusfields, provides the flexibility needed for content-driven button sizing while maintaining backward compatibility.Also applies to: 58-59
69-123: LGTM! Clean refactor using ButtonUtils.The build method correctly uses
ButtonUtils.getButtonConstraintsfor consistent sizing logic and only wraps inSizedBoxwhen explicit dimensions are provided. The conditional minimum size enforcement is appropriate.
154-160: LGTM! Correct content sizing.Setting
mainAxisSize: MainAxisSize.minallows the button content to size itself based on the text and prefix, which is essential for the flexible constructor to work correctly.packages/komodo_ui_kit/lib/src/buttons/ui_primary_button.dart (6)
8-32: LGTM! Good deprecation strategy.Marking the original constructor as deprecated while preserving backward compatibility is a sound approach. The deprecation notice clearly directs developers to use
UiPrimaryButton.flexibleinstead.
34-72: LGTM! Well-designed flexible constructor.The
UiPrimaryButton.flexibleconstructor provides sensible Material Design defaults and clear documentation. The TODO about eventually preferringchildovertextaligns with Flutter conventions.
116-127: LGTM! Well-documented optimistic UI feature.The optimistic enabled feature is clearly documented and provides a good user experience pattern for buttons that need to appear ready while waiting for enabling conditions.
133-177: LGTM! Robust timer lifecycle management.The loading state logic correctly handles all edge cases:
- Timer is properly cancelled in
dispose- When button becomes enabled during loading (in
didUpdateWidget), timer is cancelled andonPressedis called immediately_handlePresscorrectly prioritizes executingonPressedover showing loading stateThe implementation prevents timer leaks and provides good UX.
179-184: LGTM! Correct enabled state logic.The
_shouldAppearEnabledgetter correctly determines when the button should appear interactive, accounting for actual enablement, optimistic UI configuration, and loading state.
208-285: LGTM! Clean build method with loading state.The refactored build method:
- Uses
ButtonUtils.getButtonConstraintsfor consistent sizing- Correctly wires up
_handlePressfor optimistic UI handling- Shows a well-sized loading indicator (20x20 with 2px stroke) when in loading state
- Conditionally wraps in
SizedBoxonly when explicit dimensions are providedThe implementation is clean and follows Flutter best practices.
lib/views/wallet/wallet_page/wallet_main/wallet_manage_section.dart (1)
63-63: LGTM! Correct API migration.The change from
buttonKeytokeyreflects the updatedUiPrimaryButtonAPI, which now uses the standard Flutterkeyparameter instead of a custombuttonKeyparameter. This is more idiomatic and consistent with Flutter conventions.
| "securitySettings": "Security Settings", | ||
| "sendToAddress": "Only send {} to this address", | ||
| "message": "Message", | ||
| "signMessage": "Sign Message", | ||
| "selectedAddress": "Selected Address", | ||
| "selectAddress": "Select Address", | ||
| "messageToSign": "Message to Sign", | ||
| "enterMessage": "Enter message", | ||
| "signMessageButton": "Sign Message", | ||
| "signedMessage": "Signed Message", | ||
| "pleaseSelectAddress": "Please select an address first", | ||
| "pleaseEnterMessage": "Please enter a message to sign", | ||
| "failedToSignMessage": "Failed to sign message: {}", | ||
| "swapCoin": "Swap", | ||
| "komodoWalletSeed": "Komodo Wallet seed", | ||
| "failedToLoadAddresses": "Failed to load addresses: {}", | ||
| "sendFeedbackButton": "Share your feedback", | ||
| "allowCustomFee": "Allow custom seed", | ||
| "copyToClipboard": "Copy to clipboard", | ||
| "copyAllDetails": "Copy all details", | ||
| "zeroBalanceTooltip": "Insufficient balance to use Bitrefill", | ||
| "swapAddress": "Swap Address", | ||
| "userNotFoundError": "User not found", | ||
| "loginFailedError": "Login failed", | ||
| "previewWithdrawal": "Preview Withdrawal", | ||
| "chart": "Chart", | ||
| "confirmMessageSigning": "Confirm Message Signing", | ||
| "messageSigningWarning": "Only sign messages from trusted sources.", | ||
| "messageSigningCheckboxText": "I understand that signing proves ownership of this address.", | ||
| "messageSigned": "Message signed", | ||
| "addressLabel": "Address - ", | ||
| "signingAddress": "Signing address" | ||
| } |
There was a problem hiding this comment.
Verify translation text on line 784.
The key allowCustomFee has the value "Allow custom seed", which appears inconsistent. Based on the key name, this should likely read "Allow custom fee" instead of "Allow custom seed".
Apply this diff if the intention is to allow custom fees:
- "allowCustomFee": "Allow custom seed",
+ "allowCustomFee": "Allow custom fee",If the text is correct and refers to custom seeds, consider renaming the key to allowCustomSeed for clarity.
📝 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.
| "securitySettings": "Security Settings", | |
| "sendToAddress": "Only send {} to this address", | |
| "message": "Message", | |
| "signMessage": "Sign Message", | |
| "selectedAddress": "Selected Address", | |
| "selectAddress": "Select Address", | |
| "messageToSign": "Message to Sign", | |
| "enterMessage": "Enter message", | |
| "signMessageButton": "Sign Message", | |
| "signedMessage": "Signed Message", | |
| "pleaseSelectAddress": "Please select an address first", | |
| "pleaseEnterMessage": "Please enter a message to sign", | |
| "failedToSignMessage": "Failed to sign message: {}", | |
| "swapCoin": "Swap", | |
| "komodoWalletSeed": "Komodo Wallet seed", | |
| "failedToLoadAddresses": "Failed to load addresses: {}", | |
| "sendFeedbackButton": "Share your feedback", | |
| "allowCustomFee": "Allow custom seed", | |
| "copyToClipboard": "Copy to clipboard", | |
| "copyAllDetails": "Copy all details", | |
| "zeroBalanceTooltip": "Insufficient balance to use Bitrefill", | |
| "swapAddress": "Swap Address", | |
| "userNotFoundError": "User not found", | |
| "loginFailedError": "Login failed", | |
| "previewWithdrawal": "Preview Withdrawal", | |
| "chart": "Chart", | |
| "confirmMessageSigning": "Confirm Message Signing", | |
| "messageSigningWarning": "Only sign messages from trusted sources.", | |
| "messageSigningCheckboxText": "I understand that signing proves ownership of this address.", | |
| "messageSigned": "Message signed", | |
| "addressLabel": "Address - ", | |
| "signingAddress": "Signing address" | |
| } | |
| "allowCustomFee": "Allow custom fee", |
🤖 Prompt for AI Agents
In assets/translations/en.json around lines 767 to 799 (line 784 specifically),
the value for key "allowCustomFee" is "Allow custom seed" which is inconsistent
with the key name; update the value to "Allow custom fee" to match the key, or
if the text truly refers to seeds instead of fees, rename the key to
"allowCustomSeed" and update all usages accordingly.
|
|
||
| ✅ **Good** | ||
|
|
||
| ##### Subclasses |
There was a problem hiding this comment.
Fix heading level jump.
The heading on line 72 jumps from h3 to h5, skipping h4. This violates markdown best practices and can cause issues with document structure and accessibility.
Apply this diff to correct the heading level:
-##### Subclasses
+#### SubclassesAnd also fix line 76:
-##### Single Class
+#### Single Class📝 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.
| ##### Subclasses | |
| ++ b/docs/BLOC_NAMING_CONVENTIONS.md | |
| @@ -72,7 +72,7 @@ | |
| #### Subclasses | |
| @@ -76,7 +76,7 @@ | |
| #### Single Class |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
72-72: Heading levels should only increment by one level at a time
Expected: h4; Actual: h5
(MD001, heading-increment)
🤖 Prompt for AI Agents
In docs/BLOC_NAMING_CONVENTIONS.md around lines 72 and 76, the heading level
jumps incorrectly (h3 -> h5); change the "##### Subclasses" heading on line 72
to "#### Subclasses" to restore the proper h4 level under the preceding h3, and
adjust the heading on line 76 to the correct level as well so the document
heading hierarchy remains sequential (no skipped levels).
| MessageSigningState copyWith({ | ||
| List<PubkeyInfo>? addresses, | ||
| PubkeyInfo? selected, | ||
| String? signedMessage, | ||
| String? errorMessage, | ||
| MessageSigningStatus? status, | ||
| }) { | ||
| return MessageSigningState( | ||
| addresses: addresses ?? this.addresses, | ||
| selected: selected ?? this.selected, | ||
| signedMessage: signedMessage ?? this.signedMessage, | ||
| errorMessage: errorMessage ?? this.errorMessage, | ||
| status: status ?? this.status, | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Verify if nullable fields need explicit null-setting.
The copyWith implementation uses ?? fallback, which prevents explicitly setting nullable fields (selected, signedMessage, errorMessage) to null. For example, copyWith(selected: null) will retain the current selected value instead of clearing it.
If your Bloc logic requires explicitly nulling these fields during state transitions, consider using an Optional<T> wrapper or a sentinel pattern to distinguish "not provided" from "explicitly null".
Run the following script to check if any Bloc events or handlers attempt to explicitly null these fields:
🏁 Script executed:
#!/bin/bash
# Description: Search for copyWith calls in MessageSigningBloc that may attempt to explicitly set nullable fields to null.
# Search for copyWith calls in the message_signing bloc and related files
rg -n -A 2 -B 2 --type=dart 'copyWith\s*\(' lib/bloc/message_signing/Length of output: 4449
copyWith doesn’t support explicit null resets
In lib/bloc/message_signing/message_signing_state.dart (copyWith at lines 37–51), all nullable parameters fall back via param ?? this.field. As a result, calls like
emit(state.copyWith(status: MessageSigningStatus.loading, errorMessage: null));(message_signing_bloc.dart:24–26)emit(state.copyWith(selected: null));(message_signing_bloc.dart:48–49)emit(state.copyWith(status: MessageSigningStatus.submitting, errorMessage: null));(message_signing_bloc.dart:78–80)
will not clear those fields but instead retain their previous values. Refactor copyWith to distinguish “no override” from “override to null” (for example, by adding boolean flags per field, using an Optional<T> wrapper, or switching to a freezed-generated copyWith) so nullable fields can be explicitly set to null.
| text: 'Share', | ||
| onPressed: () { | ||
| Clipboard.setData(ClipboardData(text: signedMessage)); | ||
| ScaffoldMessenger.of(context).showSnackBar( | ||
| const SnackBar(content: Text('Copied to clipboard')), | ||
| ); | ||
| }, | ||
| ), | ||
| ), | ||
| const SizedBox(width: 12), | ||
| Expanded( | ||
| child: UiPrimaryButton( | ||
| text: 'Copy', | ||
| onPressed: () { | ||
| // TODO: Add share logic | ||
| }, | ||
| ), |
There was a problem hiding this comment.
Localize button labels and snackbar message.
The button text labels ('Share', 'Copy') and the snackbar message ('Copied to clipboard') are hard-coded English strings, while other UI text in this widget uses LocaleKeys for i18n. This creates inconsistency and prevents translation.
Add locale keys for these strings and use .tr():
child: UiSecondaryButton(
- text: 'Share',
+ text: LocaleKeys.share.tr(),
onPressed: () {
Clipboard.setData(ClipboardData(text: signedMessage));
ScaffoldMessenger.of(context).showSnackBar(
- const SnackBar(content: Text('Copied to clipboard')),
+ SnackBar(content: Text(LocaleKeys.copiedToClipboard.tr())),
);
},
),
),
const SizedBox(width: 12),
Expanded(
child: UiPrimaryButton(
- text: 'Copy',
+ text: LocaleKeys.copy.tr(),📝 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.
| text: 'Share', | |
| onPressed: () { | |
| Clipboard.setData(ClipboardData(text: signedMessage)); | |
| ScaffoldMessenger.of(context).showSnackBar( | |
| const SnackBar(content: Text('Copied to clipboard')), | |
| ); | |
| }, | |
| ), | |
| ), | |
| const SizedBox(width: 12), | |
| Expanded( | |
| child: UiPrimaryButton( | |
| text: 'Copy', | |
| onPressed: () { | |
| // TODO: Add share logic | |
| }, | |
| ), | |
| child: UiSecondaryButton( | |
| text: LocaleKeys.share.tr(), | |
| onPressed: () { | |
| Clipboard.setData(ClipboardData(text: signedMessage)); | |
| ScaffoldMessenger.of(context).showSnackBar( | |
| SnackBar(content: Text(LocaleKeys.copiedToClipboard.tr())), | |
| ); | |
| }, | |
| ), | |
| ), | |
| const SizedBox(width: 12), | |
| Expanded( | |
| child: UiPrimaryButton( | |
| text: LocaleKeys.copy.tr(), | |
| onPressed: () { | |
| // TODO: Add share logic | |
| }, | |
| ), | |
| ), |
🤖 Prompt for AI Agents
In
lib/views/wallet/coin_details/message_signing/Widgets/message_signed_result.dart
around lines 88 to 104, the 'Share', 'Copy' button labels and the SnackBar
message 'Copied to clipboard' are hard-coded; replace them with localized
strings (e.g. LocaleKeys.share, LocaleKeys.copy, LocaleKeys.copied_to_clipboard)
and call .tr() on each usage, update the SnackBar content to use the localized
text, ensure the file imports the localization extension (e.g.
easy_localization) if not already, and add the corresponding keys and
translations to your locale JSON/ARB files so translations exist for each
supported language.
|
|
||
| @override | ||
| Widget build(BuildContext context) { | ||
| final selected = context.read<MessageSigningBloc>().state.selected; |
There was a problem hiding this comment.
Consider using BlocBuilder or BlocSelector instead of context.read for state access.
Accessing context.read<MessageSigningBloc>().state.selected directly in the build method means this widget won't rebuild when the selected address changes. If the selected address can change while this confirmation card is visible, the displayed address will be stale.
Consider using BlocSelector or BlocBuilder to reactively rebuild when state.selected changes:
@override
Widget build(BuildContext context) {
- final selected = context.read<MessageSigningBloc>().state.selected;
-
- return Card(
+ return BlocSelector<MessageSigningBloc, MessageSigningState, PubkeyInfo?>(
+ selector: (state) => state.selected,
+ builder: (context, selected) {
+ return Card(
elevation: 8,
shape: RoundedRectangleBorder(
borderRadius: BorderRadius.circular(16),
side: BorderSide(
color: theme.colorScheme.primary.withOpacity(0.2),
),
),
+ ...
+ );
+ });📝 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.
| final selected = context.read<MessageSigningBloc>().state.selected; | |
| @override | |
| Widget build(BuildContext context) { | |
| - final selected = context.read<MessageSigningBloc>().state.selected; | |
| - | |
| return BlocSelector<MessageSigningBloc, MessageSigningState, PubkeyInfo?>( | |
| selector: (state) => state.selected, | |
| builder: (context, selected) { | |
| return Card( | |
| elevation: 8, | |
| shape: RoundedRectangleBorder( | |
| borderRadius: BorderRadius.circular(16), | |
| side: BorderSide( | |
| color: theme.colorScheme.primary.withOpacity(0.2), | |
| ), | |
| ), | |
| // …the rest of your Card’s child widgets go here, using `selected` as needed… | |
| ); | |
| }, | |
| ); | |
| } |
| onPressed: understood | ||
| ? () { | ||
| context.read<MessageSigningBloc>().add( | ||
| MessageSigningFormSubmitted( | ||
| message: message, | ||
| coinAbbr: coinAbbr, | ||
| ), | ||
| ); | ||
| onCancel(); | ||
| } | ||
| : null, |
There was a problem hiding this comment.
Potential race condition: onCancel called immediately after dispatching event.
Calling onCancel() immediately after dispatching MessageSigningFormSubmitted may close/hide the confirmation card before the bloc processes the signing request. If signing fails, the user won't see the error in context.
Consider one of these approaches:
- Let the parent screen listen to bloc state and navigate/dismiss on success/failure states
- Remove
onCancel()here and rely on bloc state changes to trigger navigation - Add a brief delay or await the event processing (though Bloc events are fire-and-forget by design)
Preferred approach: remove the onCancel() call and let the parent screen handle navigation based on MessageSigningStatus.success or MessageSigningStatus.failure:
child: UiPrimaryButton(
text: LocaleKeys.confirm.tr(),
onPressed: understood
? () {
context.read<MessageSigningBloc>().add(
MessageSigningFormSubmitted(
message: message,
coinAbbr: coinAbbr,
),
);
- onCancel();
}
: null,
),📝 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.
| onPressed: understood | |
| ? () { | |
| context.read<MessageSigningBloc>().add( | |
| MessageSigningFormSubmitted( | |
| message: message, | |
| coinAbbr: coinAbbr, | |
| ), | |
| ); | |
| onCancel(); | |
| } | |
| : null, | |
| onPressed: understood | |
| ? () { | |
| context.read<MessageSigningBloc>().add( | |
| MessageSigningFormSubmitted( | |
| message: message, | |
| coinAbbr: coinAbbr, | |
| ), | |
| ); | |
| } | |
| : null, |
🤖 Prompt for AI Agents
In
lib/views/wallet/coin_details/message_signing/Widgets/message_signing_confirmation.dart
around lines 92–102, calling onCancel() immediately after dispatching
MessageSigningFormSubmitted can close the confirmation UI before the bloc
finishes processing; remove the onCancel() invocation from this onPressed
handler so the confirmation remains visible, and ensure the parent screen
listens to MessageSigningBloc state (MessageSigningStatus.success / .failure) to
dismiss or navigate only after a terminal state is emitted.
| mainAxisAlignment: MainAxisAlignment.end, | ||
| children: [ | ||
| Text( | ||
| '$charCount characters', |
There was a problem hiding this comment.
Hardcoded string should be localized.
The text "characters" is hardcoded and won't be translated for users in other locales. Consider adding a localization key.
Text(
- '$charCount characters',
+ LocaleKeys.characterCount.tr(args: [charCount.toString()]),
style: theme.textTheme.bodySmall?.copyWith(
color: theme.colorScheme.onSurface.withOpacity(0.6),
),
),Or with plural support:
Text(
- '$charCount characters',
+ LocaleKeys.characterCount.plural(charCount),
style: theme.textTheme.bodySmall?.copyWith(
color: theme.colorScheme.onSurface.withOpacity(0.6),
),
),Remember to add the corresponding key to your locale files.
📝 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.
| '$charCount characters', | |
| Text( | |
| LocaleKeys.characterCount.tr(args: [charCount.toString()]), | |
| style: theme.textTheme.bodySmall?.copyWith( | |
| color: theme.colorScheme.onSurface.withOpacity(0.6), | |
| ), | |
| ), |
🤖 Prompt for AI Agents
In
lib/views/wallet/coin_details/message_signing/Widgets/message_signing_form.dart
around line 278, the string literal '$charCount characters' is hardcoded;
replace it with a localized/pluralized lookup (e.g. use your Intl/localizations
class or context.l10n) that supports singular/plural forms and injects charCount
into the formatted string, then add the corresponding key(s) to the locale
ARB/JSON files (both singular and plural or an Intl plural entry) so
translations can be provided.
| // For backward compatibility, if explicit dimensions are provided or | ||
| // we're not enforcing minimum size, use the provided dimensions directly | ||
| if (!shouldEnforceMinimumSize || (width != null && height != null)) { | ||
| if (width != null && height != null) { | ||
| return BoxConstraints.tightFor(width: width, height: height); | ||
| } else if (width != null) { | ||
| return BoxConstraints(minWidth: width); | ||
| } else if (height != null) { | ||
| return BoxConstraints(minHeight: height); | ||
| } else if (expandToFillParent) { | ||
| // If we're expanding to fill parent and no other constraints are set, | ||
| // make sure we at least apply minimum height | ||
| return const BoxConstraints(minHeight: 36); | ||
| } else { | ||
| return const BoxConstraints(); | ||
| } | ||
| } | ||
|
|
||
| // Only apply Material Design minimum dimensions for flexible constructors | ||
| // when shouldEnforceMinimumSize is true | ||
| double minWidth; | ||
| double minHeight; | ||
|
|
||
| // Determine minimum dimensions based on button type | ||
| switch (buttonType) { | ||
| case ButtonType.text: | ||
| minWidth = 64; | ||
| minHeight = 36; | ||
| break; | ||
| case ButtonType.containedOrOutlined: | ||
| minWidth = 88; | ||
| minHeight = 36; | ||
| break; | ||
| case ButtonType.icon: | ||
| minWidth = 48; | ||
| minHeight = 48; | ||
| break; | ||
| } | ||
|
|
||
| // For flexible constructors, use constraints that allow the button to grow | ||
| // beyond the minimum dimensions while still respecting the minimums | ||
| if (expandToFillParent) { | ||
| return BoxConstraints( | ||
| minWidth: double.infinity, | ||
| minHeight: minHeight, | ||
| ); | ||
| } else { | ||
| return BoxConstraints( | ||
| minWidth: minWidth, | ||
| minHeight: minHeight, | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Verify handling of explicit dimensions when shouldEnforceMinimumSize is true.
The constraint logic has a potential issue: when shouldEnforceMinimumSize is true and only one dimension (width OR height) is explicitly provided, that dimension is ignored and Material Design minimums are used instead.
For example:
ButtonUtils.getButtonConstraints(
width: 200,
shouldEnforceMinimumSize: true,
)
// Returns minWidth: 88, minHeight: 36 (ignores explicit width: 200)This may be unexpected. Consider whether explicit dimensions should take precedence even when shouldEnforceMinimumSize is true, or document this behavior clearly.
The current logic at line 27 only enters the backward-compatibility path if:
!shouldEnforceMinimumSizeOR- BOTH
widthandheightare non-null
Clarify the intended behavior: should an explicit width or height override Material minimums when shouldEnforceMinimumSize is true, or should Material minimums always win in that mode?
If explicit dimensions should be respected, apply this fix:
- if (!shouldEnforceMinimumSize || (width != null && height != null)) {
+ if (!shouldEnforceMinimumSize) {
if (width != null && height != null) {
return BoxConstraints.tightFor(width: width, height: height);
} else if (width != null) {Then handle the Material minimum enforcement separately by using max(width, minWidth) and max(height, minHeight) when shouldEnforceMinimumSize is true.
🏁 Script executed:
#!/bin/bash
# Locate the ui_base_button.dart file
fd ui_base_button.dart
# Show signature and implementation of getButtonConstraints
rg -n 'getButtonConstraints' -C5 packages/komodo_ui_kit/lib/src/buttons/ui_base_button.dartLength of output: 449
🏁 Script executed:
#!/bin/bash
sed -n '25,77p' packages/komodo_ui_kit/lib/src/buttons/ui_base_button.dartLength of output: 1748
Fix logic to honor single explicit width/height when enforcing minimum size
Lines 27–41: the condition if (!shouldEnforceMinimumSize || (width != null && height != null)) causes a call like width:200, shouldEnforceMinimumSize:true to fall back to Material minimums (minWidth = 88) and ignore the explicit width. Either extend that branch to handle width != null or height != null, or after computing minWidth/minHeight, use
minWidth = width != null ? max(width, minWidth) : minWidth;
minHeight = height != null ? max(height, minHeight) : minHeight;so single-dimension overrides are respected.
🤖 Prompt for AI Agents
In packages/komodo_ui_kit/lib/src/buttons/ui_base_button.dart around lines
25–77, the current condition `if (!shouldEnforceMinimumSize || (width != null &&
height != null))` causes a single explicit width or height to be ignored when
shouldEnforceMinimumSize is true; update the logic so single-dimension overrides
are respected: either extend the early-return branch to handle width!=null or
height!=null cases (return tight or min constraints for that single dimension,
preserving expandToFillParent handling), or keep the existing branch but after
computing minWidth/minHeight apply the overrides using minWidth = width != null
? max(width, minWidth) : minWidth and minHeight = height != null ? max(height,
minHeight) : minHeight (and adjust the expandToFillParent branch to use the
overridden minHeight/minWidth appropriately).
Re-opening #3160 using internal branch
KomodoPlatform:feat/kyc-signing-ui-bloc. Supersedes the fork-based PR. Original description and discussion are in #3160.Note
Adds a full message-signing flow (BLoC, UI, i18n) integrated into coin details, and refactors UI kit buttons with flexible sizing and optimistic enabled behavior, plus minor DX/UX tweaks.
MessageSigningBloc, events, and state.MessageSigningScreenwith form, confirmation, and result widgets; navigable via newCoinPageType.signMessage.LocaleKeys.UiPrimaryButton,UiSecondaryButton, andUiBorderButtonwith flexible constructors, Material min-size constraints, and optional optimistic-enabled loading; introduceButtonUtils.mm2.BestOrder; add stable key for orders table.WalletsTypeList.docs/BLOC_NAMING_CONVENTIONS.md.Written by Cursor Bugbot for commit 03e303d. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
New Features
Improvements
Documentation