[bug] Horizontal scrolling on mobile.

[eugenekasimov]

PR Summary:

Should be a quick fix for horizontal scrolling issue on mobile while the announcement bar slider has two or more slides.

To reproduce the issue you need to open our demo-store on a real mobile device, the announcement bar has to have at least two slides. Then just try to drag/pull any part of the site to the left or right.

The issue

RPReplay_Final1683580892.MP4

What approach did you take?

Change margin for slider's buttons for tablet and mobile to match page paddings and prevent exceeding width that caused horizontal scrolling effect.

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

[metamoni]

I tested this on my mobile and it looks like the issue has been fixed. I haven't noticed any unintentional side-effects either, so this looks good to me 👍

RPReplay_Final1683618155.MP4

[ludoboludo]

I wonder if this is the best way to go about it. I think there is a also another way for mobile to deal with it 🤔 Here we might create this fix when maybe we should revisit our spacing approach ?

We're using negative margin, -1.6rem, for the slider buttons that isn't matching the mobile padding 1.5rem.

I think this could be a more logical fix 🤔 If in fact it does also solve our issue.

[eugenekasimov]

Thanks Ludo, I think it's more logical and right solution from technical perspective, however it takes minor visual changes.

I also can assume that we have the same issue for tablet devices, unfortunately I don't have a real tablet to test it, but I would change margin for tablet from -3.8rem to -3.2rem.

All of these lead us to a small visual change but it's really minor.

@melissaperreault @YoannJailin Could you please confirm that we are okay to make those changes?

Before

After

[ludoboludo]

I think you can just change the 1.6 value to 1.5 and leave the rest untouched 🤔

We don't have the same issue for tablet devices 👍
On tablet sizes, over 750px and under 990px, the padding on the sides is 5rem while the negative margin applied to the buttons is -3.8rem so we should be safe here 🎉

[eugenekasimov]

Oh, you are right, I was actually looking at the header where paddings are 3.2rem.

But anyway, don't you think that, after we change 1.6 to 1.5, 3.2 rem looks a bit better when you try to see how the buttons and hamburger icons are aligned?

If don't change margin for tablet it looks like this 👇

And if we go for changing 3.8 to 3.2 for tablet it looks like this 👇

which is more closer to what it looks on mobile 👇

@ludoboludo @melissaperreault @YoannJailin

[ludoboludo]

Yeah I don't think it's much of an issue for me to keep the tablet look how it currently is. I think that alignment is ideally what we would have on mobile actually. But since the svg icon is a different size compared to the hamburger menu icon it makes it a bit trickier to align.

There is also the possibility to add some extra negative margin to the icon. So the 0.1rem we removed by going from 1.6 -> 1.5 can be added to .slider-button--prev .icon through margin-left: -0.1rem;

Though I don't really think we need to go to that level of nitpicking. Curious to see what designers think 👀

[melissaperreault]

I am okay with the alignment proposed! 👍

[melissaperreault]

The alignment on what is live currently on the PR demo link works for me. We could be nitpicking for a long time. Someone could argue that the visual impression of the alignment feels right even though it's not when you trace a line to the edge of the icon. The live version of this PR feels good for now, both for Tablet and Mobile. 👍

[melissaperreault]

LGTM!

[ludoboludo]

👍