Conversation
|
Originally I had something like this which matches the card payment method, but then I realized it doesn't match non-card methods and we need something generic instead... Screen.Recording.2026-09-10.at.2.37.49.PM.mov |
|
cursor review |
|
cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 59c2975. Configure here.
richapatel1510
left a comment
richapatel1510
left a comment
There was a problem hiding this comment.
Couple of questions, rest LGTM! 👏
| }, [isEmbedded, isUsingMultiShipping, method, onUnhandledError]); | ||
|
|
||
| // Custom checklist items manage their own loading UI | ||
| return enhancedThemeV1 && !isCustomChecklistItem ? ( |
There was a problem hiding this comment.
Custom checklist items were previously covered by the list-wide overlay and now get nothing. The comment says they manage their own loading UI — is that true for all of them, or just the wallet buttons you checked? If any of them don't, they'll just sit there looking like a dead radio during init.
| padding: spacing("half") 0; | ||
|
|
||
| div { | ||
| animation: payment-method-skeleton-pulse 4000ms infinite cubic-bezier(0.13, 0.615, 0.315, 0.915); |
There was a problem hiding this comment.
💅
4000ms infinite cubic-bezier(0.13, 0.615, 0.315, 0.915) is character-for-character $loading-skeleton-grey-box-animation from settings/foundation/forms/_settings.scss. Worth pulling the timing from the variable so the two don't drift apart later — only the colour stops actually differ here.
| // Custom checklist items manage their own loading UI | ||
| return enhancedThemeV1 && !isCustomChecklistItem ? ( | ||
| <LoadingOverlay | ||
| hideContentWhenLoading |
There was a problem hiding this comment.
Heads up that hideContentWhenLoading sets display: none on the content rather than overlaying it, so the method initializes with no layout box.
No change for credit-card / hosted-payment / Adyen / Braintree / PayPal Commerce — they already self-wrap in this with the same flag. But the integrations that don't (Stripe, CheckoutCom, Mollie, Moneris, Worldpay) used to render visible under the overlay and now go fully hidden during init. Stripe UPE mounts Elements into #stripe-${method.id}-component-field right in that window.
Can you try to test Stripe manally? That one plus another hosted-field provider would be good to check.
2 participants
What/Why?
A small re-design of payment methods loaders only impacting enhancedThemeV1.
Right now we render a skeleton when loading payment methods and then show a spinner over all payment methods while they initialize.
With this PR we remove the spinner and instead keep the same skeleton (for consistency) and show it inside of payment methods. This makes more sense because we are actually waiting for a method to initialize so it makes sense to scope the loader to a payment method.
Rollout/Rollback
Revert the PR.
Testing
New CI tests + manual testing.
Before
Screen.Recording.2026-09-10.at.10.37.58.AM.mov
After
Screen.Recording.2026-09-10.at.3.16.53.PM.mov
Note
Low Risk
Checkout UX change gated to enhancedThemeV1 with legacy behavior preserved; limited to payment loading presentation and new tests.
Overview
For enhancedThemeV1, payment initialization no longer wraps the whole payment method list in a loading overlay. The checklist stays visible; only the selected method shows a payment-method skeleton while
isInitializingPaymentis true (custom checklist items are unchanged).Adds
PaymentMethodSkeletonin the UI package plus enhanced-theme styles so initializing radios look non-interactive (disabled cursor/opacity). PaymentForm tests cover no full-list overlay, blocked selection during init, and the per-method skeleton.Reviewed by Cursor Bugbot for commit 59c2975. Bugbot is set up for automated code reviews on this repo. Configure here.