Hosted buttons component #2276
Hosted buttons component #2276
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2276 +/- ##
=======================================
Coverage 51.30% 51.30%
=======================================
Files 104 104
Lines 2037 2037
Branches 604 604
=======================================
Hits 1045 1045
Misses 889 889
Partials 103 103
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
jshawl
force-pushed
the
hosted-buttons-component
branch
2 times, most recently
from
fe860b2 to
4fb5d3e
Compare
jshawl
force-pushed
the
hosted-buttons-component
branch
4 times, most recently
from
4fcdeed to
ce7a68a
Compare
jshawl
force-pushed
the
hosted-buttons-component
branch
from
0e9bb77 to
2364799
Compare
jshawl
force-pushed
the
hosted-buttons-component
branch
2 times, most recently
from
1407d26 to
174fd15
Compare
There was a problem hiding this comment.
Nice work, @jshawl! It's looking good! Just a couple minor questions/comments for you.
src/hosted-buttons/utils.js
Outdated
| variables?.find((variable) => variable.name === key)?.value ?? ""; | ||
|
|
||
| const getFundingSource = (paymentSource) => { | ||
| if (paymentSource === "credit") { |
There was a problem hiding this comment.
Maybe we could import these values from FUNDING in @paypal/sdk-constants/src instead of hardcoding "credit" and 'CARD'?
There was a problem hiding this comment.
yes! I like it 72e75b0
| if (elm) { | ||
| elm.innerHTML = html + htmlScript; | ||
| const newScriptEl = document.createElement("script"); | ||
| const oldScriptEl = elm.querySelector("script"); |
There was a problem hiding this comment.
Will there only ever be one script per element? I'm assuming so, but thought I'd doublecheck.
There was a problem hiding this comment.
that is right! this is only because it is sent as one property in the response here - https://github.com/paypal/paypal-checkout-components/pull/2276/files#diff-dc0ab5efddb0f4c5dc8c699ccafce1daa87906e1b839480c5be57048cee5482bR88
| entry_point: entryPoint, | ||
| funding_source: getFundingSource(data.paymentSource), | ||
| merchant_id: merchantId, | ||
| ...userInputs, |
There was a problem hiding this comment.
Is this a possible vector for malicious JS that needs to be guarded against?
There was a problem hiding this comment.
I'm having trouble thinking of an attack vector here but only because the user input is never put back into the DOM; it will be sent to the hosted buttons API though. I think we're good here but lmk if you had a sanitization strategy in mind and/or if you have an idea for how to exploit.
There was a problem hiding this comment.
I can't think of anything either, unless the API executes a request body value (which seems unlikely).
jshawl
force-pushed
the
hosted-buttons-component
branch
from
b005bc1 to
8559586
Compare
Description
This PR implements a new
paypal.HostedButtons()component. Example usage:Why are we making these changes?
This new component will allow merchants to configure styles, prices, and quantities through a hosted UI. These changes then fetch the details of that hosted button and render them using v5 of the JS SDK.
Reproduction Steps
N/A - the Hosted Buttons UI is not publicly available
❤️ Thank you!