Skip to content

Conversation

@alpadev
Copy link
Contributor

@alpadev alpadev commented Jun 1, 2021

Closes #34103

Since #34114 is more focused around changing the way we register our components with jQuery, this PR is changing the onDOMContentLoaded utility function, to make use of an array to cache callbacks (while the document is loading) and execute them all at once. Therefore we register only one DOMContentLoaded event listener instead of one for every call to the function.

@alpadev alpadev requested a review from a team as a code owner June 1, 2021 10:42
@GeoSot
Copy link
Member

GeoSot commented Jun 1, 2021

Seems fine by me.

(You may could add a test)

@GeoSot GeoSot self-requested a review June 1, 2021 12:18
@alpadev alpadev force-pushed the alpa/refactor-ondomcontentloaded branch from 6760d0a to 7278a2b Compare June 2, 2021 07:06
@alpadev
Copy link
Contributor Author

alpadev commented Jun 2, 2021

@GeoSot Changed the implementation so it wouldn't add an event listener when the function isn't called during loading state. Updated the test to check that addEventListener isn't called more than once.

@GeoSot GeoSot requested a review from rohit2sharma95 June 2, 2021 09:18
@GeoSot
Copy link
Member

GeoSot commented Jun 2, 2021

@rohit2sharma95 give it a look, if you like, for a second opinion

Copy link
Contributor

@rohit2sharma95 rohit2sharma95 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work @alpadev 🥇

@alpadev alpadev force-pushed the alpa/refactor-ondomcontentloaded branch from 7278a2b to 9c9bb7c Compare June 2, 2021 09:51
@alpadev alpadev requested a review from XhmikosR June 3, 2021 22:17
alpadev added 2 commits June 4, 2021 01:03
…aded function

Instead of adding an event listener everytime the utility function is called, cache the callbacks and execute them all at once.
@alpadev alpadev force-pushed the alpa/refactor-ondomcontentloaded branch from d4dd813 to 99e448e Compare June 3, 2021 23:04
@alpadev
Copy link
Contributor Author

alpadev commented Jun 3, 2021

@XhmikosR removed the IIFE as requested

@XhmikosR XhmikosR changed the title Register only one DOMContentLoaded event listener in onDOMContentLoaded Register only one DOMContentLoaded event listener in onDOMContentLoaded Jun 22, 2021
@XhmikosR XhmikosR merged commit 4927388 into main Jun 22, 2021
@XhmikosR XhmikosR deleted the alpa/refactor-ondomcontentloaded branch June 22, 2021 17:19
marvin-hinkley-vortx pushed a commit to Vortx-Inc/bootstrap that referenced this pull request Aug 18, 2021
…oaded` (twbs#34158)

* refactor: reuse one DOMContentLoaded event listener in onDOMContentLoaded function

Instead of adding an event listener everytime the utility function is called, cache the callbacks and execute them all at once.

* refactor: drop iife for onDOMContentLoaded

Co-authored-by: XhmikosR <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove duplicate DOMContentLoaded events

6 participants