Simplify product-media-gallery snippet and consumers

[lhoffbeck]

PR Summary:

This PR is a fairly minor refactor to product images that simplifies the code and significantly improves liquid render performance with minimal impact to the existing UX.

Why are these changes introduced?

Iterating over product.variants can be bad for perf
The primary motivation for this change was to eliminate an iteration over product.variants in featured-products.liquid. While building Combined Listings, we found that this iteration becomes a perf bottleneck as the variant count increases, and we should eliminate as many instances of it as possible.

Media gallery was being rendered twice by main and featured product
Additionally, I found that when the merchant setting positioned the image gallery to the right, we were rendering the media gallery twice, in both main-product and featured-product. Since this snippet iterates over all of a product's media, this may cause poor theme perf if a product contains many images.

Main and Featured product rendered more images than it displayed
To facilitate swapping between highlighted images, main and featured product render thumbnails as hidden and then use JS to swap them into view if their associated variant is selected. With many images, this can cause theme perf issues and it's likely that many of the hidden images will never be displayed.

Featured product didn't use the product-media-gallery widget
Introduced in this PR, the featured-product section rendered the product media gallery in nearly the same way as main-product. This can increase drift and maintenance headaches.

What approach did you take?

(1) Updated the product-media-gallery widget to have a limit prop so that it supported the featured-product use case, which enabled eliminating iterating over product.variants.

(2) Removed the duplicate render of product-media-gallery and position with a CSS order. This also enabled me to simplify product-media-gallery because it contained logic to handle the case where it was rendered twice by a section (which doesn't happen anymore).

(3) Removed hidden thumbnail rendering for main and featured product, and instead update images using the response from the section rendering api. This is probably the most controversial part of the change since it takes sync behavior and makes it async. There is also some UX impact (see below).

Impact of these changes

tl;dr 15-65% improved load times for featured product alone, 10-69% improved load times when featured product and main product are rendered together.

The perf impact was pretty substantial. I set up 5 experiments with 20 overall test cases with an increasing number of variants and images. For each experiment, I ran a control (current dawn theme) vs a test (dawn theme with my changes), two test cases each--one for a page with just featured-product and one with featured-product+main-product together on a page. The merchant setting hide other variants' media after selecting a variant was checked. In storefront with caches disabled, I then measured render times using a Shopify-internal tracing tool once for each test case, by loading the storefront page with all caches disabled. Reach out to me via internal slack if you want more info on process. Results below:

⚠️ Note -- these numbers should be taken with a gigantic grain of salt and only be used as a rough indicator of performance since there is only 1 run per test case. Ideally I'd like to run a much higher number of tests and have something more statistically significant.

page with just featured product

# variant images	control - total time	test - total time	% reduction
3	145.88	122.97	-15.70
10	277.65	222.88	-19.73
50	404.63	263.35	-34.92
100	493.41	211.42	-57.15
250	1180	417.38	-64.63

page with main and featured product rendered together

# variant images	control - total time	test - total time	% reduction
3	150.25	134.01	-10.81
10	340	194.55	-42.78
50	532.99	266.61	-49.98
100	958.43	304.77	-68.20
250	2010	621.55	-69.08

Visual impact on existing themes

1. The image gallery is now rendered AFTER the section rendering api response, instead of being rendered hidden in the dom and revealed. This doesn't have a very noticeable impact with a low number of images (new code on left) but is a bit more noticeable if the product has a lot of images (new code on the left)
2. The theme used to always show the last selected variant featured media, even after switching to a different variant. You can see an example in this demo where the black variant has no featured image. After my changes, we only ever show the featured image associated with the current variant, meaning that you will never end up in a scenario where the white cat is shown when black cat is selected: demo. (tbh I see this as more of a bug fix than a regression)

Testing steps/scenarios

• Test PDP / pw: test
• Control PDP / pw: test

• Images are displayed the same for both main and featured product between test and control
• Image carousel shows the same images for both main and featured product between test and control
• Changing selected option values changes the featured image

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

[LucasLacerdaUX]

Just got pinged on this and realized I'm still part of the Themes FED group haha :P

I didn't review this, but just leaving some quick suggestions from what I remember about how we built the Product Page. Also, I highly recommend looking for more regressions when using 3D Models and Videos.

I don't have any of the recent context of what changes are being made right now, but let me know if I can help in any way.

[LucasLacerdaUX]

The reason we originally had two was because of tab order. In our accessibility review, we came to the conclusion that using CSS order could result in a misleading tab order considering there are multiple elements in the Product Media column.

[ludoboludo]

Yes! Accessibility wise we wouldn't want to do that 👍

[lhoffbeck]

Ahh great callout, I'll dig into this a bit more.

[lhoffbeck]

Okay! I caught up with @svinkle (our internal a11y legend) about this. He called out that while it definitely would be preferable to have the visual order match the tab order, this likely wouldn't be a high-SEV issue and that the perf benefit may be a worthwhile tradeoff. We narrowed down on two approaches for when the image gallery is positioned on the right:

• Skiplink: Continue using order to position and add a product info skiplink to the image gallery. Buyers landing on the page on desktop would tab first to the image gallery skiplink and (optionally) then to the product info, on mobile the behavior is unchanged from how it works today. Demo
• Reposition content with JS: This would be a lot easier with a reactive UI framework but it's doable. This approach works but is brittle because it relies on the structure of the DOM. Demo

I put together a test store for each approach and Scott floated them past the Head of Accessibility Innovation at Fable:

She agreed that it’s a tough spot. In one hand, tab order is not being followed, but given the context that the user is in (navigating content related to the PDP context) and the close proximity of the content, she agreed that the skip link approach would be a viable option. She was sympathetic to the dev experience and worried about the other option could result in brittle code that could break at some point in the future without notice.
Let’s move ahead with the skip link option which should make things easier for you, result in better code, a more performant user experience, and a pretty good accessibility experience.

[melissaperreault]

Hey! 👋 I made a request to have access to the test store so I can stress test other media gallery layout along with this Variant setting 🙏

[melissaperreault]

Hey @lhoffbeck ! Thanks for this!

Stress tested a few scenarios. Couldn't find regression evidence! 💪

[MiniLuke]

When I tophat, I can't seem to change the media shown just by clicking on the images like you can in the control store:

[tauthomas01]

Tophat works, tested on desktop and mobile. I see a very slight delay when switching media gallery, but for the sake of performance gain, this is totally acceptable.

Great refactor and thank you for sharing context with the videos you provided.

[lhoffbeck]

When I tophat, I can't seem to change the media shown just by clicking on the images like you can in the control store:

Hmmm that looks like it got updated since I initially wrote this, I'll take a look

[MiniLuke]

New change and tophat look good. Thanks for the work on this!