feat(payment): PI-3068 moved my account logic to hosted card v2 package #2767
Conversation
vitalii-koshovyi
force-pushed
the
PI-3068
branch
3 times, most recently
from
e1f494f to
9c691e9
Compare
| typeof _orderId !== 'undefined' | ||
| ? `/admin/payments/${this._orderId}/hosted-form-field?version=${LIBRARY_VERSION}` | ||
| : `/checkout/payment/hosted-field?version=${LIBRARY_VERSION}`; |
There was a problem hiding this comment.
| typeof _orderId !== 'undefined' | |
| ? `/admin/payments/${this._orderId}/hosted-form-field?version=${LIBRARY_VERSION}` | |
| : `/checkout/payment/hosted-field?version=${LIBRARY_VERSION}`; | |
| !(_orderId) | |
| ? `/checkout/payment/hosted-field?version=${LIBRARY_VERSION}` | |
| : `/admin/payments/${this._orderId}/hosted-form-field?version=${LIBRARY_VERSION}` |
There was a problem hiding this comment.
I'm not sure about that, theoretically orderId could be equal 0, in that case yours suggestion could cause issues.
Generally, I thought to pass some kind of a flag like isControlPannelframe but it looks excessive.
There was a problem hiding this comment.
I personally don't mind the idea of a function like getFrameSrc that returns a url based on some params passed. Since this could eventually be rolled out for checkout page or some other context.
There was a problem hiding this comment.
Also AFAIK orderId should never be 0 in this case, but don't mind the typeof check. Can also do !!(_orderId) which checks if the value is truthy or not.
There was a problem hiding this comment.
!!(_orderId) linter will not pass it
There was a problem hiding this comment.
I will create a getFrameSrc, it'll look a lot cleaner.
| fieldOptions.placeholder || '', | ||
| fieldOptions.accessibilityLabel || '', | ||
| options.styles || {}, | ||
| new IframeEventPoster(host), | ||
| new IframeEventListener(host), | ||
| new DetachmentObserver(new MutationObserverFactory()), | ||
| options.orderId, |
There was a problem hiding this comment.
Is there a reason we are moving the order here?
There was a problem hiding this comment.
Yes, because it's not required param anymore, due to the usage with vaulting functionality.
By our linting rules it should be after required params.
|
|
||
| export interface HostedInputStoredCardErrorEvent { | ||
| type: HostedInputEventType.StoredCardFailed; | ||
| payload?: { |
There was a problem hiding this comment.
Will there ever be a case that payload is not present?
There was a problem hiding this comment.
Saw this https://github.com/bigcommerce/checkout-sdk-js/pull/2767/files#diff-7bea3ab0d43b6b804c7f55d7e6f0d3bd485fde61e420afbd8265aa6942554770R58 so makes sense, but shall we return a payload there?
There was a problem hiding this comment.
Yes, we had similar comment in the initial PR. It gonna be done in separate ticket.
We need to improve error handling in general for the Stored cards functionality.
There was a problem hiding this comment.
We need to make strict types for the possible endpoint response errors, so it'll be done in the future My account developing, feature is still under development, not released yet.
…ge in the checkout-sdk
vitalii-koshovyi
force-pushed
the
PI-3068
branch
from
9c691e9 to
f449944
Compare
What?
Moved my account logic to the hosted card v2 package.
But still need to improve coverage for the submitManualOrderForm and submitStoredCardForm methods, didn't do that due to the already huge changes;
Why?
Storefront account payments could use hosted fields without importing whole SDK.
Testing / Proof
manual-orders.mov
My.Account.mov
*logs in the console show that modified version of SDK loaded both in the microapp and in the iframe
@bigcommerce/team-checkout @bigcommerce/team-payments