Skip to content
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

Resolve add to cart modal mobile isssue #1450

Merged

Conversation

mattcoy-arcticleaf
Copy link
Contributor

What?

This change brings the add to cart modal to the proper location on mobile.

On mobile screens, the add to cart modal appears off-screen. This issue stems from an edit made to the modal.scss file where the left, top, and transform CSS properties were removed.

To replicate, go to any product on the Cornerstone demo theme, for example, the Able Brewing System, resize your window to a width under 551px, and add the product to your cart.

The edit made to the modal.scss file appears to be a fix to center the small modal. This issue can be resolved by setting margin to 0, while keeping the left, top, and transform properties. I have changed the margin to 0 to prevent breaking the small modal with this edit.

Tickets / Documentation

N/A

Screenshots

Before:

add-to-cart-modal-before

After:

add-to-cart-modal-after

On mobile screens, the add to cart modal appears off screen. This change brings the modal to the proper location.
@bigbot
Copy link

bigbot commented Feb 20, 2019

Autotagging @bigcommerce/storefront-team @davidchin

@junedkazi
Copy link
Contributor

@mattcoy-arcticleaf the PR looks good. Can you please add a screenshot for desktop as well to confirm it is backward compatible.

@mattcoy-arcticleaf
Copy link
Contributor Author

@junedkazi here are the screenshots you requested:

Add to cart modal on desktop:

add-to-cart-modal-desktop

Small modal on desktop (modal--small class):

modal--small-desktop

I realize the alert modal is empty, but that was just copied from the alert-modal.html component, so it does not have content that would normally be injected when this file is called. The screenshot is just to show that the small modal is centered with the updated code.

@junedkazi junedkazi merged commit c8001cc into bigcommerce:master Feb 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants