-
Notifications
You must be signed in to change notification settings - Fork 943
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use confirmCardPayment instead of deprecated handleCardPayment #1339
Conversation
5475e3d
to
d01b149
Compare
@@ -469,7 +469,7 @@ | |||
"PaymentMethodsForm.billingDetailsNamePlaceholder": "Escribe el nombre…", | |||
"PaymentMethodsForm.paymentCardDetails": "Datos de la tarjeta de crédito", | |||
"PaymentMethodsForm.genericError": "Ha habido algún problema con la información de pagos. Por favor, inténtalo de nuevo.", | |||
"PaymentMethodsForm.handleCardPaymentError": "No hemos podido autenticar el método de pago. Por favor checa los detalles de autenticación e intenta de nuevo.", | |||
"PaymentMethodsForm.confirmCardPaymentError": "No hemos podido autenticar el método de pago. Por favor checa los detalles de autenticación e intenta de nuevo.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you check that WTI keeps up with these?
I'm not sure if it fetches these changes automatically or if they need to be changed manually there. I just have a feeling that last time there was strange keys that contained english - and it could be because we have renamed translation keys only on Github.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Gnito It seems that the renamed keys are showing in WTI as new keys, so it doesn't keep up with renaming but there should not be keys containing English either. Should I do the manual updates (as only the key changed and not the content) or should they wait for translators to check them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since there's just one key, I guess you could copy-paste the content in WTI. (It should end up to translator's proofreading queue)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't test this, but LGTM.
d01b149
to
754d37f
Compare
Use Stripe's
confirmCardPayment
function instead of deprecatedhandleCardPayment
to confirm PaymentIntent. In addition to the rename, the arguments passed tohandleCardPayment
are slightly different. Otherwise, these changes should not affect the behavior of the function.See https://stripe.com/docs/js/deprecated/handle_card_payment
The biggest difference when using the new function is that the card element is passed inside the
payment_method
object and not directly. Also, there is nopayment_method_data
object but e.g. billing information should be also insidepayment_method
: https://stripe.com/docs/js/payment_intents/confirm_card_paymentThis also means that we need to rename a bunch of other functions that had
handleCardPayment
in their name. It generates more diff but I think it's better to change all the naming toconfirmCardPayment
so that the relationship is still clear.(I also added missing translation key for CheckoutPage.loadingData which has been missing for a while)