Remove passive event "polyfill" and unused randomElement() utility
#1653
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ Deploy Preview for codingtrain ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
Hi @fturmel are there any pages I should load and test to confirm that these changes are working as intended! All looks great from my end but just want to double check before merging! |
|
You could look at the only feature connected to the passive event, which is persisting the scroll position of the content panel on the right side of the video component. See #82 and #46 for historical context. This track for example: https://deploy-preview-1653--codingtrain.netlify.app/tracks/data-and-apis-in-javascript CleanShot.2024-07-12.at.17.29.52.mp4 |
|
Great, thanks for this update and improvement!! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
We can remove this passive event listener polyfill thing - I don't think it ever was needed as it was introduced in 2022 (#82) but only exists because of old/dead Edge browser (pre Chromium I assume) from before 2017 and IE 11.
https://caniuse.com/passive-event-listener
I corrected the listeners usage too. The
captureoption isfalseby default, and thepassiveoption is not required when removing the listener (it's not even a valid option to begin with).https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener#passive
https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/removeEventListener#parameters
I also noticed the
randomElementutility was never used and can be safely removed from the codebase.