Re-apply slider shadow padding

[kmeleta]

Why are these changes introduced?

Refs #1217
Fixes #1157

• Fixes slider shadow padding on non-product media sliders
• Adds some additional padding to the same logic for focus outlines

What approach did you take?

Reverted a change from a previous commit that limited the scope of the shadow padding styles.
Also noticed that the blogs slider may have been prevented from receiving the proper shadow offset values.

Other considerations

There are other improvements we can make to this logic, but this commit just aims to get it back to working for all sliders in the theme.

Testing notes:

• Collection list (tablet/mobile)
• Featured collection (tablet/mobile)
• Multicolumn (mobile)
• Featured blog (tablet/mobile)
• Product media gallery (mobile)

Demo links

• Editor

Checklist

• Followed theme code principles
• Linted with Theme Check
• Tested on mobile
• Tested on multiple browsers
• Tested for accessibility

[kmeleta]

We can update these to use shared classes in the future.

[ludoboludo]

Looks good 👌 Just wondering if we should tweak the calculation a little 🤔

[ludoboludo]

I think we might always need to have a bit of padding top and bottom whatever the vertical offset value is 🤔 maybe using a max(0.5rem, calc(var(--shadow-vertical-offset) * -1 + var(--shadow-blur-radius))) to account for focus styles:

[kmeleta]

I was going to include .5rem here actually but then we may not want an additional .5rem if there's already a shadow offset that will cover it. It also didn't fix it in one case (because the calculation currently relies on negative padding not being a problem), I believe with positive offset shadow applied?. tl;dr I just want sliders to work again and will handle more complicated stuff in follow ups.

[kmeleta]

Edit: Actually, using max() may straighten that up nicely. Good call. Will update if all looks well 👍

[kmeleta]

Donezo

[martinamarien]

Let me know if you think this should be in a different PR.

[martinamarien]

I know this isn't technically part of this PR, but this is something I've observed:

For cards that use the standard style, and particularly for the featured collection product cards and collection list collection cards, I think this adds a lot of unintended space below the cards on mobile and tablet. When the slider is enabled and when it's not. The list itself already adds margin-bottom: 1.5rem on tablet, so we probably need to make some exceptions for these types of cards.

I'm wondering if the padding should be applied below the image instead of the slider when the card type is Standard.

In this example, you can see that all padding has been removed from the section, but there's still a lot of space between the cards and the next section.

This makes sense if the card style is "Card" (see below), but it's causing some unintended spacing for the "Standard" style.

Places to look at:

• Featured collection product cards
• Collection list collection cards
• Blog post cards

[ludoboludo]

good point, so should be conditional to a class applied based on the card/standard layout then

[kmeleta]

That's definitely valid and I believe Meli mentioned it in the ticket too (I don't have this PR closing the issue for this reason). If the card discrepancy is also a sev 1 concern that we want done asap I can play with it before merging this. Otherwise, the current fix in this PR is definitely sev 1 and should be merged separately so it doesn't potentially get held up by reviews/discussions for the card aspect. cc @melissaperreault

[melissaperreault]

Aligned to tackle separately!

[ludoboludo]

It fixes this too: #1157 if we add the max(... for focus style.

[kmeleta]

It fixes this too: #1157 if we add the max(... for focus style.

Thanks Ludo I'll add it to the PR description