New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Array.from(elements).forEach #4

Merged
merged 2 commits into from May 29, 2017

Conversation

Projects
None yet
2 participants
@scmx
Contributor

scmx commented Apr 27, 2017

Because .forEach is not always available on elements in every browser. Can also be solved by doing [].forEach.call(elements, function () {})

TypeError elements.forEach is not a function Function.init
Mozilla/5.0 (Linux; Android 4.4.4; DL Build/KTU84Q) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/19.77.34.5 Safari/537.36

I'm getting this error on a TV with an old version of chrome :)

@scmx

This comment has been minimized.

Contributor

scmx commented Apr 27, 2017

Now I ran npm run release and updated commit to include priv/js/react_phoenix.js as well.

@geolessel

This comment has been minimized.

Owner

geolessel commented Apr 29, 2017

@scmx I see that Array.from is not supported in Internet Explorer. Will that cause issues? I don't have a version of IE that I can test with unfortunately.

@scmx

This comment has been minimized.

Contributor

scmx commented May 16, 2017

@geolessel Hello, I added a new commit that changes it to use Array.prototype.forEach.call(elements, e => {... instead. That should work in older browsers.

What I did before in my project was add a polyfill for Array.from etc. So we could potentially write in readme that Array.from might need a polyfill if we had wished to use that. I think in this case it was easy enough to just use Array.prototype.forEach.call.

I've not tested this in IE, but I'm pretty sure this part works in IE9+. It will not work in IE8 out of the box since forEach is missing. But I don't think this library is responsible for bringing in a polyfill for that. :)

@scmx

This comment has been minimized.

Contributor

scmx commented May 21, 2017

@geolessel What did you think of the change above? Do you want me to squash the commits and force push to remove the Array.from commit? You could also use squash during merge.

@scmx

This comment has been minimized.

Contributor

scmx commented May 29, 2017

@geolessel Hey, what do you think of getting this merged? It fixes #16

@geolessel

This comment has been minimized.

Owner

geolessel commented May 29, 2017

@scmx I will merge this thank you.

For some reason, I'm not getting github notifications in my email. I had no idea all these issues were open. :-/

@geolessel geolessel merged commit 7a1dbea into geolessel:master May 29, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@scmx scmx deleted the scmx:array-from-elements-foreach branch May 29, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment