replace window.onload with window.addEventListener #2379
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR Summary:
window.onload
only allows a single callback to be attached.As soon as a second callback is added, the previous one is removed.
Some third party scripts contain
window.onload
handlers which breaks the address forms on customer account pages. The edit form can't be toggled for example.What approach did you take?
Replaces
window.onload
withwindow.addEventListener("load")
which supports multiple event listeners.Demo
onload
vsaddEventListener
: https://jsfiddle.net/0ydwoen7/2/onload 1
is never logged because it's been overridden by the second callback.addEventListener 2
is shown, however.addEventListener
isn't supported in IE<=8, but I think those are outside of the supported browser matrix and it's already been widely used throughout the code base https://github.com/search?q=repo%3AShopify%2Fdawn%20addEventListener&type=codeVisual impact on existing themes
none
Testing steps/scenarios
Demo links
Checklist