fix: Clicking on the PayButton in Button Generator #1084
Conversation
WalkthroughThe ButtonGenerator component now conditionally passes props to PayButtonWidget and PayButton. The amount prop becomes undefined when empty instead of always being parsed; randomSatoshis is only passed when amount is non-empty; onOpen and onClose become undefined when their corresponding fields are empty. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (1)
components/ButtonGenerator/index.tsx(4 hunks)
⏰ 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 Tests
🔇 Additional comments (2)
components/ButtonGenerator/index.tsx (2)
431-432: LGTM! Dialog-closing behavior fix looks correct.Passing
undefinedinstead of empty strings foronOpenandonCloseprops correctly addresses the dialog-closing behavior issue mentioned in the PR objectives. This allows the components to handle these callbacks properly when they're not provided.
401-401: Line 401 is correct. BothPayButtonandWidgetfrom@paybutton/reactexpect theamountprop as a string. Line 401 correctly passesbutton.amountas a string. The type inconsistency actually exists at line 366, wherePayButtonWidgetincorrectly receivesparseFloat(button.amount)(a number) instead of the string value.Likely an incorrect or invalid review comment.
| key={`widget-${JSON.stringify(button)}`} | ||
| to={button.to} | ||
| amount={parseFloat(button.amount)} | ||
| amount={button.amount === '' ? undefined : parseFloat(button.amount)} |
There was a problem hiding this comment.
Inconsistent checks for amount and randomSatoshis logic.
Line 366 correctly parses amount to float for the widget. However, line 388's check button.amount !== '' is inconsistent with the UI visibility logic at line 325, which uses parseFloat(button.amount) > 0. This mismatch means that if a user enters "0" or "0.0", the randomSatoshis field won't be shown in the form (line 325 returns null), but the prop would still be passed to the component (line 388 would evaluate to true).
Consider aligning the check with the UI visibility logic:
- randomSatoshis={button.amount !== '' ? button.randomSatoshis : undefined}
+ randomSatoshis={parseFloat(button.amount) > 0 ? button.randomSatoshis : undefined}Also applies to: 388-388
🤖 Prompt for AI Agents
In components/ButtonGenerator/index.tsx around lines 325, 366 and 388, the
visibility logic uses parseFloat(button.amount) > 0 but the prop passing uses
button.amount !== '' which causes a mismatch for "0" values; update the prop
checks to mirror the UI logic by parsing the amount and only treating it as
present when parseFloat(amount) > 0, i.e. pass amount={parseFloat(button.amount)
> 0 ? parseFloat(button.amount) : undefined} and set randomSatoshis to be passed
only when parseFloat(button.amount) > 0 (handle NaN by treating non-numeric or
<=0 as absent), ensuring consistent behavior and guarding with Number.isFinite
or isNaN checks.
| : button.onTransaction | ||
| } | ||
| randomSatoshis={button.randomSatoshis} | ||
| randomSatoshis={button.amount !== '' ? button.randomSatoshis : undefined} |
There was a problem hiding this comment.
Inconsistent check for randomSatoshis (same as line 388).
The check button.amount !== '' is inconsistent with the UI visibility logic at line 325 (parseFloat(button.amount) > 0). This could cause the prop to be passed even when the form field is hidden (e.g., when amount is "0").
Apply the same fix as suggested for line 388:
- randomSatoshis={button.amount !== '' ? button.randomSatoshis : undefined}
+ randomSatoshis={parseFloat(button.amount) > 0 ? button.randomSatoshis : undefined}📝 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.
| randomSatoshis={button.amount !== '' ? button.randomSatoshis : undefined} | |
| randomSatoshis={parseFloat(button.amount) > 0 ? button.randomSatoshis : undefined} |
🤖 Prompt for AI Agents
In components/ButtonGenerator/index.tsx around line 423, the prop randomSatoshis
is conditionally passed using button.amount !== '' which is inconsistent with
the UI visibility check (parseFloat(button.amount) > 0); change the condition to
use parseFloat(button.amount) > 0 so randomSatoshis is only passed when the
numeric amount is greater than zero (i.e., replace the current check with the
same parseFloat(button.amount) > 0 guard used elsewhere).
Related to #1080
Description
PayButton generator not working unless USD value is set.
Test plan
Open the landing page, setup a button using the Button Generator. Click it to open the dialog. Try a bunch of combinations of things. Also make sure dialog closing works as that was broken regardless of if USD was set or not.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.