Improve payment methods: expand list, always-visible custom input, and fix submit validation#455
Improve payment methods: expand list, always-visible custom input, and fix submit validation#455
Conversation
- Remove Other checkbox requirement to show custom input - Custom text field always visible below payment method selector - Remove showCustomField parameter and _showCustomPaymentMethod state - Simplify onMethodsChanged callback signature - Match custom field UI style with amount input (fontSize, borders, margins)
- Add _minFiatAmount null check in _getSubmitCallback() - Prevents null check error when submitting without entering an amount - Button stays disabled until currency, amount, and payment method are filled
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughRestructures payment methods data by adding and extending many currency entries; simplifies payment-method UI by removing the custom-field visibility flag and changing the Changes
Sequence Diagram(s)(omitted — changes are UI/data-focused and do not introduce new multi-component sequential control flow) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
lib/features/order/screens/add_order_screen.dart (2)
430-432:⚠️ Potential issue | 🟠 MajorBug: custom-only payment method won't enable submit.
With the "Other" option removed and the custom field always visible, a user may reasonably enter only a custom payment method without selecting any from the dialog. However, this check returns
null(disabling submit) when_selectedPaymentMethodsis empty, even if_customPaymentMethodControllerhas valid text.Proposed fix
- if (_selectedPaymentMethods.isEmpty) { + if (_selectedPaymentMethods.isEmpty && + _customPaymentMethodController.text.trim().isEmpty) { return null; }Apply the same logic to the guard in
_submitOrderat line 452:- if (_selectedPaymentMethods.isEmpty) { + if (_selectedPaymentMethods.isEmpty && + _customPaymentMethodController.text.trim().isEmpty) {
504-518:⚠️ Potential issue | 🟠 MajorSanitization logic looks correct, but submit reactivity may be stale for custom input.
The sanitization of the custom payment method is sound. However, since
_getSubmitCallback()is evaluated duringbuild(), and_customPaymentMethodControllerchanges don't triggersetState, the submit button won't reactively enable/disable as the user types in the custom field. If you fix the validation check above, you'll also need to listen to the controller to rebuild when custom text changes.Proposed approach — add a listener in initState
`@override` void initState() { super.initState(); + _customPaymentMethodController.addListener(() { + setState(() {}); + }); ...
🤖 Fix all issues with AI agents
In `@assets/data/payment_methods.json`:
- Around line 2-23: The payment_methods.json contains mixed Spanish/English
user-facing strings causing localization gaps; update the data and widget so all
displayed strings are localized: convert all payment method entries to a
canonical (English) key set in payment_methods.json, then extend
_translatePaymentMethod() (or replace it with ARB keys looked up via
S.of(context)) to map those canonical keys to localized display strings;
alternatively, if these are intentional brand/service names, document that
choice and ensure the widget uses S.of(context) or a pass-through for brand
names so no untranslated Spanish literals are shown.
In `@lib/features/order/widgets/payment_methods_section.dart`:
- Around line 147-163: The dialog can appear empty when availableMethods is
empty; update the dialog logic that builds the CheckboxListTiles (using
availableMethods, dialogSelectedMethods, setDialogState and CheckboxListTile) to
guard against an empty list by either providing a sensible fallback list (ensure
availableMethods contains at least one entry beyond "Other") or rendering a
placeholder/notice widget and disabling the Confirm action when
availableMethods.isEmpty; implement the guard where the children are built so
the dialog shows a helpful message or disabled confirm button instead of a blank
content area.
🧹 Nitpick comments (2)
lib/features/order/widgets/payment_methods_section.dart (2)
45-57: Always-visible custom payment field — clean simplification.This removes the need for the "Other" toggle and provides a better UX. The
extraContentplacement withinFormSectionkeeps it visually grouped with the payment methods.Minor nit: line 54 can use
const.Nit
- hintStyle: TextStyle(color: Colors.grey, fontSize: 13), + hintStyle: const TextStyle(color: Colors.grey, fontSize: 13),
69-78: Default fallback after "Other" filtering may leave only two items — verify this is acceptable.The default list in JSON is
["Bank Transfer", "Cash in person", "Other"]. After filtering "Other" at line 120, the default becomes["Bank Transfer", "Cash in person"](translated). This is fine functionally, but if the intent was always to have at least a few options, consider adding more defaults to the JSON.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@assets/data/payment_methods.json`:
- Line 2: The "AED" entry in assets/data/payment_methods.json contains
incorrect, non-UAE banks (the "AED" array currently includes "MBank", "Emirates
NBD", "DBS", "Attijariwafa Bank", "Brubank", "Cash"); update the AED array to
only include UAE-relevant payment methods (e.g., banks like "First Abu Dhabi
Bank (FAB)", "Abu Dhabi Commercial Bank (ADCB)", "Mashreq", and common UAE
payment options like "Apple Pay", "Samsung Pay", "Cash", or local wallets) so
the AED key reflects accurate UAE payment methods.
- Line 22: The "TRY" payment methods array is incorrect and appears copied from
another currency; update the "TRY" array to include relevant Turkish payment
options such as "Papara", "İninal", "Garanti BBVA", "İşBank" or "İş Bankası",
"Ziraat Bankası", "Akbank", "VakıfBank" (and keep generic options like "Bank
Transfer" and "Cash" if desired). Locate the "TRY" JSON array in
payment_methods.json and replace the current values with the appropriate Turkish
payment method strings, preserving JSON syntax and ordering.
- Line 25: Remove "Other" from the JSON fallback list under the "default" key in
assets/data/payment_methods.json so it matches the UI behavior that strips
"Other"; update the "default" array to only include ["Bank Transfer", "Cash in
person"] (this ensures consistency with the logic in PaymentMethodsProvider /
payment_methods_provider.dart and the UI handling in PaymentMethodsSection and
OrderFilter where "Other" is explicitly removed).
- Check custom text field in addition to selected methods for submit validation - Add listener to update button state as user types custom payment method
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/features/order/providers/payment_methods_provider.dart (1)
22-24:⚠️ Potential issue | 🟡 MinorRemove the dead "Other" injection — it's bypassed by the UI layer anyway.
The provider adds
'Other'to every currency's method list (lines 22-24), but the widget (payment_methods_section.dart) usespaymentMethodsDataProviderdirectly instead and explicitly filters out "Other" at lines 118-122 because the custom text field is always visible. This code has no effect and contradicts the design; remove these three lines.♻️ Suggested fix
data: (data) { if (data.containsKey(currencyCode)) { - final methods = List<String>.from(data[currencyCode]); - - if (!methods.contains('Other')) { - methods.add('Other'); - } - return methods; + return List<String>.from(data[currencyCode]); } else {
|
@Catrya Why removing other payment method? |
2 participants
fix #433
INR, CHF, MYR) and expand methods for existing ones
Summary by CodeRabbit
New Features
Improvements