Add an option to hide the send button in the widget#624
Add an option to hide the send button in the widget#624Klakurka merged 4 commits intoPayButton:masterfrom
Conversation
Only the QR code and the decorations remain. This makes more sense in the context of a PoS. Closes PayButton#613. Test Plan: yarn test Try it using the paybutton-generator.html page
📝 WalkthroughWalkthroughA feature addition implementing the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
|
Wasn't working, so I fixed by adding the new props to |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
react/lib/tests/components/PayButton.test.tsx (1)
472-570: NewhideSendButtontests cover the key scenarios — consider adding fiat coverage.The three scenarios (
true,false,undefined) are correctly structured: each waits forcreatePaymentto fire and the loading state to clear before asserting button visibility. No issues with the logic or assertions.One optional gap: all three cases use
CRYPTO_CASES. Since fiat flows go through the same Widget rendering path for this prop, addingFIAT_CASES(orALL_CASES) would give fuller confidence, particularly for the default-undefined case.♻️ Suggested expansion to cover fiat cases
- test.each(CRYPTO_CASES)( + test.each(ALL_CASES)( 'send button is hidden in dialog when hideSendButton=true (%s)', - test.each(CRYPTO_CASES)( + test.each(ALL_CASES)( 'send button is visible in dialog when hideSendButton=false (%s)', - test.each(CRYPTO_CASES)( + test.each(ALL_CASES)( 'send button is visible by default (hideSendButton undefined) (%s)',Note: fiat cases require a double-click to open (same pattern as other fiat tests in this file, e.g. line 109):
// Open the dialog await user.click(screen.getByRole('button', { name: /donate/i })) + if (isFiat(currency)) { + await user.click(screen.getByRole('button', { name: /donate/i })) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@react/lib/tests/components/PayButton.test.tsx` around lines 472 - 570, Tests only run CRYPTO_CASES; add FIAT_CASES (or combine into ALL_CASES) to the three test.each blocks so fiat flows are covered for hideSendButton=true/false/undefined, and for fiat iterations mimic existing fiat behavior by performing a double-click to open the widget (same pattern used elsewhere in the file); update the test blocks referencing CRYPTO_CASES to use FIAT_CASES or ALL_CASES and ensure the open step for fiat cases uses two user.click calls before awaiting createPayment and checking loading/send button via createPayment, screen.queryByText, screen.findByRole/queryByRole.react/lib/components/Widget/Widget.tsx (1)
1368-1391: Consider whether the altpayment link should also be suppressed whenhideSendButton=true.The stated use-case is PoS, where the operator wants to expose only the QR code. The "Don't have any X?" altpayment link (lines 1368-1391) triggers a SideShift.ai swap — effectively another wallet-interaction button. Leaving it visible while
hideSendButton=trueis in use may be inconsistent with the intent of the feature.If the goal is truly a QR-only view, consider gating the altpayment link on
!hideSendButtonas well (or introduce a separatehideAltpayment-when-hideSendButtonbehaviour). This is a design decision rather than a correctness bug, but worth aligning on before the feature ships.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@react/lib/components/Widget/Widget.tsx` around lines 1368 - 1391, The altpayment link is shown even when hideSendButton=true; update the render condition to also check hideSendButton so the link is suppressed in QR-only mode: change the top-level conditional that uses isPropsTrue(disableAltpayment) to require !isPropsTrue(hideSendButton) as well (or introduce a new prop hideAltpayment if you prefer), leaving the inner logic for cursor, onClick (tradeWithAltpayment), and classes (isAboveMinimumAltpaymentAmount, altpaymentEditable, classes.sideShiftLink, classes.animate_sideshift, classes.hide_sideshift) unchanged so the link only appears when altpayment isn’t disabled and send button is visible.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/README.md`:
- Line 847: Close the unclosed bold span in the blockquote for the
hide-send-button parameter: update the line starting with "> **The
‘hide-send-button’ parameter..." to end the bold span before the period (e.g.,
"> **The ‘hide-send-button’ parameter specifies whether the 'Send with XEC/BCH'
button should be hidden.**"), and optionally remove the redundant "or not" to
match the style used for other parameter descriptions like 'hide-toasts'.
In `@paybutton/src/index.tsx`:
- Line 85: The HTML attribute hide-send-button is read as a string and never
coerced to boolean, so set props.hideSendButton the same way hideToasts is
handled: convert attributes.hideSendButton === 'true' before assigning to
props.hideSendButton (same pattern used at attributes.hideToasts and
props.hideToasts), updating the attribute-to-prop mapping in
paybutton/src/index.tsx (look for attributes.hideSendButton and
props.hideSendButton) so "false" correctly yields false.
In `@react/lib/components/Widget/Widget.tsx`:
- Line 1352: The conditional rendering uses the bare prop hideSendButton (in the
JSX expression currently written as {success || hideSendButton ? null : (...)})
which breaks the boolean-coercion pattern; change it to use
isPropsTrue(hideSendButton) (i.e., {success || isPropsTrue(hideSendButton) ?
null : (...)}), matching how other flags like editable, disabled, and
disableAltpayment are evaluated so string-valued props like "false" don't
erroneously hide the button.
---
Nitpick comments:
In `@react/lib/components/Widget/Widget.tsx`:
- Around line 1368-1391: The altpayment link is shown even when
hideSendButton=true; update the render condition to also check hideSendButton so
the link is suppressed in QR-only mode: change the top-level conditional that
uses isPropsTrue(disableAltpayment) to require !isPropsTrue(hideSendButton) as
well (or introduce a new prop hideAltpayment if you prefer), leaving the inner
logic for cursor, onClick (tradeWithAltpayment), and classes
(isAboveMinimumAltpaymentAmount, altpaymentEditable, classes.sideShiftLink,
classes.animate_sideshift, classes.hide_sideshift) unchanged so the link only
appears when altpayment isn’t disabled and send button is visible.
In `@react/lib/tests/components/PayButton.test.tsx`:
- Around line 472-570: Tests only run CRYPTO_CASES; add FIAT_CASES (or combine
into ALL_CASES) to the three test.each blocks so fiat flows are covered for
hideSendButton=true/false/undefined, and for fiat iterations mimic existing fiat
behavior by performing a double-click to open the widget (same pattern used
elsewhere in the file); update the test blocks referencing CRYPTO_CASES to use
FIAT_CASES or ALL_CASES and ensure the open step for fiat cases uses two
user.click calls before awaiting createPayment and checking loading/send button
via createPayment, screen.queryByText, screen.findByRole/queryByRole.
|
|
||
| ## hide-send-button | ||
|
|
||
| > **The ‘hide-send-button’ parameter specifies whether or not the 'Send with XEC/BCH' button should be hidden. |
There was a problem hiding this comment.
Missing closing ** in the blockquote description.
Every other parameter description in this file closes the bold span before the period (e.g., > **The 'hide-toasts' parameter ... detected.**). Line 847 leaves the bold open.
📝 Proposed fix
-> **The 'hide-send-button' parameter specifies whether or not the 'Send with XEC/BCH' button should be hidden.
+> **The 'hide-send-button' parameter specifies whether the 'Send with XEC/BCH' button should be hidden.**(The fix above also drops the wordy "or not", per the LanguageTool style hint.)
📝 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.
| > **The ‘hide-send-button’ parameter specifies whether or not the 'Send with XEC/BCH' button should be hidden. | |
| > **The 'hide-send-button' parameter specifies whether the 'Send with XEC/BCH' button should be hidden.** |
🧰 Tools
🪛 LanguageTool
[style] ~847-~847: ‘whether or not’ might be wordy. Consider a shorter alternative.
Context: ... ‘hide-send-button’ parameter specifies whether or not the 'Send with XEC/BCH' button should b...
(EN_WORDINESS_PREMIUM_WHETHER_OR_NOT)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/README.md` at line 847, Close the unclosed bold span in the blockquote
for the hide-send-button parameter: update the line starting with "> **The
‘hide-send-button’ parameter..." to end the bold span before the period (e.g.,
"> **The ‘hide-send-button’ parameter specifies whether the 'Send with XEC/BCH'
button should be hidden.**"), and optionally remove the redundant "or not" to
match the style used for other parameter descriptions like 'hide-toasts'.
| 'currency', | ||
| 'displayCurrency', | ||
| 'hideToasts', | ||
| 'hideSendButton', |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Get context around line 85 in index.tsx
sed -n '75,95p' paybutton/src/index.tsxRepository: PayButton/paybutton
Length of output: 333
🏁 Script executed:
# Get context around line 190 in index.tsx
sed -n '180,200p' paybutton/src/index.tsxRepository: PayButton/paybutton
Length of output: 720
🏁 Script executed:
# Search for hideSendButton usage in Widget.tsx
rg 'hideSendButton' paybutton/src/ -B 2 -A 2Repository: PayButton/paybutton
Length of output: 269
🏁 Script executed:
# Check the entire allowedProps and attributes handling section
sed -n '70,210p' paybutton/src/index.tsxRepository: PayButton/paybutton
Length of output: 3695
🏁 Script executed:
# Find and examine Widget.tsx to see how hideSendButton is used
find paybutton/src -name 'Widget*' -type fRepository: PayButton/paybutton
Length of output: 45
🏁 Script executed:
# Search for how hideSendButton is conditionally used in the codebase
rg 'hideSendButton' paybutton/src -B 3 -A 3Repository: PayButton/paybutton
Length of output: 350
🏁 Script executed:
# Check if there are any boolean coercions for hideSendButton elsewhere
rg "hideSendButton.*===" paybutton/srcRepository: PayButton/paybutton
Length of output: 45
🏁 Script executed:
# Search for Widget component definition
rg 'Widget' paybutton/src --type tsx --type ts | head -20Repository: PayButton/paybutton
Length of output: 90
🏁 Script executed:
# Look for all tsx/ts files
find paybutton/src -name '*.tsx' -o -name '*.ts'Repository: PayButton/paybutton
Length of output: 86
🏁 Script executed:
# Search for where hideSendButton is actually used in component logic
rg 'hideSendButton' . -B 5 -A 5 --type tsx --type tsRepository: PayButton/paybutton
Length of output: 90
🏁 Script executed:
# List all files in paybutton/src
ls -la paybutton/src/Repository: PayButton/paybutton
Length of output: 232
🏁 Script executed:
# Search for hideSendButton without file type restrictions
rg 'hideSendButton' paybutton/ -B 5 -A 5Repository: PayButton/paybutton
Length of output: 3482
🏁 Script executed:
# Check if Widget is imported or defined
rg 'Widget' paybutton/src -B 2 -A 2Repository: PayButton/paybutton
Length of output: 1984
🏁 Script executed:
# Verify the exact lines around the hideToasts coercion to ensure nothing was missed
sed -n '185,195p' paybutton/src/index.tsxRepository: PayButton/paybutton
Length of output: 404
hideSendButton HTML attribute is never coerced to boolean — hide-send-button="false" will incorrectly hide the button.
Every HTML attribute read through el.getAttribute() arrives as a string. hideToasts is explicitly coerced at line 190:
props.hideToasts = attributes.hideToasts === 'true';hideSendButton is missing the equivalent conversion, so:
hide-send-button="true"→ string"true"→ truthy → button hidden ✓hide-send-button="false"→ string"false"→ truthy → button hidden ✗
Note: the new Widget tests pass hideSendButton as a JS boolean directly, so they bypass this code path entirely and won't catch this.
🐛 Proposed fix
props.hideToasts = attributes.hideToasts === 'true';
+ props.hideSendButton = attributes.hideSendButton === 'true';Also applies to: 190-190
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@paybutton/src/index.tsx` at line 85, The HTML attribute hide-send-button is
read as a string and never coerced to boolean, so set props.hideSendButton the
same way hideToasts is handled: convert attributes.hideSendButton === 'true'
before assigning to props.hideSendButton (same pattern used at
attributes.hideToasts and props.hideToasts), updating the attribute-to-prop
mapping in paybutton/src/index.tsx (look for attributes.hideSendButton and
props.hideSendButton) so "false" correctly yields false.
| ) : null} | ||
|
|
||
| {success ? null : ( | ||
| {success || hideSendButton ? null : ( |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check isPropsTrue usage in Widget.tsx and surrounding boolean prop checks
rg -n "isPropsTrue" react/lib/components/Widget/Widget.tsxRepository: PayButton/paybutton
Length of output: 469
🏁 Script executed:
#!/bin/bash
# Get lines around 1352 to see the hideSendButton check
sed -n '1345,1395p' react/lib/components/Widget/Widget.tsxRepository: PayButton/paybutton
Length of output: 1830
🏁 Script executed:
#!/bin/bash
# Get lines around 1276, 1360, 1368 to compare boolean prop usage
echo "=== Line 1276 (editable) ===" && sed -n '1270,1280p' react/lib/components/Widget/Widget.tsx
echo -e "\n=== Line 1360 (disabled) ===" && sed -n '1355,1365p' react/lib/components/Widget/Widget.tsx
echo -e "\n=== Line 1368 (disableAltpayment) ===" && sed -n '1363,1375p' react/lib/components/Widget/Widget.tsxRepository: PayButton/paybutton
Length of output: 1430
🏁 Script executed:
#!/bin/bash
# Verify hideSendButton prop definition and destructuring
rg -n "hideSendButton" react/lib/components/Widget/Widget.tsx | head -20Repository: PayButton/paybutton
Length of output: 173
🏁 Script executed:
#!/bin/bash
# Find the Vue demo file to check string attribute handling
find . -type f \( -name "*.vue" -o -name "*demo*.html" -o -name "*demo*.tsx" \) | head -20Repository: PayButton/paybutton
Length of output: 45
hideSendButton must be wrapped with isPropsTrue()—it is the only boolean flag prop in this component that bypasses this check.
Every other boolean flag prop uses isPropsTrue() before truthiness evaluation: editable (line 1276), disabled (line 1360), disableAltpayment (line 1368). The bare hideSendButton at line 1352 breaks this pattern and risks malfunction if the prop arrives as a string (e.g., from the Vue demo's true-value="true" false-value="false" or from Web Component HTML attribute coercion). The string "false" is truthy in JavaScript, so toggling the checkbox off would leave the button permanently hidden.
Proposed fix
- {success || hideSendButton ? null : (
+ {success || isPropsTrue(hideSendButton) ? null : (📝 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.
| {success || hideSendButton ? null : ( | |
| {success || isPropsTrue(hideSendButton) ? null : ( |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@react/lib/components/Widget/Widget.tsx` at line 1352, The conditional
rendering uses the bare prop hideSendButton (in the JSX expression currently
written as {success || hideSendButton ? null : (...)}) which breaks the
boolean-coercion pattern; change it to use isPropsTrue(hideSendButton) (i.e.,
{success || isPropsTrue(hideSendButton) ? null : (...)}), matching how other
flags like editable, disabled, and disableAltpayment are evaluated so
string-valued props like "false" don't erroneously hide the button.
3 participants
Only the QR code and the decorations remain. This makes more sense in the context of a PoS.
Closes #613.
Test Plan:
yarn test
Try it using the paybutton-generator.html page
Summary by CodeRabbit
New Features
hide-send-buttonparameter to control send button visibility in PayButtonDocumentation
hideToastsparameter naming in all code examplesTests