CHECKOUT-3477: Show loading indicator while waiting for iframe to load #429
Conversation
|
|
||
| messageListener.trigger({ type: EmbeddedCheckoutEventType.FrameLoaded }); | ||
|
|
||
| expect(messagePoster.post).toHaveBeenCalledTimes(2); |
There was a problem hiding this comment.
should you check the arguments to make sure it reconfigured styles?
| it('toggles loading indicator', async () => { | ||
| await embeddedCheckout.attach(); | ||
|
|
||
| expect(loadingIndicator.show).toHaveBeenCalled(); |
There was a problem hiding this comment.
can you do something like, it toggles loading indicator after checkout has loaded?
expect(loadingIndicator.show).toHaveBeenCalled();
expect(loadingIndicator.hide).not.toHaveBeenCalled();
frameLoaded()
expect(loadingIndicator.hide).toHaveBeenCalled();
you could also check it hides if frame failed to load.
|
|
||
| const ROTATION_ANIMATION = 'embedded-checkout-loading-indicator-rotation'; | ||
|
|
||
| export default class LoadingIndicator { |
There was a problem hiding this comment.
I have a few doubts with the naming, maybe this could be LoadingOverlay? Indicator to me sounds like just the icon. 🍹
_container could become _overlay
There was a problem hiding this comment.
I understand where you coming from. But in this case, the indicator is in fact the spinner icon that gets shown to the shopper when the page is initially loaded. So I think it is okay to keep it as "indicator"?
| } | ||
|
|
||
| if (!this._container.parentNode) { | ||
| throw new Error('Unable to show the loading indicator because it is not attached to the DOM.'); |
There was a problem hiding this comment.
would this only happen if parentId is not provided?
There was a problem hiding this comment.
Good question. I might not even need this check. I'll double check.
| expect(() => indicator.show('invalid_container')).toThrow(); | ||
| }); | ||
|
|
||
| it('hide loading indicator', () => { |
What?
Why?
Testing / Proof
Unit
@bigcommerce/checkout @bigcommerce/payments