Adjust constrain media values for quick add modal #2113

kmeleta · 2022-11-09T22:16:37Z

PR Summary:

Allows the quick add modal to inherit the new Constrain media to viewport height and Media fit settings from the product page.

Why are these changes introduced?

Fixes #2017

This is a follow up to #2052 to tweak constrain media behavior specifically for the quick add modal. These changes are based off the code added in that original PR.

What approach did you take?

Modified the --constrained-min-height and --viewport-offset CSS variables to account for the specific sizing consideration of product media within the quick add modal UI. In the original PR, I added a style that effectively disabled the constrain effect and thus any chance of media-fit having an effect in the modal. This has been removed.

I did capture some hardcoded values in the quick add stylesheet used for padding/position in variables so they can be used to reliably calculate a more accurate --viewport-offset value. This added complexity is to help protect against the case that the quick add padding/positioning is updated in the future and the new values don't get accounted for in --viewport-offset. I'm on the fence if it's worth it or not, so let me know if a hardcoded --viewport-offset (and maybe a comment) is more desirable.

Other considerations

The settings on main-product are automatically reflected in the quick add modal. The collection rendering the quick add option is not responsible for configuring display options within the modal content. This is the case with existing product settings as well.

Product media in the modal will now be able to "constrain" large images on smaller viewports
https://screenshot.click/09-57-ec7xe-pz6bv.mp4

When constrained, the product media in the modal will also respect the media fit "fill" option. There are currently no other configurations that could cause the media fit fill effect without constraining.
https://screenshot.click/09-58-402of-9fv3h.mp4

Global popup styles will impact the quickadd modal, particularly the addition of a border can impact the amount of content visible in the modal and the constrain styles attempt to account for this if applicable.
https://screenshot.click/09-26-8pinr-3j4y4.mp4

Testing steps/scenarios

Configure Constrain media to viewport height and/or Media fit on the main product section
Enable the quick add setting from the collections page (you may need to save/reload the editor)
Ensure the settings each have the intended effect on the quick add modal
Ensure the minimum height configured for constrained product media is appropriate for the quick modal
Test with global media shadow and border styles (particularly large shadow offsets)
Test with global popup border styles (particularly large border widths)
Repeat for mobile, tablet, and desktop sized viewports

Note: It may be most beneficial to test mobile values on actual devices

Demo links

Editor

Checklist

Added PR summary for release notes
Requested review from UX (Only for changes that are affecting the experience or perceivable visual details)
Created a ticket for the help.shopify.com documentation team about updates to theme settings. (Internal-only task)
Followed theme code principles
Linted with Theme Check
Tested on mobile
Tested on multiple browsers
Tested for accessibility

ludoboludo

This seem to be specifically useful for tablets/small sizes devices that aren't phone right ? 🤔

ludoboludo · 2022-11-11T20:10:32Z

assets/quick-add.css

