Add the PaymentRequestButtonElement#100
Conversation
README.md
Outdated
| }); | ||
|
|
||
| paymentRequest.on('token', ({complete, ...data}) => { | ||
| console.log('received Stripe token': data); |
There was a problem hiding this comment.
The token would be data.token, right? How about, Received customer information:
And maybe a separate log for the token?
README.md
Outdated
| }); | ||
|
|
||
| paymentRequest.canMakePayment().then(result => { | ||
| this.setState({canMakePayment: result}); |
There was a problem hiding this comment.
result is not a boolean -- let's do !!result
| }; | ||
| ``` | ||
|
|
||
| The props for the `PaymentRequestButtonElement` are: |
There was a problem hiding this comment.
onFocus and onBlur are also available!
Should we make this type a union type of ElementProps and {paymentRequest: ...}?
There was a problem hiding this comment.
We could! It has no onChange prop, though.
The only reason I made this a separate component, instead of using the existing Element component, was to get more specific prop validation and types.
There was a problem hiding this comment.
ACK, makes sense. Thinking about it more, maybe let's hold off on exposing more API surface area unless folks need it.
|
Looks good generally; good set of tests! I think this Element shares more with the other Elements than we're exposing here, though. ptal @cweiss-stripe |
|
lgtm! |
jez
left a comment
jez
left a comment
There was a problem hiding this comment.
Mostly just questions for my own understanding. Feel free to merge unless you feel strongly about making any of the suggested changes.
README.md
Outdated
|
|
||
| The [Payment Request Button](https://stripe.com/docs/elements/payment-request-button) lets you collect payment and address information from your customers using Apple Pay and the Payment Request API. | ||
|
|
||
| To use the `PaymentRequestButtonElement` you need to first create a [`PaymentRequest` Object](https://stripe.com/docs/stripe.js#the-payment-request-object). You can then conditionally render the `PaymentRequestButtonElement` based on the result of `paymentRequest.canMakePayment` and pass the `PaymentRequest` Object as a prop. |
There was a problem hiding this comment.
We downcase "object" (in PaymentRequest object and others) throughout our pages on https://stripe.com/docs.
In the longer term, I'm still bullish on moving the docs in this README into /docs for consistency.
| this.context.unregisterElement(this._element); | ||
| } | ||
| props: Props; | ||
|
|
There was a problem hiding this comment.
Maybe I'm missing something, but what did this get removed for? Seems like maybe we're not being as strict with types as we could be, but maybe we're making up for this somewhere else.
There was a problem hiding this comment.
New Flow does not require Props to be declared this way. (See the class signature)
| on: Function, | ||
| show: Function, | ||
| }, | ||
| }; |
There was a problem hiding this comment.
I imagine we're using Function here because that's how it was done in the old Element component. We could get more type safety using Flow's SyntheticEvent<T> types: https://flow.org/en/docs/react/events/. Not sure if this is important enough to block.
There was a problem hiding this comment.
Hmm, but the events are not DOM Events, but events from the wrapped Stripe Element component. We could still be more specific, though! I'll leave that for a separate PR.
| }, | ||
| }; | ||
|
|
||
| const noop = () => {}; |
There was a problem hiding this comment.
For example, if we were using SyntheticEvent<T> for our props, I imagine Flow would catch that noop should take a single event argument, instead of "no" arguments.
4 participants
This PR adds the
PaymentRequestButtonElement(+tests, +demo, +docs)