Introduced max editable amounts #584
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
WalkthroughAdds a currency-specific max amount constant and uses it in the Widget numeric input to restrict value and decimal precision; updates NumericFormat Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ 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)
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: 0
🧹 Nitpick comments (2)
react/lib/util/constants.ts (1)
11-14: MAX_AMOUNT values are consistent with existing currency configThe per-currency caps line up with current decimal precision (8 for BCH, 2 for XEC) and give sensible upper bounds against pathological inputs. If you later expand the currency set, you might consider tightening the typing to something like
Record<CryptoCurrency, number>so missing entries are caught at compile time, but what you have is fine for now.react/lib/components/Widget/Widget.tsx (1)
27-27: Max-amount clamping and numeric input props look correct; consider centralizing clamp logic
- Importing
MAX_AMOUNTand clamping inhandleAmountChangeensures user-edited amounts can’t exceed the per-currency caps, which directly addresses the “huge editable amount” issue.- Using
type="number"withmin: 0,max: MAX_AMOUNT[thisAddressType] || MAX_AMOUNT.XEC, and BCH‑specificstep(1e‑8 vs 0.01) is consistent with your existing decimal configuration and helps keep the UI in a sane range.- Note that only this input path applies the clamp; other paths that set
thisAmount(e.g., viaprops.amountor viaupdateAmountfrom child components) can still provide values aboveMAX_AMOUNT. If you want a stronger invariant, consider extracting a small helper (e.g.,clampToMaxAmount(thisAddressType, rawAmount)) and reusing it whereverthisAmountis set, but that’s an optional hardening step rather than a blocker.Also applies to: 865-878, 1051-1058
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
react/lib/components/Widget/Widget.tsx(3 hunks)react/lib/util/constants.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
react/lib/components/Widget/Widget.tsx (2)
react/lib/util/constants.ts (1)
MAX_AMOUNT(11-14)react/lib/util/format.ts (1)
amount(5-12)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
package.json (1)
27-28:clean:watchcomposition is sound; be aware it's a heavy operationChaining
clean:buildthenwatchis correct and predictable, and the script looks good. Just keep in mind it will nuke node_modules/dist and do a full reinstall+build before starting dev, so it’s best reserved for “start from scratch” scenarios rather than routine watch usage.react/lib/util/constants.ts (2)
11-14: STEP_AMOUNT values align with DECIMALS; optional DRY-up opportunityThe BCH/XEC step values correctly match their declared decimal precision (10^-8 and 10^-2), so this is consistent with existing DECIMALS and suitable for numeric inputs.
If you want to reduce duplication in the future, you could derive step at use‑site from
DECIMALS(e.g.,10 ** -DECIMALS[symbol]) instead of maintaining a separate map, but that’s purely a convenience refactor.
16-19: MAX_AMOUNT bounds look reasonable; consider centralizing per‑currency config laterThe BCH and XEC max amounts are within a safe numeric range for JS and match the intent of preventing unrealistic values, so they’re fine as standalone constants.
Longer term, you might want to group per‑currency settings (decimals, step, min/max, etc.) into a single typed config object per currency to avoid spreading several parallel maps across this file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
package.json(1 hunks)react/lib/components/Widget/Widget.tsx(4 hunks)react/lib/util/constants.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- react/lib/components/Widget/Widget.tsx
⏰ 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: Run build
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
react/lib/components/Widget/Widget.tsx (2)
1203-1214: Well-implemented max amount validation.The
isAllowedvalidation correctly constrains user input to the currency-specific maximum. The fallback toMAX_AMOUNT.XECis reasonable.Consider adding defensive handling if
thisAddressTypecould be undefined or an unexpected value:isAllowed={ (values) => { const { floatValue } = values + const maxForType = MAX_AMOUNT[thisAddressType ?? 'XEC'] ?? MAX_AMOUNT.XEC return ( floatValue === undefined || ( - floatValue <= (MAX_AMOUNT[thisAddressType] ?? MAX_AMOUNT.XEC) && + floatValue <= maxForType && floatValue >= 0 ) ) } }
1217-1219: Minor: Additional attributes unrelated to PR objective.The
name,placeholder, andidattributes are standard form field improvements but weren't mentioned in the PR objectives. While these are acceptable additions, the placeholder text "Enter Amount" duplicates the label.Consider removing the redundant placeholder or making it more specific:
name="Amount" - placeholder="Enter Amount" + placeholder={`0.00${STEP_AMOUNT[thisAddressType] === STEP_AMOUNT.BCH ? '000001' : ''}`} id="userEditedAmount"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
react/lib/components/Widget/Widget.tsx(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
react/lib/components/Widget/Widget.tsx (1)
react/lib/util/constants.ts (2)
MAX_AMOUNT(16-19)STEP_AMOUNT(11-14)
⏰ 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: Run build
🔇 Additional comments (2)
react/lib/components/Widget/Widget.tsx (2)
19-19: LGTM!The imports for
STEP_AMOUNTandMAX_AMOUNTare correctly added to support the maximum editable amount feature.Also applies to: 28-28
1228-1239: LGTM! Button styling improvements.The Confirm button styling has been refactored to use inline
sxprop with theme values, improving maintainability. The visibility logic and functionality remain unchanged.
Related to #582
Test plan
Create a widget/button w/ editable=true, then try to enter 9999999999999999999999999999 and see that it stops you.
Greptile Overview
Greptile Summary
Implemented maximum amount limits for the editable amount field to prevent users from entering unrealistic cryptocurrency values. The limits are set to 999,999.99999999 BCH and 999,999,999,999.99 XEC, aligned with the related issue requirements.
Key Changes:
MAX_AMOUNTconstant inconstants.tsdefining currency-specific maximum valuesTextFieldcomponent to usetype="number"withmax,min, andstepattributes for better UXhandleAmountChangeto clamp values exceeding the maximumThe implementation provides both browser-level validation hints (via HTML5 attributes) and JavaScript-level enforcement (via the onChange handler) to ensure amounts stay within acceptable ranges.
Confidence Score: 4/5
maxLengthconstraint, which could allow users to briefly type very long numbers before they're clamped by validation.Important Files Changed
File Analysis
MAX_AMOUNTconstant defining maximum allowed amounts for BCH (999,999.99999999) and XEC (999,999,999,999.99)handleAmountChangehandler and TextField inputProps, changed input type to number with min/max/step attributesSequence Diagram
sequenceDiagram participant User participant TextField participant handleAmountChange participant MAX_AMOUNT participant State User->>TextField: Types amount in editable field TextField->>TextField: Validates with max/min/step attributes TextField->>handleAmountChange: onChange event with value handleAmountChange->>handleAmountChange: Check if empty, set to "0" handleAmountChange->>MAX_AMOUNT: Get max for thisAddressType handleAmountChange->>handleAmountChange: Compare +amount > maxAmount alt Amount exceeds max handleAmountChange->>handleAmountChange: Clamp to maxAmount.toString() end handleAmountChange->>State: setUserEditedAmount(getCurrencyObject) handleAmountChange->>State: updateAmount(clamped value) State->>TextField: Re-render with clamped valueSummary by CodeRabbit
Bug Fixes
Improvements
Chores
✏️ Tip: You can customize this high-level summary in your review settings.