search

jlmakes / scrollreveal

Animate elements as they scroll into view.
https://scrollrevealjs.org/
22.39k stars 2.26k forks source link

Added Math.sign polyfill #391

Closed ryanturnerwebdev closed 7 years ago

ryanturnerwebdev commented 7 years ago

Saw this throwing an error on Safari 8 when doing some cross browser test and found out that compatibility for Math.sign isn't the best so have used the provided polyfill.

Could remove the comments if you wanted to keep the bundle as lean as possible? Else you could add it to the delegate function file if not being used anywhere else as it is essentially one line of code!

jlmakes commented 7 years ago

Oh wow, this is my mistake—I thought this was an ES5 feature. Thanks for catching this.

This is really helpful having another person with eyes on this!

jlmakes commented 7 years ago

You're 💯 with removing the comments; I ended up with:

export const polyfill = x => (x > 0) - (x < 0) || +x
export const mathSign = Math.sign || polyfill

I’ll patch to beta.21 shortly.

ryanturnerwebdev commented 7 years ago

Haha, yeah the comments were a bit excessive for a relatively simple function weren't they. Good to see the changes are making into beta.21 though; Cheers matey.