feat: donation rate prop #556
Conversation
lissavxo
requested review from
Klakurka and
chedieck
and removed request for
chedieck
|
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 donation support across the React PayButton stack and documentation: new Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Integrator as App
participant PB as PayButton
participant PD as PaymentDialog
participant WC as WidgetContainer
participant W as Widget
Integrator->>PB: Render PayButton({ donationAddress?, donationRate? })
PB->>PB: Apply defaults (config.donationAddress, DEFAULT_DONATION_RATE)
PB->>PD: Pass { donationAddress, donationRate, ... }
PD->>WC: Pass { donationAddress, donationRate, ... }
WC->>W: Pass { donationAddress, donationRate, ... }
rect rgba(200,230,255,0.25)
note over W: Display amounts\nIf donationRate ≥ DONATION_RATE_FIAT_THRESHOLD, compute donation amounts\nStore converted donationAmount
end
W-->>Integrator: UI updates with donation-inclusive amounts (if applicable)
sequenceDiagram
autonumber
participant PB as PayButton
participant Chronik as checkSuccessInfo
participant W as Widget
participant S as PaymentServer
PB->>Chronik: checkSuccessInfo{ ..., donationRate }
Chronik-->>PB: Success events (may include donationRate)
W->>W: Build submission URL
alt donationAddress && donationRate && donationAmount ≥ DEFAULT_MINIMUM_DONATION_AMOUNT
W->>S: GET /pay?amount=<base>&addr=<donationAddress>&donation=<calc>
else
W->>S: GET /pay?amount=<base>
end
S-->>W: Response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 (1)
🧰 Additional context used🧬 Code graph analysis (1)react/lib/components/Widget/Widget.tsx (3)
🔇 Additional comments (5)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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 |
lissavxo
force-pushed
the
feat/donation_address_multiple_outputs
branch
from
7de49d4 to
2cac42c
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (10)
react/lib/util/constants.ts (1)
16-16: Nit: normalize quotes and avoid includingundefinedin the union.
Includingundefinedin the type is usually unnecessary when the prop itself is optional. Also align quotes for consistency.Apply if you agree:
-export type ButtonSize = 'xs' | 'sm' | 'md' | 'lg' | 'xl' | "extrasmall" | "small" | "medium" | "large" | "extralarge" | undefined; +export type ButtonSize = + | 'xs' | 'sm' | 'md' | 'lg' | 'xl' + | 'extrasmall' | 'small' | 'medium' | 'large' | 'extralarge';docs/README.md (1)
1101-1129: Fix typos, add behavior note, and satisfy markdownlint (MD001/MD037).
- “intergers” → “integers”; tighten wording.
- Add the fiat behavior note (donationRate > 5).
- Change subheadings from h4 with bold to h3 plain to fix heading increment and emphasis-spacing warnings.
-## donation-rate - -> **The ‘donationRate’ parameter represents the amount that will be donated to the donation address.** - -?> The donationRate parameter is optional values accepted are intergers between 0-100. Default value is 2. - -**Example:** -<!-- tabs:start --> - -#### ** HTML ** +## donation-rate + +> The ‘donationRate’ parameter represents the percentage of the amount that will be sent to the donation address. + +?> Optional. Integer 0–100 (percent). Default: 2. +!> Note: If the currency is fiat (USD/CAD) and donationRate > 5, the dialog amount will include the donation amount. + +**Example:** +<!-- tabs:start --> + +### HTML ```html donation-rate="10"
-#### ** JavaScript **
+### JavaScriptdonationRate: 10
-#### ** React **
+### ReactdonationRate = 10</blockquote></details> <details> <summary>docs/zh-tw/README.md (1)</summary><blockquote> `1096-1124`: **Add behavior note and fix markdownlint (heading level/emphasis spacing).** ```diff -## donation-rate - -> **「donationRate」參數用於設定將捐贈到捐贈地址的金額。** - -?> donationRate 參數是可選的，接受的值為 0 到 100 之間的整數。預設值為 2。 - -**Example:** -<!-- tabs:start --> - -#### ** HTML ** +## donation-rate + +> 「donationRate」參數用於設定將捐贈到捐贈地址的比例（百分比）。 + +?> 選填。整數 0–100（百分比）。預設：2。 +!> 注意：當貨幣為法幣（USD/CAD）且 donationRate > 5 時，對話框中的金額會包含捐贈金額。 + +**Example:** +<!-- tabs:start --> + +### HTML ```html donation-rate="10"
-#### ** JavaScript **
+### JavaScriptdonationRate: 10
-#### ** React **
+### ReactdonationRate = 10</blockquote></details> <details> <summary>docs/zh-cn/README.md (1)</summary><blockquote> `1100-1128`: **Add behavior note and fix markdownlint (heading level/emphasis spacing).** ```diff -## donation-rate - -> **「donationRate」参数用于设置将捐赠到捐赠地址的金额。** - -?> donationRate 参数是可选的，接受的值为 0 到 100 之间的整数。默认值为 2。 - -**Example:** -<!-- tabs:start --> - -#### ** HTML ** +## donation-rate + +> 「donationRate」参数表示将捐赠到捐赠地址的比例（百分比）。 + +?> 可选。整数 0–100（百分比）。默认：2。 +!> 注意：当货币为法币（USD/CAD）且 donationRate > 5 时，对话框中的金额会包含捐赠金额。 + +**Example:** +<!-- tabs:start --> + +### HTML ```html donation-rate="10"
-#### ** JavaScript **
+### JavaScriptdonationRate: 10
-#### ** React **
+### ReactdonationRate = 10</blockquote></details> <details> <summary>react/lib/components/Widget/Widget.tsx (3)</summary><blockquote> `680-680`: **Recompute text/URL when donation props change.** Missing dependencies cause stale text/URL when donationRate or donationAddress changes. Apply this diff: ```diff - }, [to, thisCurrencyObject, price, thisAmount, opReturn, hasPrice, isCashtabAvailable]); + }, [to, thisCurrencyObject, price, thisAmount, opReturn, hasPrice, isCashtabAvailable, donationRate, donationAddress]);
790-790: Include donation props in resolveUrl deps to avoid stale closures.Apply this diff:
- }, [disabled, to, currency, opReturn]); + }, [disabled, to, currency, opReturn, donationAddress, donationRate, donationAmount]);
440-441: Optional: name the state to reflect units.donationAmount is crypto-denominated; consider donationCryptoAmount for clarity.
paybutton/src/index.tsx (1)
182-191: Coerce and validate donationRate from attributes (0–100).Currently passed as a string; normalize and validate like amount/randomSatoshis.
Apply this diff:
if (isNaN(props.amount)) { console.error('Amount must be a number') } } + if (attributes.donationRate != null) { + const n = Number(attributes.donationRate); + if (Number.isFinite(n) && n >= 0 && n <= 100) { + props.donationRate = n; + } else { + console.error('donationRate must be a number between 0 and 100'); + delete props.donationRate; + } + } + props.hideToasts = attributes.hideToasts === 'true';react/lib/components/Widget/WidgetContainer.tsx (2)
142-144: Normalize/clamp donationRate once at the boundary.Accept numeric strings; clamp to 0–100; default if invalid.
Apply this diff:
- if (donationRate === undefined){ - donationRate = DEFAULT_DONATE_RATE - } + // Normalize/clamp + const n = donationRate as unknown as number; + const parsed = Number(n); + donationRate = + donationRate === undefined + ? DEFAULT_DONATE_RATE + : Number.isFinite(parsed) + ? Math.max(0, Math.min(100, parsed)) + : DEFAULT_DONATE_RATE;
243-245: Nit: remove unused dependency.donationRate isn’t referenced in handlePayment; drop it to avoid needless re-creations.
Apply this diff:
- randomSatoshis, - donationRate + randomSatoshis
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
docs/README.md(1 hunks)docs/_sidebar.md(1 hunks)docs/zh-cn/README.md(1 hunks)docs/zh-cn/_sidebar.md(1 hunks)docs/zh-tw/README.md(1 hunks)paybutton/src/index.tsx(1 hunks)react/lib/components/PayButton/PayButton.tsx(7 hunks)react/lib/components/PaymentDialog/PaymentDialog.tsx(3 hunks)react/lib/components/Widget/Widget.tsx(8 hunks)react/lib/components/Widget/WidgetContainer.tsx(6 hunks)react/lib/example-config.json(1 hunks)react/lib/util/constants.ts(1 hunks)react/lib/util/types.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
react/lib/components/Widget/Widget.tsx (2)
react/lib/util/satoshis.ts (1)
getCurrencyObject(8-41)react/lib/util/constants.ts (3)
CURRENCY_PREFIXES_MAP(11-14)DECIMALS(5-9)DEFAULT_DONATE_RATE(18-18)
react/lib/components/Widget/WidgetContainer.tsx (1)
react/lib/util/constants.ts (1)
DEFAULT_DONATE_RATE(18-18)
react/lib/components/PayButton/PayButton.tsx (1)
react/lib/util/constants.ts (1)
DEFAULT_DONATE_RATE(18-18)
🪛 markdownlint-cli2 (0.17.2)
docs/zh-cn/README.md
1110-1110: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
1110-1110: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
1110-1110: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
1116-1116: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
1116-1116: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
1122-1122: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
1122-1122: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
docs/README.md
1111-1111: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
1111-1111: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
1111-1111: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
1117-1117: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
1117-1117: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
1123-1123: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
1123-1123: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
docs/zh-tw/README.md
1106-1106: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
1106-1106: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
1106-1106: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
1112-1112: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
1112-1112: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
1118-1118: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
1118-1118: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
🔇 Additional comments (9)
docs/zh-cn/_sidebar.md (1)
35-35: LGTM: adds donation-rate to 参数 section in the right spot.react/lib/util/constants.ts (1)
18-18: Default donation rate constant looks good.
No behavior concerns here.docs/_sidebar.md (1)
36-36: LGTM: new “donation-rate” nav item added under Parameters.react/lib/example-config.json (1)
19-19: LGTM — confirm example donationAddress is intended to be publicThis value is used as the default (config.donationAddress) in: react/lib/example-config.json (line 19), react/lib/components/Widget/Widget.tsx (defaultProps ~1038), react/lib/components/PayButton/PayButton.tsx (defaultProps ~384), react/lib/components/Widget/WidgetContainer.tsx, and react/lib/components/PaymentDialog/PaymentDialog.tsx. If this is a private/receiving address, replace with a placeholder or test address; otherwise OK.
docs/README.md (1)
1101-1129: No change needed —donation-rateis supported. findAndRender converts element attribute names with camelcase(name) and allowedProps includes "donationRate", so an HTML attribute like donation-rate="10" will map to donationRate and be accepted. See paybutton/src/index.tsx (allowedProps includes 'donationRate' and the camelcase(name) check in the attribute reduction).react/lib/components/Widget/Widget.tsx (1)
1038-1040: Defaults look good.Sensible defaults via config and constant.
react/lib/components/PaymentDialog/PaymentDialog.tsx (2)
65-68: Prop additions look correct and are safely forwarded.No issues spotted.
239-241: Forwarding donation props to WidgetContainer LGTM.Matches the new API.
react/lib/components/PayButton/PayButton.tsx (1)
384-386: Defaults LGTM.Using config + DEFAULT_DONATE_RATE is consistent with Widget.
| currencyObj?: CurrencyObject, | ||
| donationRate?: number | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
🧩 Analysis chain
Document units for donationRate in the type.
Clarify it’s a percentage (0–100) to prevent misuse.
export interface CheckSuccessInfo {
@@
- currencyObj?: CurrencyObject,
- donationRate?: number
+ currencyObj?: CurrencyObject,
+ /** Donation percentage (0–100). */
+ donationRate?: number
}To reduce integration bugs, please also verify that donationRate is clamped/validated to [0,100] before use and passed as an integer everywhere it’s consumed.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Where donationRate is used:"
rg -nC2 --type=ts --type=tsx '\bdonationRate\b' | sed -n '1,200p'
echo
echo "Look for clamping/validation:"
rg -nC2 --type=ts --type=tsx 'donationRate\s*=\s*Math\.(min|max)|clamp|inRange|between|assert|zod|yup|Joi'Length of output: 55
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching for donationRate occurrences (ts/tsx/js/jsx)..."
rg -nC2 '\bdonationRate\b' --hidden --glob '!.git/**' --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' --glob '**/*.ts' --glob '**/*.tsx' --glob '**/*.js' --glob '**/*.jsx' -S || true
echo
echo "Searching for clamping/validation patterns (Math.min/max, clamp, inRange, between, assert, zod, yup, Joi, manual bounds checks)..."
rg -nC2 '\bdonationRate\b.*Math\.(min|max)|Math\.(min|max).*donationRate' --hidden --glob '!.git/**' --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' -S || true
rg -nC2 'donationRate.*(clamp|inRange|between|assert|zod|yup|Joi)' --hidden --glob '!.git/**' --glob '!**/node_modules/**' --glob '!**/dist/**' -S || true
rg -nC2 'if\s*\(.*donationRate' --hidden --glob '!.git/**' --glob '!**/node_modules/**' --glob '!**/dist/**' -S || true
rg -nC2 'donationRate\s*[<>]=?\s*[0-9]+' --hidden --glob '!.git/**' --glob '!**/node_modules/**' --glob '!**/dist/**' -S || true
echo
echo "Search for CheckSuccessInfo type/interface definition(s)..."
rg -nC2 'interface\s+CheckSuccessInfo\b|type\s+CheckSuccessInfo\b' --hidden --glob '**/*.ts' -S || trueLength of output: 9593
Document donationRate as a percentage (0–100) and enforce bounds
Add JSDoc to the type and clamp/validate donationRate to [0,100] at the props-normalization point — current uses in react/lib/components/Widget/Widget.tsx, WidgetContainer.tsx, PaymentDialog.tsx, PayButton.tsx do not perform bounds checks.
export interface CheckSuccessInfo {
@@
- currencyObj?: CurrencyObject,
- donationRate?: number
+ currencyObj?: CurrencyObject,
+ /** Donation percentage (0–100). */
+ donationRate?: number
}Normalize once (e.g. in WidgetContainer when applying defaults):
donationRate = Math.min(100, Math.max(0, Number(donationRate) ?? DEFAULT_DONATE_RATE));
📝 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.
| currencyObj?: CurrencyObject, | |
| donationRate?: number | |
| } | |
| currencyObj?: CurrencyObject, | |
| /** Donation percentage (0–100). */ | |
| donationRate?: number | |
| } |
🤖 Prompt for AI Agents
In react/lib/util/types.ts around lines 157-159, add a JSDoc comment to document
donationRate as a percentage in the range 0–100, and then normalize/validate the
prop once at the props-normalization point (e.g. in WidgetContainer when
applying defaults) by converting to Number, falling back to DEFAULT_DONATE_RATE
if missing/NaN, and clamping to [0,100] (e.g. donationRate = Math.min(100,
Math.max(0, Number(donationRate) ?? DEFAULT_DONATE_RATE))). Ensure other
components rely on the normalized value rather than re-validating.
There was a problem hiding this comment.
Pull Request Overview
This PR implements configurable donation functionality for PayButton by adding optional donation support with customizable donation rates. It allows developers to set donation addresses and rates, with special handling for fiat currencies when donation rates exceed 5%.
- Adds
donationAddressanddonationRateprops to PayButton components with default donation rate of 2% - Implements donation amount calculation and payment URL generation with multi-destination support
- Adds comprehensive documentation for the new donation-rate parameter in multiple languages
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| react/lib/util/types.ts | Adds donationRate field to CheckSuccessInfo interface |
| react/lib/util/constants.ts | Defines DEFAULT_DONATE_RATE constant (2%) |
| react/lib/example-config.json | Adds default donation address configuration |
| react/lib/components/Widget/WidgetContainer.tsx | Passes donation props and sets default rate |
| react/lib/components/Widget/Widget.tsx | Implements donation logic and payment URL generation |
| react/lib/components/PaymentDialog/PaymentDialog.tsx | Forwards donation props to widget |
| react/lib/components/PayButton/PayButton.tsx | Adds donation props with defaults |
| paybutton/src/index.tsx | Adds donationRate to allowed props |
| docs/* | Adds donation-rate documentation in English and Chinese |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if (donationRate === undefined){ | ||
| donationRate = DEFAULT_DONATE_RATE | ||
| } |
There was a problem hiding this comment.
The donationRate variable is being reassigned directly, but it's a parameter from props destructuring. This creates a confusing mutation of what should be an immutable prop. Consider using a separate variable like const effectiveDonationRate = donationRate ?? DEFAULT_DONATE_RATE instead.
| if (donationRate === undefined){ | |
| donationRate = DEFAULT_DONATE_RATE | |
| } | |
| const effectiveDonationRate = donationRate ?? DEFAULT_DONATE_RATE; |
| if (convertedObj) { | ||
| let amountToDisplay = thisCurrencyObject.string; | ||
| let convertedAmountToDisplay = convertedObj.string | ||
| if ( donationRate && donationRate >= 5){ |
There was a problem hiding this comment.
The hardcoded value 5 should be extracted as a named constant (e.g., MINIMUM_FIAT_DONATION_RATE_FOR_DISPLAY_ADJUSTMENT) to make the business logic more maintainable and self-documenting.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/README.md (1)
1105-1105: Fix grammatical issues in the parameter description.Line 1105 is grammatically awkward and needs punctuation. Restructure to separate the two independent clauses:
-?> The donationRate parameter is optional values accepted are integers between 0-100. Default value is 2. +?> The donationRate parameter is optional. Values accepted are integers between 0-100. Default value is 2.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/README.md(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
docs/README.md
1111-1111: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
1111-1111: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
1111-1111: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
1117-1117: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
1117-1117: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
1123-1123: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
1123-1123: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
🔇 Additional comments (1)
docs/README.md (1)
1101-1128: The review comment is based on an incorrect characterization of the code behavior.The actual implementation in
react/lib/components/Widget/Widget.tsx(lines 641-660) shows that the donation amount is added to the display amount wheneverhasPriceis true anddonationRate >= 5, regardless of currency type. The code contains no fiat-specific condition (isFiat()check). This behavior applies equally to crypto and fiat currencies when price data is available.Additionally, the code updates the display strings (
amountToDisplay,convertedAmountToDisplay), not the underlying transaction state. The review's characterization of "special fiat currency behavior" is inaccurate.The current documentation at lines 1101-1128 is sufficient for the actual behavior implemented. If documentation improvements are desired, they should clarify that when a price is available and
donationRate >= 5, the displayed amount includes the donation, rather than incorrectly implying this is fiat-specific behavior.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
react/lib/components/Widget/Widget.tsx (3)
642-642: Optional: Extract magic number to named constant.The hardcoded value
5should be extracted as a named constant (e.g.,MINIMUM_FIAT_DONATION_RATE_FOR_DISPLAY_ADJUSTMENT) for maintainability. This was previously flagged by Copilot.
640-661: Critical: Donation amount stores total instead of donation portion (double-counting).Line 660 stores
convertedAmountWithDonationObj.float(which is base + donation) intodonationAmountstate. Later, at line 776, this value is used as the donation output amount, resulting in the URI containing two outputs: base amount + (base + donation amount) = overcharge.This exact issue was flagged in previous reviews and remains unresolved.
Apply this diff to store only the donation portion:
convertedAmountToDisplay = convertedAmountWithDonationObj.string - setDonationAmount(convertedAmountWithDonationObj.float) + setDonationAmount(convertedDonationAmount) }
770-784: Critical/Major: Multiple issues in multi-output URI construction.This code section has several unresolved issues flagged in previous reviews:
- Line 774:
decimalscan beundefinedif the network lookup fails, causingtoFixed(undefined)to fail at line 780.- Line 776: Uses
donationAmountwhich contains the total amount (base + donation) from line 660, not just the donation portion—resulting in incorrect URI amounts.- Line 780:
donationAddressis used without stripping any currency prefix (e.g.,bitcoincash:), which could produce an invalid address parameter.- Line 770: While
Number(donationRate)provides defensive parsing, the checkNumber(donationRate)evaluates tofalsefor0, which is acceptable here but worth noting for edge cases.Apply this diff to address these issues:
- if (donationAddress && donationRate && Number(donationRate)) { + if (donationAddress && donationRate != null && !isNaN(Number(donationRate))) { const network = Object.entries(CURRENCY_PREFIXES_MAP).find( ([, value]) => value === prefix )?.[0]; - const decimals = network ? DECIMALS[network.toUpperCase()] : undefined; + const decimals = network ? DECIMALS[network.toUpperCase()] : DECIMALS.BCH; const donationPercent = donationRate / 100 - const thisDonationAmount = donationAmount ? donationAmount : amount * donationPercent + const thisDonationAmount = donationAmount ?? (amount * donationPercent) + const cleanDonationAddr = donationAddress.replace(/^.*:/, ''); - thisUrl+=`?amount=${amount}` + thisUrl += `?amount=${amount}` if(thisDonationAmount > DEFAULT_MINIMUM_DONATE_AMOUNT){ - thisUrl += `&addr=${donationAddress}&amount=${thisDonationAmount.toFixed(decimals)}`; + thisUrl += `&addr=${cleanDonationAddr}&amount=${thisDonationAmount.toFixed(decimals)}`; }Note: This fix assumes line 660 is corrected first to store only the donation portion in
donationAmount.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
react/lib/components/Widget/Widget.tsx(8 hunks)react/lib/util/constants.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- react/lib/util/constants.ts
🧰 Additional context used
🧬 Code graph analysis (1)
react/lib/components/Widget/Widget.tsx (2)
react/lib/util/satoshis.ts (1)
getCurrencyObject(8-41)react/lib/util/constants.ts (4)
CURRENCY_PREFIXES_MAP(11-14)DECIMALS(5-9)DEFAULT_MINIMUM_DONATE_AMOUNT(20-20)DEFAULT_DONATE_RATE(18-18)
🔇 Additional comments (2)
react/lib/components/Widget/Widget.tsx (2)
16-16: LGTM: Donation props and imports added correctly.The imports, interface additions, and state initialization for donation support are properly structured and typed.
Also applies to: 38-39, 102-103, 441-441
1037-1043: LGTM: Default props configured correctly.The default values for
donationAddressanddonationRateare properly sourced from the imported config and constants.
| donationAddress, | ||
| donationRate |
There was a problem hiding this comment.
🧩 Analysis chain
Verify donation props propagate correctly through component tree.
The donation props are properly destructured and included in the relevant useEffect dependency array. However, ensure that these props are correctly propagated from parent components (e.g., WidgetContainer, PaymentDialog) as mentioned in the summary.
Run the following script to verify prop propagation:
Also applies to: 681-681
🏁 Script executed:
#!/bin/bash
# Description: Verify donationAddress and donationRate props are passed through the component hierarchy
# Search for donationAddress prop usage in parent components
rg -nP --type=tsx --type=ts -C3 'donationAddress\s*[=:]' -g '!Widget.tsx'
# Search for donationRate prop usage in parent components
rg -nP --type=tsx --type=ts -C3 'donationRate\s*[=:]' -g '!Widget.tsx'
# Verify WidgetContainer passes these props
ast-grep --pattern $'export const WidgetContainer = $_ => {
$$$
<Widget
$$$
donationAddress={$_}
$$$
/>
$$$
}'Length of output: 55
Let me re-run the verification with corrected syntax:
🏁 Script executed:
#!/bin/bash
# Description: Verify donationAddress and donationRate props are passed through the component hierarchy
# Find and search for donationAddress in TypeScript/React files
echo "=== Searching for donationAddress usage ==="
fd '\.(tsx?|jsx?)$' | xargs rg -n 'donationAddress\s*[=:]' -C2 | grep -v 'Widget.tsx' | head -50
# Find and search for donationRate in TypeScript/React files
echo "=== Searching for donationRate usage ==="
fd '\.(tsx?|jsx?)$' | xargs rg -n 'donationRate\s*[=:]' -C2 | grep -v 'Widget.tsx' | head -50
# Search for WidgetContainer component definition
echo "=== Searching for WidgetContainer ==="
fd '\.(tsx?|jsx?)$' | xargs rg -n 'WidgetContainer|export.*Widget' -C5 | head -100
# Check for PaymentDialog component
echo "=== Searching for PaymentDialog ==="
fd '\.(tsx?|jsx?)$' | xargs rg -n 'PaymentDialog' -C3 | head -50Length of output: 15088
Let me verify the dependency array and confirm these props are in the WidgetProps interface:
🏁 Script executed:
#!/bin/bash
# Verify donationAddress and donationRate are in WidgetProps interface
echo "=== WidgetProps interface definition ==="
rg -n 'export interface WidgetProps' -A50 react/lib/components/Widget/Widget.tsx | head -80
# Check for useEffect with these props in dependency array
echo "=== useEffect with donation props in dependencies ==="
rg -n 'useEffect' -A15 react/lib/components/Widget/Widget.tsx | grep -A15 -B5 'donationAddress\|donationRate'
# Verify line 681 context for useEffect
echo "=== Context around line 681 ==="
sed -n '675,690p' react/lib/components/Widget/Widget.tsxLength of output: 2151
Let me get the complete WidgetProps interface and search for actual usage of these props:
🏁 Script executed:
#!/bin/bash
# Get complete WidgetProps interface
echo "=== Complete WidgetProps interface ==="
rg -n 'export interface WidgetProps' -A100 react/lib/components/Widget/Widget.tsx | grep -E '(donationAddress|donationRate|^[0-9]+-\}'
# Search for destructuring of donationAddress and donationRate in Widget component
echo "=== Prop destructuring in Widget component (lines 308-320) ==="
sed -n '308,330p' react/lib/components/Widget/Widget.tsx
# Search for all uses of these props in Widget.tsx
echo "=== All occurrences of donationAddress in Widget.tsx ==="
rg -n 'donationAddress' react/lib/components/Widget/Widget.tsx
echo "=== All occurrences of donationRate in Widget.tsx ==="
rg -n 'donationRate' react/lib/components/Widget/Widget.tsxLength of output: 1464
Let me check if the prop usage is within useEffect blocks and verify the dependency arrays:
🏁 Script executed:
#!/bin/bash
# Get context around line 642 to see if it's in useEffect
echo "=== Context around line 642 (donation usage) ==="
sed -n '635,660p' react/lib/components/Widget/Widget.tsx
# Get context around line 770 to see if it's in useEffect
echo "=== Context around line 770 (donation URL building) ==="
sed -n '765,790p' react/lib/components/Widget/Widget.tsx
# Find all useEffect blocks and show their dependency arrays
echo "=== All useEffect blocks with dependencies ==="
rg -n 'useEffect' -A20 react/lib/components/Widget/Widget.tsx | grep -E '(useEffect|^\s*\}.*\['Length of output: 2249
Let me fix the regex and check the broader structure:
🏁 Script executed:
#!/bin/bash
# Get more context to see which function/effect contains line 642
echo "=== Broader context for line 642 (which function?) ==="
sed -n '600,645p' react/lib/components/Widget/Widget.tsx | tail -50
# Get more context for line 770
echo "=== Broader context for line 770 (which function?) ==="
sed -n '750,790p' react/lib/components/Widget/Widget.tsx
# Search for useEffect definitions with line numbers
echo "=== useEffect locations ==="
rg -n 'useEffect\(' react/lib/components/Widget/Widget.tsx
# Get all useEffect blocks with their full dependency arrays
echo "=== useEffect 1 (around line 681) ==="
sed -n '678,690p' react/lib/components/Widget/Widget.tsx
# Look for other useEffect blocks
echo "=== Check more useEffect blocks ==="
sed -n '640,670p' react/lib/components/Widget/Widget.tsxLength of output: 5317
Let me check the useCallback definition for resolveUrl:
🏁 Script executed:
#!/bin/bash
# Get the resolveUrl useCallback with its dependency array
echo "=== resolveUrl useCallback definition ==="
sed -n '753,810p' react/lib/components/Widget/Widget.tsx
# Look for the dependency array of resolveUrl
echo "=== Finding resolveUrl dependency array ==="
sed -n '753,835p' react/lib/components/Widget/Widget.tsx | tail -30Length of output: 2631
Add missing donation props to dependency arrays in Widget.tsx.
While donation props correctly propagate through the component tree (PayButton → PaymentDialog → WidgetContainer → Widget), they're not included in the dependency arrays where they're used, causing stale closures:
- Line 681 — useEffect dependency array is missing
donationRate(used at lines 642, 643, 652) - resolveUrl useCallback — dependency array is missing
donationAddress,donationRate, anddonationAmount(used at lines 770, 775, 776, 780)
Update both dependency arrays to include these props.
🤖 Prompt for AI Agents
In react/lib/components/Widget/Widget.tsx around lines 353 and specifically the
effects/callbacks referenced near lines 642–652, 681 and 770–780, the dependency
arrays are missing donation props which causes stale closures; update the
useEffect at ~line 681 to include donationRate, and update the resolveUrl
useCallback dependency array to include donationAddress, donationRate, and
donationAmount so the hook re-runs when these props change; ensure you add only
these props to the respective dependency arrays and run lint/tests to verify no
other missing deps are flagged.
lissavxo
force-pushed
the
feat/donation_address_multiple_outputs
branch
from
e912f0a to
83c595b
Compare
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (1)
react/lib/components/PayButton/PayButton.tsx (1)
211-211: donationRate passed to checkSuccessInfo but not used in validation.This issue was previously identified:
donationRateis included incheckSuccessInfobutshouldTriggerOnSuccessdoes not accept or use it. Either implement donation-aware logic inshouldTriggerOnSuccessand update all call sites, or removedonationRatefromcheckSuccessInfoif it shouldn't affect success detection.
🧹 Nitpick comments (2)
docs/README.md (1)
1105-1105: Clarify awkward phrasing in parameter description.Line 1105 reads: "The donationRate parameter is optional values accepted are integers between 0-100." This is grammatically unclear. Revise for clarity:
-?> The donationRate parameter is optional values accepted are integers between 0-100. Default value is 2. +?> The donationRate parameter is optional. Values accepted are integers between 0–100. Default value is 2.react/lib/components/PayButton/PayButton.tsx (1)
60-61: Consider adding runtime validation for donationRate.While the props are correctly typed, consider validating that
donationRateis within a reasonable range (e.g., 0-100 for percentage) to prevent misconfiguration. The PR description mentions special behavior when "donationRate is above 5", suggesting percentage semantics.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
docs/README.md(1 hunks)docs/_sidebar.md(1 hunks)docs/zh-cn/README.md(1 hunks)docs/zh-cn/_sidebar.md(1 hunks)docs/zh-tw/README.md(1 hunks)paybutton/src/index.tsx(1 hunks)react/lib/components/PayButton/PayButton.tsx(6 hunks)react/lib/components/PaymentDialog/PaymentDialog.tsx(3 hunks)react/lib/components/Widget/Widget.tsx(7 hunks)react/lib/components/Widget/WidgetContainer.tsx(6 hunks)react/lib/example-config.json(1 hunks)react/lib/util/constants.ts(1 hunks)react/lib/util/types.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- react/lib/components/PaymentDialog/PaymentDialog.tsx
- docs/zh-cn/_sidebar.md
- react/lib/components/Widget/Widget.tsx
- react/lib/util/types.ts
- paybutton/src/index.tsx
- docs/_sidebar.md
- react/lib/components/Widget/WidgetContainer.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
react/lib/components/PayButton/PayButton.tsx (1)
react/lib/util/constants.ts (1)
DEFAULT_DONATE_RATE(21-21)
🪛 markdownlint-cli2 (0.18.1)
docs/README.md
1111-1111: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
1111-1111: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
1111-1111: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
1117-1117: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
1117-1117: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
1123-1123: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
1123-1123: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
docs/zh-cn/README.md
1110-1110: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
1110-1110: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
1110-1110: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
1116-1116: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
1116-1116: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
1122-1122: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
1122-1122: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
docs/zh-tw/README.md
1106-1106: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
1106-1106: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
1106-1106: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
1112-1112: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
1112-1112: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
1118-1118: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
1118-1118: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
⏰ 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). (2)
- GitHub Check: Run unit tests
- GitHub Check: Run build
🔇 Additional comments (4)
react/lib/components/PayButton/PayButton.tsx (3)
94-95: LGTM! Default values are appropriately set.The defaults correctly use
DEFAULT_DONATE_RATE(2%) and fall back toconfig.donationAddress. Ensure the config import verification passes to confirm this works at runtime.
355-356: LGTM! Props correctly forwarded to PaymentDialog.The
donationAddressanddonationRateprops are properly passed through toPaymentDialog, following the established pattern for prop forwarding in this component.
94-96: PR description does not match implementation: donation logic is not explicitly fiat-specific.The PR states: "If currency is fiat and donationRate is above 5, the component updates the state amount to amount + donation amount."
However, at
Widget.tsx:641, the code only checksif (donationRate && donationRate >= 5)without an explicitisFiat()check. The donation logic executes wheneverhasPriceis true (meaningprops.priceis provided and > 0), regardless of currency type. This is broader than the PR description suggests—it applies to any currency with price data, not just fiat currencies.Verify whether this behavior is intentional or if an explicit
isFiat(currency)check should be added before the donation calculation.react/lib/example-config.json (1)
19-19: Address is valid and already in use.The donation address
ecash:qp2v7kemclu7mv5y3h9qprwp0mrevkqt9gprvmm7ylis a valid XEC address that is already validated by theisValidXecAddress()function using the xecaddrjs library throughout the codebase. The same address is already extensively used in demo files and example configurations, confirming its validity.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (6)
react/lib/components/Widget/Widget.tsx (3)
615-615: Missing donation props in useEffect dependency array.The useEffect at lines 559-615 uses
donationRate(line 576) but the dependency array on line 615 doesn't include it. This can cause the effect to use stale donation values.Apply this diff:
- }, [to, thisCurrencyObject, price, thisAmount, opReturn, hasPrice, isCashtabAvailable]) + }, [to, thisCurrencyObject, price, thisAmount, opReturn, hasPrice, isCashtabAvailable, donationRate, donationAddress])Based on learnings.
15-15: CRITICAL: Non-existent config file import will break the build.Same issue as PayButton.tsx—this import references a file that doesn't exist.
Apply this fix:
-import config from '../../paybutton-config.json' +import config from '../../example-config.json'
691-725: Missing donation props in resolveUrl dependency array causes stale closures.The
resolveUrlcallback usesdonationAddress(line 700),donationRate(line 700, 705-706),donationAmount(line 706), andopReturn(line 717), but the dependency array on line 724 only includes[disabled, to, opReturn]. This causes stale closures where the callback won't see updated donation values.Apply this diff:
[disabled, to, opReturn] + [disabled, to, opReturn, donationAddress, donationRate, donationAmount] )Based on learnings.
react/lib/components/PayButton/PayButton.tsx (2)
6-6: CRITICAL: Non-existent config file import will break the build.The import references
'../../paybutton-config.json'which does not exist in the repository (confirmed by past reviews). This causes TypeScript compilation to fail and blocks the PR from merging.Apply one of these fixes:
Option 1 (recommended): Update the import to use the existing
example-config.json:-import config from '../../paybutton-config.json' +import config from '../../example-config.json'Option 2: Create
paybutton-config.jsonatreact/lib/paybutton-config.jsonwith the required structure:{ "donationAddress": "bitcoincash:qr..." }
211-211: CRITICAL: donationRate passed to checkSuccessInfo but never consumed in validation logic.Past reviews confirmed that
donationRateis added toCheckSuccessInfotype butshouldTriggerOnSuccess(react/lib/util/validate.ts) does not accept or use it. All call sites in react/lib/util/socket.ts and react/lib/components/Widget/WidgetContainer.tsx also fail to forward it.Either:
- Add
donationRateparameter toshouldTriggerOnSuccess, update all call sites (socket.ts lines for txsListener/onMessage, WidgetContainer.tsx handlePayment at ~line 195), and implement donation-aware validation logic, OR- Remove
donationRatefromCheckSuccessInfotype (react/lib/util/types.ts line 158) and stop passing it here if success detection should not account for donations.Based on learnings.
react/lib/components/Widget/WidgetContainer.tsx (1)
147-149: Avoid reassigning props—use a separate variable for the effective value.Directly reassigning
donationRate(a destructured parameter) is confusing and makes the code harder to reason about.Apply this diff:
- if (donationRate === undefined){ - donationRate = DEFAULT_DONATE_RATE - } + const effectiveDonationRate = donationRate ?? DEFAULT_DONATE_RATE;Then update all references to
donationRatein this file to useeffectiveDonationRate(lines 249, 328).As per coding guidelines.
🧹 Nitpick comments (2)
react/lib/components/Widget/Widget.tsx (2)
574-595: Extract magic number 5 as a named constant.The hardcoded threshold
5on line 576 controls when donation amounts are added to the displayed total. This business logic should be self-documenting.Add a constant at the top of the file or in
react/lib/util/constants.ts:export const MINIMUM_DONATION_RATE_FOR_DISPLAY = 5;Then use it:
- if ( donationRate && donationRate >= 5){ + if ( donationRate && donationRate >= MINIMUM_DONATION_RATE_FOR_DISPLAY){Based on learnings.
708-708: Minor: Inconsistent spacing around+=operator.Line 708 uses
thisUrl+=without spaces, while line 710 usesthisUrl +=with a space.Apply this diff for consistency:
- thisUrl+=`?amount=${amount}` + thisUrl += `?amount=${amount}`
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
paybutton/src/index.tsx(1 hunks)react/lib/components/PayButton/PayButton.tsx(6 hunks)react/lib/components/PaymentDialog/PaymentDialog.tsx(3 hunks)react/lib/components/Widget/Widget.tsx(7 hunks)react/lib/components/Widget/WidgetContainer.tsx(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- react/lib/components/PaymentDialog/PaymentDialog.tsx
- paybutton/src/index.tsx
🧰 Additional context used
🧬 Code graph analysis (3)
react/lib/components/Widget/WidgetContainer.tsx (1)
react/lib/util/constants.ts (1)
DEFAULT_DONATE_RATE(21-21)
react/lib/components/PayButton/PayButton.tsx (1)
react/lib/util/constants.ts (1)
DEFAULT_DONATE_RATE(21-21)
react/lib/components/Widget/Widget.tsx (3)
react/lib/util/constants.ts (5)
DEFAULT_DONATE_RATE(21-21)CURRENCY_PREFIXES_MAP(14-17)CRYPTO_CURRENCIES(3-3)DECIMALS(5-9)DEFAULT_MINIMUM_DONATE_AMOUNT(23-23)react/lib/util/format.ts (2)
isPropsTrue(66-75)amount(5-12)react/lib/util/satoshis.ts (1)
getCurrencyObject(8-41)
🔇 Additional comments (8)
react/lib/components/PayButton/PayButton.tsx (3)
60-61: Good: Donation props properly typed and defaulted.The new props are correctly typed as optional, and sensible defaults are provided using
DEFAULT_DONATE_RATEand the config file's donation address.Also applies to: 94-95
355-356: Donation props correctly forwarded to PaymentDialog.The props are passed down to enable the donation flow in child components.
22-23: The original review comment is incorrect. ButtonSize IS used in this file on line 58 as a type annotation (size?: ButtonSize;). The import on line 22 is legitimate and necessary for this usage. No changes are required.Likely an incorrect or invalid review comment.
react/lib/components/Widget/WidgetContainer.tsx (2)
22-22: Good: Donation constants and props properly imported and typed.The import of
DEFAULT_DONATE_RATEand addition of optional donation props to the interface are correctly implemented.Also applies to: 55-56
327-328: Donation props correctly forwarded to Widget.Props are passed through to the Widget component as expected.
react/lib/components/Widget/Widget.tsx (3)
37-38: Good: Donation constants and defaults properly imported and applied.The new constants and props are correctly integrated with sensible defaults.
Also applies to: 106-107, 163-164
258-263: Donation amount state properly initialized.The
donationAmountstate is correctly initialized to track the computed donation portion for multi-output URIs.
700-711: Address the toFixed(undefined) precision loss issue.When toFixed receives undefined, it defaults to 0 decimal places, causing donation amounts to lose precision when the network lookup fails (making
decimalsundefined on line 704). This contradicts the review comment's claim of safe handling.Additionally, the value and units of
DEFAULT_MINIMUM_DONATE_AMOUNT = 10need verification—confirm whether 10 represents crypto units or if unit conversion is missing.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
react/lib/components/Widget/WidgetContainer.tsx (1)
235-250: Remove unuseddonationRatefromhandlePaymentdependencies.
donationRateis listed in theuseCallbackdependency array but never referenced insidehandlePayment, so changes to it unnecessarily recreate the callback without affecting behavior. You can safely drop it:- [ + [ onSuccess, onTransaction, enqueueSnackbar, hideToasts, sound, txSound, cryptoAmount, successText, to, thisPaymentId, altpaymentShift, thisPrice, currencyObj, - randomSatoshis, - donationRate + randomSatoshis, ],react/lib/components/Widget/Widget.tsx (3)
616-616: Include donationRate in fiat conversion effect dependencies.The effect that computes donation-aware display text and sets
donationAmountusesdonationRatebut doesn’t list it in the dependency array, so changingdonationRatewon’t recompute the text/amount:if (donationRate && donationRate >= DONATION_RATE_FIAT_THRESHOLD) { // ... }Consider adding
donationRate:- }, [to, thisCurrencyObject, price, thisAmount, opReturn, hasPrice, isCashtabAvailable]) + }, [to, thisCurrencyObject, price, thisAmount, opReturn, hasPrice, isCashtabAvailable, donationRate])
692-725: Harden multi-output URI construction and fix stale dependencies inresolveUrl.
resolveUrlnow depends on donation-related state but:
- Doesn’t sanitize
donationAddress(could include a scheme/prefix).- Doesn’t robustly parse
donationRate(assumes truthy and numeric).- Omits
donationAddress/donationRate/donationAmountfrom theuseCallbackdeps, so changes won’t update the URL.Suggested tightening:
- const resolveUrl = useCallback((currency: string, amount?: number) => { + const resolveUrl = useCallback((currency: string, amount?: number) => { if (disabled || !to) return; const prefix = CURRENCY_PREFIXES_MAP[currency.toLowerCase() as typeof CRYPTO_CURRENCIES[number]]; if (!prefix) return; let thisUrl = `${prefix}:${to.replace(/^.*:/, '')}`; if (amount) { - if (donationAddress && donationRate && Number(donationRate)) { + const numericDonationRate = + donationRate != null ? Number(donationRate) : NaN; + if (donationAddress && !Number.isNaN(numericDonationRate) && numericDonationRate > 0) { const network = Object.entries(CURRENCY_PREFIXES_MAP).find( ([, value]) => value === prefix )?.[0]; - const decimals = network ? DECIMALS[network.toUpperCase()] : undefined; - const donationPercent = donationRate / 100 - const thisDonationAmount = donationAmount ? donationAmount : amount * donationPercent + const decimals = network ? DECIMALS[network.toUpperCase()] : DECIMALS.BCH; + const donationPercent = numericDonationRate / 100; + const thisDonationAmount = donationAmount ?? (amount * donationPercent); const minimumDonationAmount = network ? DEFAULT_MINIMUM_DONATION_AMOUNT[network.toUpperCase()] : 0; - thisUrl+=`?amount=${amount}` - if(thisDonationAmount > minimumDonationAmount){ - thisUrl += `&addr=${donationAddress}&amount=${thisDonationAmount.toFixed(decimals)}`; + thisUrl += `?amount=${amount}`; + if (thisDonationAmount > minimumDonationAmount) { + const cleanDonationAddr = donationAddress.replace(/^.*:/, ''); + thisUrl += `&addr=${cleanDonationAddr}&amount=${thisDonationAmount.toFixed(decimals)}`; } - }else{ - thisUrl += `?amount=${amount}` + } else { + thisUrl += `?amount=${amount}`; } } if (opReturn) { const separator = thisUrl.includes('?') ? '&' : '?'; thisUrl += `${separator}op_return_raw=${opReturn}`; } return thisUrl; - }, - [disabled, to, opReturn] - ) + }, [disabled, to, opReturn, donationAddress, donationRate, donationAmount])This keeps behavior the same for valid numeric rates while making URL construction more robust and ensuring the callback tracks donation-related changes.
259-265: Critical: donationAmount stores total instead of donation portion, leading to overcharge.In the fiat-price branch,
donationAmountis currently set to the total converted amount (base + donation):const convertedDonationAmount = convertedObj.float * (donationRate / 100) const convertedAmountWithDonation = convertedObj.float + convertedDonationAmount const convertedAmountWithDonationObj = getCurrencyObject( convertedAmountWithDonation, thisAddressType, randomSatoshis, ) convertedAmountToDisplay = convertedAmountWithDonationObj.string setDonationAmount(convertedAmountWithDonationObj.float)Later,
resolveUrltreatsdonationAmountas the donation output:const donationPercent = donationRate / 100 const thisDonationAmount = donationAmount ? donationAmount : amount * donationPercentSo the QR encodes outputs: base
amountplus a “donation” equal tobase + donation, i.e., total paid becomesamount + (amount + donation)instead ofamount + donation.Store only the donation portion in
donationAmount:- const convertedDonationAmount = convertedObj.float * (donationRate / 100) - const convertedAmountWithDonation = convertedObj.float + convertedDonationAmount - const convertedAmountWithDonationObj = getCurrencyObject( - convertedAmountWithDonation, - thisAddressType, - randomSatoshis, - ) - convertedAmountToDisplay = convertedAmountWithDonationObj.string - setDonationAmount(convertedAmountWithDonationObj.float) + const convertedDonationAmount = convertedObj.float * (donationRate / 100) + const convertedAmountWithDonation = convertedObj.float + convertedDonationAmount + const convertedAmountWithDonationObj = getCurrencyObject( + convertedAmountWithDonation, + thisAddressType, + randomSatoshis, + ) + convertedAmountToDisplay = convertedAmountWithDonationObj.string + // Track only the donation portion for the secondary output + setDonationAmount(convertedDonationAmount)This keeps the displayed totals the same while ensuring the QR encodes a donation output equal to just the donation portion.
Also applies to: 575-596
🧹 Nitpick comments (1)
react/lib/components/Widget/WidgetContainer.tsx (1)
55-57: Donation props wiring is correct; consider avoiding prop-like mutation and duplicate defaults.
donationAddress/donationRateare correctly threaded intoWidget, and usingDEFAULT_DONATION_RATEwhendonationRateisundefinedmatches the intended default behavior.For readability and to avoid duplicating defaulting logic already present in
Widget, you could prefer an explicit derived variable instead of mutatingdonationRate:- let { + let { // ... - donationRate, + donationRate, ...widgetProps } = props; - if (donationRate === undefined){ - donationRate = DEFAULT_DONATION_RATE - } + const effectiveDonationRate = + donationRate === undefined ? DEFAULT_DONATION_RATE : donationRate;and then pass
effectiveDonationRatetoWidget.Also applies to: 137-149, 327-328
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
react/lib/components/PayButton/PayButton.tsx(6 hunks)react/lib/components/Widget/Widget.tsx(7 hunks)react/lib/components/Widget/WidgetContainer.tsx(6 hunks)react/lib/util/constants.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- react/lib/components/PayButton/PayButton.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
react/lib/components/Widget/Widget.tsx (3)
react/lib/util/constants.ts (6)
DEFAULT_DONATION_RATE(21-21)DONATION_RATE_FIAT_THRESHOLD(23-23)CURRENCY_PREFIXES_MAP(14-17)CRYPTO_CURRENCIES(3-3)DECIMALS(5-9)DEFAULT_MINIMUM_DONATION_AMOUNT(26-29)react/lib/util/format.ts (2)
isPropsTrue(66-75)amount(5-12)react/lib/util/satoshis.ts (1)
getCurrencyObject(8-41)
react/lib/components/Widget/WidgetContainer.tsx (1)
react/lib/util/constants.ts (1)
DEFAULT_DONATION_RATE(21-21)
⏰ 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/util/constants.ts (1)
19-29: Donation-related constants and ButtonSize typing look consistent.
DEFAULT_DONATION_RATE,DONATION_RATE_FIAT_THRESHOLD, andDEFAULT_MINIMUM_DONATION_AMOUNTare coherent with existingDECIMALS/ticker usage, and the expandedButtonSizeunion preserves backwards compatibility.react/lib/components/Widget/Widget.tsx (1)
15-40: Donation props/config defaults are wired coherently.Importing config and donation-related constants, extending
WidgetPropswithdonationAddress/donationRate, and defaulting them via:donationAddress = config.donationAddress, donationRate = DEFAULT_DONATION_RATEmatches the PR’s intent and keeps a sensible global default.
Also applies to: 107-109, 164-166
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| if (convertedObj) { | ||
| let amountToDisplay = thisCurrencyObject.string; | ||
| let convertedAmountToDisplay = convertedObj.string | ||
| if ( donationRate && donationRate >= 5){ |
There was a problem hiding this comment.
No magic number. Make this 5 a constant imported from the constants file.
Related to #326
Description
Implemented configurable donateRate prop and fixed donationAddress prop
If currency is fiat and donation rate above 5 we change the state amount to the amount plus donation amount
Test plan
Summary by CodeRabbit
New Features
Documentation