Hello,

This is about version 8.x

From what I understand, Drupal.behaviors is to be used instead of $(document).ready, so the presence of $(document).ready inside Drupal.behaviors in script.js is useless. Actually, I tried putting some code inside $(document).ready located inside Drupal.behaviors and it executes twice. So not only this is not necessary, but it actually does not work properly.

I don't know about $(window).load, $(window).resize and $(window).scroll, but something tels me they should not be put inside Drupal.behaviors either.
The Zen theme is pretty well documented, yet it does not add anything like those inside Drupal.behaviors

What do you guys think?

CommentFileSizeAuthor
#6 basic_scripts-js-2778723.patch1.8 KBleahtard
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Nicolas Bouteille created an issue. See original summary.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented

Agreed and we shold probably change the documentation link to the D8 one
https://www.drupal.org/node/2269515

Those jQuery methods will still work, but DOM ready is a bit redundant.

It's called twice because there must be some ajax on the page I'm guessing, it should only call once unless there is a bug in core when there is no ajax calls.

Nicolas Bouteille’s picture

Nicolas Bouteille CreditAttribution: Nicolas Bouteille commented

Also, looks like the anonymous closure syntax is not correct though it still works...
It should be
(function($) {})(jQuery);
when it currently is
(function($) {}(jQuery));

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented

Yeah we can change that closure for both 7 and 8

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented

@Nicolas Bouteille would you mind making a patch for these changes?

leahtard’s picture

leahtard CreditAttribution: leahtard at The Jibe commented
Status: Active » Needs review
FileSize
basic_scripts-js-2778723.patch1.8 KB

Hey all,

Here is a patch.

Cheers, Leah

  • leahtard authored 4a8c0c1 on 8.x-1.x 
    Issue #2778723 by leahtard, joelpittet, Nicolas Bouteille: Remove $(...
leahtard’s picture

leahtard CreditAttribution: leahtard at The Jibe commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.