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

Update default value of constrain media setting #2266

Merged
merged 2 commits into from
Jan 30, 2023
Merged

Conversation

kmeleta
Copy link
Contributor

@kmeleta kmeleta commented Jan 30, 2023

PR Summary:

Changes the default value of the Constrain media to screen height setting.

Important: Could cause a visual change in existing themes for some users depending on product image sizes.

Why are these changes introduced?

Relates to #2052

The constrain setting was originally included with the default set to false. Which would prevent any visual changes. However, we intend for the setting to be true moving forward so this PR just changes that default value.

We originally considered a softer rollout of this change, where we update the setting default in a subsequent release, but adding the desired end state now as a breaking change for a major release comes with less logistical overhead.

What approach did you take?

Updated default setting in the main-product and featured-product section schemas and also added the missing default in the product.json template.

Visual impact on existing themes

This is a visually breaking change.

Depending on the selected media width setting, the aspect ratio of the product images, and the end user's viewport dimensions, it is possible some product images will be scaled down to fit within the viewport. This will be most common with taller portrait images on relatively shorter viewports. Otherwise the effect of this change might not be immediately noticeable.

Before:

After:

Testing steps/scenarios

  • Ensure constrain setting is checked and applied by default on the product page
  • Ensure constrain setting is checked and applied by default when adding new featured product sections

Demo links

Checklist

Copy link
Contributor

@melissaperreault melissaperreault left a comment

Choose a reason for hiding this comment

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

Thanks Ken! Looks good!
It's definitely an improvement for keeping tall images shorter on larger screens and for normalizing the height when all media have different ratio. 🚢 🚢 When they do manual update, merchants can always uncheck this to revert to what they had.

@andrewetchen andrewetchen self-requested a review January 30, 2023 18:47
Copy link
Contributor

@andrewetchen andrewetchen left a comment

Choose a reason for hiding this comment

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

  • Ensure constrain setting is checked and applied by default on the product page
  • Ensure constrain setting is checked and applied by default when adding new featured product sections

This looks good to me!

Thanks for adding the "Settings change" label and for adding the "constrain_to_viewport" setting to the product.json template. All other themes already have this so we'll just need to update the values etc. 🙌🏼

@kmeleta
Copy link
Contributor Author

kmeleta commented Jan 30, 2023

All other themes already have this so we'll just need to update the values etc. 🙌🏼

Oh.. yeah I forgot we did that already.. Oh well.

@kmeleta kmeleta merged commit 11df00a into main Jan 30, 2023
@kmeleta kmeleta deleted the constrain-setting-update branch January 30, 2023 19:29
phapsidesGT pushed a commit to Gravytrain-UK/gt-shopify-dawn-theme that referenced this pull request Sep 3, 2024
* Update constrain default for main and featured product

* Add missing default to product template
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.

4 participants