Skip to content

W-2129587: removed wrapperStyle for LoadingSpinner to have full screen overlay #3696

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.

Sign up for GitHub

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

Open
nayanavishwa wants to merge 1 commit into
base: t/team404/sfp-on-pwa
Choose a base branch
from
Open

W-2129587: removed wrapperStyle for LoadingSpinner to have full screen overlay #3696

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/template-retail-react-app/app/pages/cart/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1493,7 +1493,7 @@ const Cart = () => {
)}

{/* Loading overlay during express payment confirmation */}
{confirmingBasket && <LoadingSpinner wrapperStyles={{height: '100vh'}} />}
{confirmingBasket && <LoadingSpinner />}
</Box>
)
}
Expand Down
4 changes: 2 additions & 2 deletions packages/template-retail-react-app/app/pages/checkout/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ const Checkout = () => {
)}

{/* Loading overlay during express payment confirmation */}
{confirmingBasket && <LoadingSpinner wrapperStyles={{height: '100vh'}} />}
{confirmingBasket && <LoadingSpinner />}
</Box>
)
}
Expand Down Expand Up @@ -340,7 +340,7 @@ const CheckoutContainer = () => {

return (
<CheckoutProvider>
{isDeletingUnavailableItem && <LoadingSpinner wrapperStyles={{height: '100vh'}} />}
{isDeletingUnavailableItem && <LoadingSpinner />}
Copy link
Collaborator

@jeffraab-sfdc jeffraab-sfdc Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure we should change this. I understand why you would identify the case though.

If this one existed before our branch, was it copied from yet another spinner case? I just wonder if there's a reason why it was done this way, which only fails for our payments case for some reason. After all 100vh is 100% of the viewport height.

Copy link
Collaborator Author

@nayanavishwa nayanavishwa Feb 27, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screen.Recording.2026-02-27.at.11.37.02.AM.mov

Please check the recording. The issue might be more at the LoadingSpinner component level (LoadingSpinner -> Box component)where the position has to be fixed. I understand that changing at the component level will require testing through out the application.

The other alternative fix is:
<LoadingSpinner wrapperStyles={{ position: 'fixed' }} />

Let me know if you agree with this change. I will go ahead and update the PR.

The only other deviation I found in our code was checkout/index.jsx has used LoadingSpinner twice. I changed it to use it only once at CheckoutContainer, but this change did not solve the overlay issue.

Copy link
Collaborator

@jeffraab-sfdc jeffraab-sfdc Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really know the answer. I suggest invite the PWA Kit team (or whoever owns the spinner common component) to weigh in on the best practice. If there's an issue with the spinner maybe they can take it on and we fix/close our own bug when they've fixed it. If it's really just the wrapperStyles they should be able to choose the right value for it, and then we can simply apply that to our owned uses.

<GoogleAPIProvider>
<Checkout />
</GoogleAPIProvider>
Expand Down
Loading