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

Fix stretched product modal images on Safari #131

Merged
merged 3 commits into from
Jul 7, 2021

Conversation

LucasLacerdaUX
Copy link
Contributor

@LucasLacerdaUX LucasLacerdaUX commented Jul 6, 2021

Why are these changes introduced?

When viewing large images in the product modal on Safari, the images would get vertically distorted.
Fixes #65.

This PR also removes leftover code for thumbnails component - this is no longer used in the context of the product page.

Before & After

Before
After

What approach did you take?
Unlike other modern browsers, Safari will stretch images to its natural height when they're contained in a flex wrapper. This StackOverflow thread provides more details on the issue. Unfortunately, the suggested fix in this thread will not completely solve our issues because we need to account for the multiple types of media inside the modal.

  • Removed display: flex and flex-direction from product-modal__content, responsible for causing this issue.
  • Added display:flex to the parent element and limited it's height to 100vh.

Demo links

Checklist

Testing instructions

  • Do images look stretched on Safari (desktop and mobile)?
  • Are 3D models and videos still center-aligned on mobile?

@LucasLacerdaUX LucasLacerdaUX changed the title Fix distorted product modal images on Safari Fix stretched product modal images on Safari Jul 6, 2021
@ludoboludo ludoboludo self-requested a review July 6, 2021 21:37
Copy link
Contributor

@ludoboludo ludoboludo left a comment

Choose a reason for hiding this comment

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

Looks good to me. I didn't run into any other issue. Tested on iPhone - safari, iPad - safari and Desktop - chrome.

One thing I noticed while testing, it's not pausing the video playing if you click on an image. Not related to this issue tho 😅 (video).

@tyleralsbury tyleralsbury self-requested a review July 7, 2021 14:53
Copy link
Contributor

@tyleralsbury tyleralsbury left a comment

Choose a reason for hiding this comment

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

Images look good. Love the bonus deletions! 🎩 and code ✅

@LucasLacerdaUX LucasLacerdaUX merged commit 749b643 into main Jul 7, 2021
@LucasLacerdaUX LucasLacerdaUX deleted the product-media-stretching branch July 7, 2021 18:42
phapsidesGT pushed a commit to Gravytrain-UK/gt-shopify-dawn-theme that referenced this pull request Sep 3, 2024
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.

Product Page: Media Modal: Images get "squished" on mobile iOS
4 participants