Fix product media slider peek

[kmeleta]

PR Summary:

Fixes visual inconsistencies in product media mobile slider.

Why are these changes introduced?

This PR fixes an inaccurate slider count issue by addressing the broken "peek" on product media sliders.

This PR updated the slider logic (for all sliders) so slide counts were more accurate, but that logic doesn't account for the broken nature of the product slider "peek" and the slide count can still be incorrect when the global horizontal gap setting is larger than 30px/3rem.

Fixes #1668

Also fixes an issue mentioned in review where the counter shows for a single media item when hide_variants = true

What approach did you take?

This PR resolves the issue in Sense and any other theme utilizing larger horizontal gap values by ensuring the product media slider displays the proper grid peek as other sliders already do.

Current:

With this fix:

Other considerations

⚠️ It's worth noting that this fix will cause a slight visual change. Themes using smaller gap values currently will show some peek (like this). Sometimes this just looks awkward and this fix will definitely improve those cases. But themes using large horizontal spacing (like Sense) currently don't show any grid peek but will as a result of this change.

I think this is change is the right way to go as the peek was always intended, but perhaps its worth discussing the possibility of a slider peek setting to configure this.

Testing steps/scenarios

• Ensure product media slider counts are always accurate with various horizontal gap values
• No unintended regressions to product slider

Note: there is another bug where the first variant image (when hide variants setting is enabled) will have undesired right padding, which will inadvertently hide the peek anyway on the first slide. This is being handled separately.

Demo links

• Editor

Checklist

• Added PR summary for release notes
• Notified the help.shopify.com team about updates to theme settings (current contact: Kimli)
• Followed theme code principles
• Linted with Theme Check
• Tested on mobile
• Tested on multiple browsers
• Tested for accessibility

[LucasLacerdaUX]

Thanks for making this change, Ken.

Looks great to me and seems to fix the issue! :DD

I believe the "new" peek look is exactly what it's was designed to be like. Unless we changed it at some point on purpose since the product page was implemented, I've always assumed the "no peek for first media" was a bug. In earlier versions of the theme, we had the peek exactly like your fix reintroduces :D

[ludoboludo]

Looking good. I suggest another quick fix added to this as well in my comment.

As for the grid peek, I'd like to add that currently it's not representative of what it should look like when using the highest horizontal grid value. We currently have another issue and PR open where we will be removing the padding right applied on the first variant image. So for example if I go to the sandal product, I currently don't see the next image peeking while looking at the first image but I do for the subsequent ones.

Once we fix that padding applied, it will look like this consistently:

[ludoboludo]

I think there is an additional change we can add. On a single media product, we get the controls that show up even though there is only 1 image.

It happens because of the way we set the media_count value:

assign media_count = product.media.size
if section.settings.hide_variants
  assign media_count = media_count | minus: variant_images.size | plus: 1
endif

Which seem wrong. As it means that in our single image product case, we're doing: 1 - 0 + 1 = 2 when it should just be one.
The other situation could be that we have a product with a single variant and the media is attached to it. Then it's fine cause it ends up being 1 - 1 + 1.

So I think we could add this to the if statement: if section.settings.hide_variants and product.media.size > 1

here is a video of this ^

[kmeleta]

Good point. Updated.

[kmeleta]

And yes, the padding issue with the first variant is still an issue here. I added a note in the PR for reference.

[kmeleta]

@melissaperreault just want to make sure UX is aware of this change. Let me know if you have any concerns.

[melissaperreault]

The count works as expected, will follow up on another issue to discuss peek behaviour! Thanks! 🚢