search

samccone / fidgetspin.xyz

https://fidgetspin.xyz
MIT License
299 stars 19 forks source link

Update container width to be at most 90% of viewport #59

Closed hihuz closed 7 years ago

hihuz commented 7 years ago

Since the size is a flat 400px, the spinner is overflowing on smaller viewports (360px below, but this gets worse when lower) : spinner_before

I made a few tweaks to the css file so that it takes at most 90% of the viewport : spinner_after

It is a bit of a hack so i'm not sure you would accept this :sweat_smile: but it is working consistently on all devices that I could try it on.

samccone commented 7 years ago

Tested on desktop + android need to test on IOS to confirm this works alright.

link to test: https://fidgetspin-xyz-pr59.now.sh/

hihuz commented 7 years ago

Sorry no actual iOS device handy :( I found that it rendered correctly on browserstack, but I couldn't test the behavior on a real device so far. I'll try to see if I can get someone to check this out.

samccone commented 7 years ago

@Haroenv do you have an iOS device to test this with?

Haroenv commented 7 years ago

Yes, I can test it tomorrow. I can already confirm that it's going slightly out of viewport right now

Haroenv commented 7 years ago

on IOS this PR makes the whole page scroll 10% out of view horizontally and the spinner doesn’t do anything...

samccone commented 7 years ago

confirmed on IOS that it does make the page scroll as @Haroenv says. The spinner does still work :) just need to make sure to closure-compile the code 💃

hihuz commented 7 years ago

Hmm that's not good. I find it odd that it would mess with the spinning but I totally get that it would introduce layout related issues.

Sorry for wasting your time on this, I should have tested more thoroughly before submitting.

I'll go ahead and close this for now, if I can find a more robust and fully tested way to do this i'll add some commits