Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

remove $(document).ready() #8017

Closed
wants to merge 1 commit into from
Closed

remove $(document).ready() #8017

wants to merge 1 commit into from

Conversation

marcobiedermann
Copy link
Contributor

All functions are called at the bottom of the page, which means the document is already ready.

@leofeyer
Copy link
Member

This may be right, but since we don't know, why not leave the function call there? Does it noticeably affect the performance?

@marcobiedermann
Copy link
Contributor Author

It does not affect the performance that much, just a bit.
I think this is irrelevant and so less code is cleaner code. It will work perfectly fine without the extra function call.

@marcobiedermann
Copy link
Contributor Author

So to get the best perceived load time, put your script at the bottom of the page. […] And if you're going to do that, then there's no need to use ready, though of course you could if you liked.

http://stackoverflow.com/questions/6026645/document-readyfunction-vs-script-at-the-bottom-of-page

@leofeyer
Copy link
Member

I know. But can we be sure that the scripts are always at the bottom of the page? What if someone adds a module via insert tag?

@Aybee
Copy link
Contributor

Aybee commented Sep 16, 2015

What if something is included with $GLOBALS['TL_BODY'][]?
I like this domready wrapper as it also is a reminding marker for that the script needs the DOM to be loaded first.

@leofeyer
Copy link
Member

@contao/developers /cc

@Toflar
Copy link
Member

Toflar commented Apr 15, 2016

We can safely remove them. That's my opinion :)

@aschempp
Copy link
Member

I would not remove them. Also because they're a safe wrapper for variables and jquery (like so:)

jQuery(document).ready(function($) {
   // all variables wrapped in function and $ is jQuery
});

@aschempp
Copy link
Member

Which means I would remove the outer function ;-)

@leofeyer
Copy link
Member

But the outer function is shorter, so in terms of page size optimization, it would make more sense to remove the inner one :)

@ausi
Copy link
Member

ausi commented Apr 15, 2016

IMO we should still listen to domready, because someone could rely on the current behaviour. But there is a shorter version:

Before:

(function($) {
    $(document).ready(function() {
        // code…
    });
})(jQuery);

After:

jQuery(function($) {
    // code…
});

@backbone87
Copy link
Contributor

i think ausi's variant is the way to go. keeps the global clean and you get a save jQuery reference and are safe, if your script isnt at the end of the body.

@leofeyer
Copy link
Member

Is there a shorter version for MooTools as well?

// Current wrapper
(function($) {
    window.addEvent('domready', function() {
        // …
    });
})(document.id);

@aschempp
Copy link
Member

aschempp commented Apr 16, 2016 via email

@leofeyer
Copy link
Member

I know. But that is actually not the answer to the question :)

@backbone87
Copy link
Contributor

window.addEvent("domready", function() {
  var $ = document.id;
});

leofeyer added a commit to contao/core-bundle that referenced this pull request Apr 18, 2016
@leofeyer leofeyer added this to the 4.2.0 milestone Apr 18, 2016
@leofeyer leofeyer self-assigned this Apr 18, 2016
@leofeyer
Copy link
Member

Changed in contao/core-bundle@ef20adf.

@leofeyer leofeyer closed this Apr 18, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants