Closed ten1seven closed 8 years ago
It's not in bindEvents()
. Might be how you're reading the diff. Check out the full version.
I meant add document.removeEventListener('DOMContentLoaded', bindEvents);
to Line 173
I don't think so. The DOM needs to be ready before you can start binding to the body and you only need DOMContentLoaded
if it's not available when the script is first executed. So the order of operations would be:
Is document.body
available?
Yes? Bind all the events.
No? Wait for DOMContentLoaded
, then bind all the events.
yes sure, but once DOMContentLoaded is triggered, you can remove the listener. no use in it persisting ... so once bindEvents() is called, the listener is no longer needed. i don't mean to change anything in your coed - it's a good solution.
There's virtually no overhead in leaving the event bound and keeps the amount and complexity of the code that much lower. Sure, it's probably just a few extra lines but the payoff doesn't seem worth it at this point.
Yea, great - that's better - much cleaner and it prevents binding the listener if bindEvents() can not be called anyway (indexOf not present etc). You could maybe remove the DOMContentLoadedListener in bindEvents() - don't you think?