Feat/offramp fee breakdown 62#63
Open
AvatarMiiDe wants to merge 3 commits intoFundable-Protocol:mainfrom
Open
Conversation
Implemented visual fee breakdown, exchange rate display, and estimated arrival time to satisfy task requirements.
Added a detailed transaction summary, exchange rate calculation, and estimated arrival time to the success modal for better user transparency.
Contributor
📝 WalkthroughWalkthroughUpdates the offramp UI: introduces exchange-rate and fee breakdown rows, standardizes total payout presentation, adds estimated processing-time banner and adjusted processing/failure status handling, and refreshes layout, styling, and copy in the summary and success modal components. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
apps/web/src/components/offramp/OfframpSummary.tsx (1)
68-69: Normalize monetary precision across the breakdown rows.Line 68, Line 77, and Line 121 use different formatting styles (raw values vs fixed decimals), which can show inconsistent precision for money values.
💡 Suggested refactor
+ const formatAmount = (value: number, digits = 2) => + value.toLocaleString(undefined, { + minimumFractionDigits: digits, + maximumFractionDigits: digits, + }); ... - 1 {formState.token} = {quote.currency} {quote.rate?.toLocaleString()} + 1 {formState.token} = {quote.currency} {formatAmount(quote.rate, 4)} ... - -{quote.fee} {quote.currency} + -{formatAmount(quote.fee)} {quote.currency} ... - {quote.fiatAmount?.toLocaleString(undefined, { minimumFractionDigits: 2, maximumFractionDigits: 2 })} + {formatAmount(quote.fiatAmount)}Also applies to: 77-78, 121-122
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/offramp/OfframpSummary.tsx` around lines 68 - 69, The money values in OfframpSummary.tsx (where JSX outputs formState.token, quote.currency and quote.rate) use inconsistent formatting across the breakdown rows (lines around where the JSX renders quote.rate and other quote values). Normalize monetary precision by applying a single formatting utility (e.g., a shared formatMoney or Intl.NumberFormat call) wherever those values are rendered (references: formState.token, quote.currency, quote.rate) so all rows use the same fixed decimal/locale formatting; replace raw value outputs with a call to that formatter in each JSX location that shows monetary amounts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/components/offramp/OfframpSuccessModal.tsx`:
- Around line 48-50: The code marks payoutStatus?.status === "expired" as
processing; update the logic so "expired" is treated as a terminal state rather
than processing — e.g., expand the terminal/failure checks used by
isCompleted/isFailed (or add an isExpired flag) so that payoutStatus?.status ===
"expired" is not included in isProcessing; adjust isFailed or isCompleted
accordingly in the OfframpSuccessModal component so the spinner/copy is not
shown for expired payouts.
- Around line 90-95: Replace the hardcoded "Estimated: 2-5 Minutes" text with
the ETA provided by the BridgeFeeBreakdown object: read
BridgeFeeBreakdown.estimatedTime (or props.bridgeFeeBreakdown?.estimatedTime)
inside the isProcessing block and render that value instead (e.g., `Estimated:
{estimatedTime}`), with a sensible fallback (keep "2-5 Minutes" or "N/A") if
estimatedTime is missing; update the span rendering in OfframpSuccessModal (the
isProcessing / Clock block) so it formats the provided estimatedTime safely.
- Around line 154-156: The icon-only copy button in OfframpSuccessModal lacks an
accessible label; update the <button> rendered near the copied variable and
handleCopy handler to include an accessible name (e.g., aria-label="Copy code"
or aria-label that toggles to "Copied" when copied is true) or add visually
hidden text inside the button so screen readers can announce the action; ensure
the change references the same button that conditionally renders CheckCircle2
and Copy so behavior and styling remain unchanged.
- Around line 119-125: OfframpSuccessModal currently recomputes the exchange
rate inline from feeBreakdown fields (parseFloat(feeBreakdown.fiatPayout) /
(parseFloat(feeBreakdown.sendAmount) - parseFloat(feeBreakdown.bridgeFee)))
which can yield NaN/Infinity and diverge from the backend; instead, use a
canonical exchange rate value provided by the backend (e.g.,
feeBreakdown.exchangeRate) or add an exchangeRate prop to OfframpSuccessModal,
display that value in the Exchange Rate span, and include a defensive fallback
(e.g., empty string or "—") if the canonical value is missing/invalid to avoid
inline arithmetic in render.
---
Nitpick comments:
In `@apps/web/src/components/offramp/OfframpSummary.tsx`:
- Around line 68-69: The money values in OfframpSummary.tsx (where JSX outputs
formState.token, quote.currency and quote.rate) use inconsistent formatting
across the breakdown rows (lines around where the JSX renders quote.rate and
other quote values). Normalize monetary precision by applying a single
formatting utility (e.g., a shared formatMoney or Intl.NumberFormat call)
wherever those values are rendered (references: formState.token, quote.currency,
quote.rate) so all rows use the same fixed decimal/locale formatting; replace
raw value outputs with a call to that formatter in each JSX location that shows
monetary amounts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 408caa43-6964-4042-8ced-b2234ae69932
📒 Files selected for processing (2)
apps/web/src/components/offramp/OfframpSuccessModal.tsxapps/web/src/components/offramp/OfframpSummary.tsx
Comment on lines
+90
to
+95
| {isProcessing && ( | ||
| <div className="inline-flex items-center gap-2 px-3 py-1 bg-fundable-purple/10 border border-fundable-purple/20 rounded-full"> | ||
| <Clock size={12} className="text-fundable-purple" /> | ||
| <span className="text-[10px] font-bold text-fundable-purple uppercase tracking-wider"> | ||
| Estimated: 2-5 Minutes | ||
| </span> |
Contributor
There was a problem hiding this comment.
Use API-provided ETA instead of hardcoded minutes.
Line 94 hardcodes 2-5 Minutes even though BridgeFeeBreakdown provides estimatedTime. This can present inaccurate expectations.
✅ Suggested fix
- Estimated: 2-5 Minutes
+ Estimated: {feeBreakdown?.estimatedTime ?? 5} Minutes📝 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.
Suggested change
| {isProcessing && ( | |
| <div className="inline-flex items-center gap-2 px-3 py-1 bg-fundable-purple/10 border border-fundable-purple/20 rounded-full"> | |
| <Clock size={12} className="text-fundable-purple" /> | |
| <span className="text-[10px] font-bold text-fundable-purple uppercase tracking-wider"> | |
| Estimated: 2-5 Minutes | |
| </span> | |
| {isProcessing && ( | |
| <div className="inline-flex items-center gap-2 px-3 py-1 bg-fundable-purple/10 border border-fundable-purple/20 rounded-full"> | |
| <Clock size={12} className="text-fundable-purple" /> | |
| <span className="text-[10px] font-bold text-fundable-purple uppercase tracking-wider"> | |
| Estimated: {feeBreakdown?.estimatedTime ?? 5} Minutes | |
| </span> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/offramp/OfframpSuccessModal.tsx` around lines 90 -
95, Replace the hardcoded "Estimated: 2-5 Minutes" text with the ETA provided by
the BridgeFeeBreakdown object: read BridgeFeeBreakdown.estimatedTime (or
props.bridgeFeeBreakdown?.estimatedTime) inside the isProcessing block and
render that value instead (e.g., `Estimated: {estimatedTime}`), with a sensible
fallback (keep "2-5 Minutes" or "N/A") if estimatedTime is missing; update the
span rendering in OfframpSuccessModal (the isProcessing / Clock block) so it
formats the provided estimatedTime safely.
Comment on lines
+119
to
+125
| <div className="flex justify-between items-center text-[11px]"> | ||
| <span className="text-gray-500 flex items-center gap-1"> | ||
| Exchange Rate <Info size={10} /> | ||
| </span> | ||
| <span className="text-gray-400 font-mono"> | ||
| 1 USDC = ₦{(parseFloat(feeBreakdown.fiatPayout) / (parseFloat(feeBreakdown.sendAmount) - parseFloat(feeBreakdown.bridgeFee))).toLocaleString()} | ||
| </span> |
Contributor
There was a problem hiding this comment.
Avoid recalculating exchange rate in render; use canonical value.
Line 124 recomputes rate via division on parsed strings, which can produce NaN/Infinity and drift from backend-calculated values.
✅ Suggested fix
+ const exchangeRate = feeBreakdown ? Number(feeBreakdown.exchangeRate) : null;
...
- 1 USDC = ₦{(parseFloat(feeBreakdown.fiatPayout) / (parseFloat(feeBreakdown.sendAmount) - parseFloat(feeBreakdown.bridgeFee))).toLocaleString()}
+ 1 USDC = ₦{Number.isFinite(exchangeRate) ? exchangeRate!.toLocaleString() : "--"}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/offramp/OfframpSuccessModal.tsx` around lines 119 -
125, OfframpSuccessModal currently recomputes the exchange rate inline from
feeBreakdown fields (parseFloat(feeBreakdown.fiatPayout) /
(parseFloat(feeBreakdown.sendAmount) - parseFloat(feeBreakdown.bridgeFee)))
which can yield NaN/Infinity and diverge from the backend; instead, use a
canonical exchange rate value provided by the backend (e.g.,
feeBreakdown.exchangeRate) or add an exchangeRate prop to OfframpSuccessModal,
display that value in the Exchange Rate span, and include a defensive fallback
(e.g., empty string or "—") if the canonical value is missing/invalid to avoid
inline arithmetic in render.
Comment on lines
+154
to
+156
| <button onClick={handleCopy} className="p-2 bg-gray-800 rounded-lg text-fundable-purple hover:text-white transition-colors"> | ||
| {copied ? <CheckCircle2 className="h-4 w-4 text-green-500" /> : <Copy className="h-4 w-4" />} | ||
| </button> |
Contributor
There was a problem hiding this comment.
Add accessible name for the icon-only copy button.
Line 154 renders an icon-only <button> with no accessible label, making the action unclear for screen-reader users.
✅ Suggested fix
- <button onClick={handleCopy} className="p-2 bg-gray-800 rounded-lg text-fundable-purple hover:text-white transition-colors">
+ <button
+ type="button"
+ onClick={handleCopy}
+ aria-label={copied ? "Reference copied" : "Copy transaction reference"}
+ className="p-2 bg-gray-800 rounded-lg text-fundable-purple hover:text-white transition-colors"
+ >📝 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.
Suggested change
| <button onClick={handleCopy} className="p-2 bg-gray-800 rounded-lg text-fundable-purple hover:text-white transition-colors"> | |
| {copied ? <CheckCircle2 className="h-4 w-4 text-green-500" /> : <Copy className="h-4 w-4" />} | |
| </button> | |
| <button | |
| type="button" | |
| onClick={handleCopy} | |
| aria-label={copied ? "Reference copied" : "Copy transaction reference"} | |
| className="p-2 bg-gray-800 rounded-lg text-fundable-purple hover:text-white transition-colors" | |
| > | |
| {copied ? <CheckCircle2 className="h-4 w-4 text-green-500" /> : <Copy className="h-4 w-4" />} | |
| </button> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/offramp/OfframpSuccessModal.tsx` around lines 154 -
156, The icon-only copy button in OfframpSuccessModal lacks an accessible label;
update the <button> rendered near the copied variable and handleCopy handler to
include an accessible name (e.g., aria-label="Copy code" or aria-label that
toggles to "Copied" when copied is true) or add visually hidden text inside the
button so screen readers can announce the action; ensure the change references
the same button that conditionally renders CheckCircle2 and Copy so behavior and
styling remain unchanged.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (3)
apps/web/src/components/offramp/OfframpSuccessModal.tsx (3)
91-98:⚠️ Potential issue | 🟠 MajorUse API-provided
estimatedTimeinstead of hardcoded value.The
BridgeFeeBreakdowninterface providesestimatedTime: numberbut this banner hardcodes "2-5 Minutes". This may present inaccurate expectations to users when the actual processing time differs.Suggested fix
- {isProcessing && ( + {isProcessing && feeBreakdown && ( <div className="inline-flex items-center gap-2 px-3 py-1 bg-fundable-purple/10 border border-fundable-purple/20 rounded-full"> <Clock size={12} className="text-fundable-purple" /> <span className="text-[10px] font-bold text-fundable-purple uppercase tracking-wider"> - Estimated: 2-5 Minutes + Estimated: {feeBreakdown.estimatedTime} Minutes </span> </div> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/offramp/OfframpSuccessModal.tsx` around lines 91 - 98, Replace the hardcoded "Estimated: 2-5 Minutes" banner in OfframpSuccessModal.tsx with the API-provided value from the BridgeFeeBreakdown (use the estimatedTime property) so the UI reflects real data; locate the JSX block that renders when isProcessing is true and swap the static span text for a formatted value derived from estimatedTime (e.g., format estimatedTime into minutes and render "Estimated: {formattedEstimatedTime}" or a sensible range), ensuring you reference the BridgeFeeBreakdown.estimatedTime field used by the component instead of the literal string.
155-157:⚠️ Potential issue | 🟡 MinorAdd accessible name for the icon-only copy button.
The copy button renders only an icon with no accessible label, making the action unclear for screen-reader users.
Suggested fix
- <button onClick={handleCopy} className="p-2 bg-gray-800 rounded-lg text-fundable-purple hover:text-white transition-colors"> + <button + type="button" + onClick={handleCopy} + aria-label={copied ? "Reference copied" : "Copy transaction reference"} + className="p-2 bg-gray-800 rounded-lg text-fundable-purple hover:text-white transition-colors" + > {copied ? <CheckCircle2 className="h-4 w-4 text-green-500" /> : <Copy className="h-4 w-4" />} </button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/offramp/OfframpSuccessModal.tsx` around lines 155 - 157, The copy button in OfframpSuccessModal is icon-only and lacks an accessible name; update the button (the element using onClick={handleCopy}) to include an accessible label such as aria-label="Copy address" or aria-labelledby pointing to a hidden text node, or add visually-hidden text within the button so screen readers can announce the action while keeping the icon for sighted users.
120-127:⚠️ Potential issue | 🟠 MajorUse canonical
exchangeRatefrom API instead of recalculating.Line 125 recomputes the exchange rate via division, which can produce
NaNorInfinityif the denominator is zero, and may drift from backend-calculated values. TheBridgeFeeBreakdowninterface already providesexchangeRate: string.Suggested fix
<div className="flex justify-between items-center text-[11px]"> <span className="text-gray-500 flex items-center gap-1"> Exchange Rate <Info size={10} /> </span> <span className="text-gray-400 font-mono"> - 1 USDC = ₦{(parseFloat(feeBreakdown.fiatPayout) / (parseFloat(feeBreakdown.sendAmount) - parseFloat(feeBreakdown.bridgeFee))).toLocaleString()} + 1 USDC = ₦{parseFloat(feeBreakdown.exchangeRate).toLocaleString()} </span> </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/offramp/OfframpSuccessModal.tsx` around lines 120 - 127, The Exchange Rate display in OfframpSuccessModal currently recalculates the rate using parseFloat(feeBreakdown.fiatPayout) / (parseFloat(feeBreakdown.sendAmount) - parseFloat(feeBreakdown.bridgeFee)), which can yield NaN/Infinity and drift from backend values; replace that expression with the canonical feeBreakdown.exchangeRate (from the BridgeFeeBreakdown interface) and format it for display (e.g., toLocaleString) instead of performing the division in the component.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/web/src/components/offramp/OfframpSuccessModal.tsx`:
- Around line 91-98: Replace the hardcoded "Estimated: 2-5 Minutes" banner in
OfframpSuccessModal.tsx with the API-provided value from the BridgeFeeBreakdown
(use the estimatedTime property) so the UI reflects real data; locate the JSX
block that renders when isProcessing is true and swap the static span text for a
formatted value derived from estimatedTime (e.g., format estimatedTime into
minutes and render "Estimated: {formattedEstimatedTime}" or a sensible range),
ensuring you reference the BridgeFeeBreakdown.estimatedTime field used by the
component instead of the literal string.
- Around line 155-157: The copy button in OfframpSuccessModal is icon-only and
lacks an accessible name; update the button (the element using
onClick={handleCopy}) to include an accessible label such as aria-label="Copy
address" or aria-labelledby pointing to a hidden text node, or add
visually-hidden text within the button so screen readers can announce the action
while keeping the icon for sighted users.
- Around line 120-127: The Exchange Rate display in OfframpSuccessModal
currently recalculates the rate using parseFloat(feeBreakdown.fiatPayout) /
(parseFloat(feeBreakdown.sendAmount) - parseFloat(feeBreakdown.bridgeFee)),
which can yield NaN/Infinity and drift from backend values; replace that
expression with the canonical feeBreakdown.exchangeRate (from the
BridgeFeeBreakdown interface) and format it for display (e.g., toLocaleString)
instead of performing the division in the component.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2784c984-62a8-4957-a953-472c811ab2f8
📒 Files selected for processing (1)
apps/web/src/components/offramp/OfframpSuccessModal.tsx
Contributor
|
Please fix merge conflict |
Contributor
|
Please resolve merge conflict |
Contributor
|
Hello, kindly fix the conflict. |
Contributor
|
Hi, kindly resolve the new conflict, and add screen record of your implementation. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR completes #62 by adding a transparent fee breakdown, exchange rate display, and estimated processing time to the offramp flow.
Closes #62
Summary by CodeRabbit
New Features
UI/Style Updates