-  padding-top: var(--ratio-percent);
+quick-add-modal .product-media-container.constrain-height {
+  --viewport-offset: calc(var(--modal-height-offset) + calc(var(--modal-padding) * 2) + calc(var(--popup-border-width) * 2));
+  --constrained-min-height: 300px;


I don't think you need to re enter this for mobile as it's the same value used for the product page. Only for desktop it needs to be overwritten.

screenshot

This seem to be specifically useful for tablets/small sizes devices that aren't phone right ?

Mostly I guess yeah. It most cases it's probably unlikely that an image will need to constrain on a typical mobile phone given the additional spacing involved in modals.

ludoboludo · 2022-11-11T20:10:57Z

assets/quick-add.css


 @media screen and (min-width: 750px) {
  .quick-add-modal__content {
+    --modal-height-offset: 20rem;


Kinda seem odd to me that we're reusing the same variable in two different context.

--modal-height-offset is used for the margin-top and bottom value on mobile but not on desktop, where it's tied to the max-height instead 🤔

Hmm is it different contexts in that way though? In either case it represents the offset distance between a top/bottom edge of the modal and the edge of the viewport. It's used in the max-height calculation for desktop to effectively set a bottom offset. It doesn't use bottom or margin-bottom for this on desktop I believe because of the specific way we wanted the modal to scale/overflow.

I was however using it to represent the top or bottom as well as the combined offset of top/bottom, which is indeed odd. So now modal-height-offset will only represent the offset from either, and should be multiplied by 2 when the total space isn needed. I think this makes more sense. lmk if I'm looking at this wrong though.

assets/quick-add.css

ludoboludo

Just a small nitpick otherwise looks good 👍

assets/quick-add.css

andrewetchen

I'll approve this as it looks good to me!

I was wondering if you could provide some clarity around the different media types when viewing the quick add modal (using your testing steps/scenarios for desktop, tablet, and mobile), for products that have an image, 3D, or video as its first media -- and whether this is the expected behaviour.

Constrain media to screen height: enabled / Media fit: Fill (Screenshot)

The 3D and Video products don't "constrain" the same way as the Image product

Quick-add-constrain-and-fill-3d-and-videos.mp4

Constrain media to screen height: enabled / Media fit: Original (Screenshot)

The 3D and Video products don't "constrain" the same way as the Image product

Quick-add-constrain-and-original-3d-and-videos.mp4

Notes:

The above behaviour occurs on Chrome, Firefox, and Safari
The viewport I was testing at was 1200px, going wider (1400px +) seems to make images, 3D, and videos behave similarly

kmeleta · 2022-11-22T19:06:44Z

assets/quick-add.css

+
+@media screen and (min-width: 750px) {
+  quick-add-modal .product-media-container.constrain-height {
+    --constrained-min-height: 400px;


The 3D and Video products don't "constrain" the same way as the Image product

This may just be harder to see because those model are square. A square image should behave similarly in that in many cases especially smaller page widths (~1200px) square images aren't tall enough to exceed the minimum constrain height that prevents media from scaling too small.

The viewport I was testing at was 1200px, going wider (1400px +) seems to make images, 3D, and videos behave similarly

This is exactly correct. The more horizontal space a product media has to expand, the taller they will render as well, so you are more likely to see media qualifying for constraining. The desktop media width setting on the product page also effects this in the same way. If using a media width small or medium, it's becomes even less likely that media will need to constrain even at wider page widths.

@andrewetchen

Ah perfect, thanks for explaining all of that 😄

commit dc81806 Author: Ken Meleta <[email protected]> Date: Tue Nov 22 13:32:46 2022 -0700 Add media sizing settings to featured product (Shopify#2114) commit 9875aea Author: Ken Meleta <[email protected]> Date: Tue Nov 22 13:17:55 2022 -0700 Adjust constrain media values for quick add modal (Shopify#2113) * Adjust constrain media values for quickadd * Adjust offset logic * Minor formatting fix commit 2eb8bf6 Author: Ludo <[email protected]> Date: Tue Nov 22 11:50:50 2022 -0500 Add fixed sizes for some images to prevent errors (Shopify#2087) * remove the mention of sizes where unnecessary * remove another instance * add fixed size to the images in structured data * adjust size to 1920 * edit logo size in structured data * remove conditional info * change value to match other logo size values commit 2f9b9f3 Author: Ludo <[email protected]> Date: Mon Nov 21 15:44:24 2022 -0500 Remove alt attribute in image_tag filter where unnecessary (Shopify#2117) * Add fallback to alt attribute where image_tag filter is used * remove unnecessary use of alt attribute in image_tag filter commit 251e5d9 Author: Ludo <[email protected]> Date: Mon Nov 21 12:18:44 2022 -0500 Move share button into a snippet (Shopify#2123) * Move share button into a snippet * change the property name to context and add details in snippet * address review comments * edit props names and description * Address reviewers comment. Remove unused prop and edit existing ID * move script at the top commit 539af27 Author: Eugene Kasimov <[email protected]> Date: Mon Nov 21 09:01:58 2022 -0800 Prettier liquid files snippets. (Shopify#2115) commit 503fdf8 Author: Eugene Kasimov <[email protected]> Date: Thu Nov 17 08:51:21 2022 -0800 Remove noscript css import for product recommendations (Shopify#2101) commit b2b01fd Author: Eugene Kasimov <[email protected]> Date: Thu Nov 17 08:50:33 2022 -0800 Change variables names (Shopify#2055) commit cf05d0d Author: Francisca Calabria Rubio <[email protected]> Date: Thu Nov 17 16:50:55 2022 +0100 Select text input content on trapFocus (Shopify#2106) * Select text input content on trapFocus * PR fix commit b86d1f3 Author: Ken Meleta <[email protected]> Date: Thu Nov 17 08:50:43 2022 -0700 Change thumbnail media fit to "fill" by default (Shopify#2044) * Change thumbnail media fit to cover by default * Simplify and cleanup thumbnail fit approach commit 8ab8e61 Author: Ludo <[email protected]> Date: Thu Nov 17 10:50:28 2022 -0500 Tweak the image_url size to be the max value where necessary (Shopify#2110) * Tweak the image_url size to be the max value where necessary * fix sizing in the header. commit c337301 Author: translation-platform[bot] <34770790+translation-platform[bot]@users.noreply.github.com> Date: Thu Nov 17 10:49:52 2022 -0500 Update 1 translation file (Shopify#2130) Co-authored-by: translation-platform[bot] <34770790+translation-platform[bot]@users.noreply.github.com> commit bc8f433 Author: Ken Meleta <[email protected]> Date: Thu Nov 17 08:49:10 2022 -0700 Add constrain media and media fit settings to product page (Shopify#2052) * Add constrain height setting * Add constrain scaling functionality * Add media fit setting and functionality * Quick add override and typo fixes * Update setting language and code cosmetics * Update 8 translation files * Update 5 translation files * Update 3 translation files * Update 3 translation files * Update 1 translation file * Add comment Co-authored-by: translation-platform[bot] <34770790+translation-platform[bot]@users.noreply.github.com>

* Adjust constrain media values for quickadd * Adjust offset logic * Minor formatting fix

kmeleta force-pushed the constrain-quickadd-media branch from 8429275 to 2f460a8 Compare November 9, 2022 22:22

kmeleta requested a review from duygukalaycioglu November 9, 2022 22:29

ludoboludo reviewed Nov 11, 2022

View reviewed changes

duygukalaycioglu previously approved these changes Nov 15, 2022

View reviewed changes

Base automatically changed from constrain-product-media to main November 17, 2022 15:49

Adjust constrain media values for quickadd

0db3fa4

kmeleta dismissed duygukalaycioglu’s stale review via 0db3fa4 November 21, 2022 23:40

kmeleta force-pushed the constrain-quickadd-media branch from 2f460a8 to 0db3fa4 Compare November 21, 2022 23:40

Adjust offset logic

50c29bc

kmeleta force-pushed the constrain-quickadd-media branch from 4fc0be8 to 50c29bc Compare November 22, 2022 00:28

kmeleta requested a review from ludoboludo November 22, 2022 00:30

andrewetchen self-requested a review November 22, 2022 17:06

ludoboludo reviewed Nov 22, 2022

View reviewed changes

assets/quick-add.css Outdated Show resolved Hide resolved

Minor formatting fix

2353c7d

ludoboludo approved these changes Nov 22, 2022

View reviewed changes

andrewetchen approved these changes Nov 22, 2022

View reviewed changes

kmeleta commented Nov 22, 2022

View reviewed changes

kmeleta merged commit 9875aea into main Nov 22, 2022

kmeleta deleted the constrain-quickadd-media branch November 22, 2022 20:17

phapsidesGT pushed a commit to Gravytrain-UK/gt-shopify-dawn-theme that referenced this pull request Sep 3, 2024

Adjust constrain media values for quick add modal (Shopify#2113)

5038bcd

* Adjust constrain media values for quickadd * Adjust offset logic * Minor formatting fix

lost-end-found pushed a commit to lostendfound/librairiesanstitre-shopify that referenced this pull request Oct 8, 2025

Adjust constrain media values for quick add modal (Shopify#2113)

19e6311

* Adjust constrain media values for quickadd * Adjust offset logic * Minor formatting fix

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Adjust constrain media values for quick add modal #2113

Adjust constrain media values for quick add modal #2113

Uh oh!

kmeleta commented Nov 9, 2022 •

edited

Loading

Uh oh!

ludoboludo left a comment

Uh oh!

ludoboludo Nov 11, 2022

Uh oh!

kmeleta Nov 22, 2022 •

edited

Loading

Uh oh!

ludoboludo Nov 11, 2022

Uh oh!

kmeleta Nov 22, 2022 •

edited

Loading

Uh oh!

Uh oh!

ludoboludo left a comment

Uh oh!

Uh oh!

andrewetchen left a comment

Uh oh!

kmeleta Nov 22, 2022 •

edited

Loading

Uh oh!

kmeleta Nov 22, 2022

Uh oh!

andrewetchen Nov 22, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Adjust constrain media values for quick add modal #2113

Adjust constrain media values for quick add modal #2113

Uh oh!

Conversation

kmeleta commented Nov 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary:

Why are these changes introduced?

What approach did you take?

Other considerations

Testing steps/scenarios

Demo links

Checklist

Uh oh!

ludoboludo left a comment

Choose a reason for hiding this comment

Uh oh!

ludoboludo Nov 11, 2022

Choose a reason for hiding this comment

Uh oh!

kmeleta Nov 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ludoboludo Nov 11, 2022

Choose a reason for hiding this comment

Uh oh!

kmeleta Nov 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ludoboludo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

andrewetchen left a comment

Choose a reason for hiding this comment

Uh oh!

kmeleta Nov 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kmeleta Nov 22, 2022

Choose a reason for hiding this comment

Uh oh!

andrewetchen Nov 22, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kmeleta commented Nov 9, 2022 •

edited

Loading

kmeleta Nov 22, 2022 •

edited

Loading

kmeleta Nov 22, 2022 •

edited

Loading

kmeleta Nov 22, 2022 •

edited

Loading