2
\$\begingroup\$

I'm not that comfortable with properly writing jQuery. On my website I've used a lot of jQuery plugins and basically just dumped all the different initiating scripts between two <script> tags before the closing body tag.

I'm getting a few bugs with the various plugins which I have a feeling is because the code is messy, but I'm not sure how to clean it up.

In my <script> tags I am calling slides.js, metafizzy isotope and masonry, instafeed, and initiating zurb foundation, there is also some custom jquery to add classes on scroll.

One thing I noticed is most of these call functions are supposed to be inside $(document).ready, but according to jQuery I should only have one $(document).ready to open and close. I'm not sure how to write the code properly without reusing the $(document).ready tags for each call function.

Furthermore I'm using conditional statements from the Statamic CMS in order to perform functions on specific pages, those are the tags wrapped in curly braces like this {{ example }}. I have to open and close the <script> tags after each conditional tag since they are not javascript, so I was also wondering whether I should start a new $(document).ready every time I open a new <script> tag.

{{ if url == "/" or url == "/about" or segment_1 == "collections" or url == "/shop" or url == "/news" }} 

    <script>
        $(window).scroll(function() {    
            var scroll = $(window).scrollTop();

            if (scroll >= 100) {
                  $(".small-logo").addClass("dark-fill");
                $("#nav-full-width").addClass("fixed-header");
                $("#main-logo").addClass("fixed-logo");
                $("#page-logo").addClass("fixed-logo");
                $(".fa-long-arrow-down").addClass("fixed-logo");
            } else {
                $(".small-logo").removeClass("dark-fill");
                $("#nav-full-width").removeClass("fixed-header");
                  $("#main-logo").removeClass("fixed-logo");
                  $("#page-logo").removeClass("fixed-logo");
                  $(".fa-long-arrow-down").removeClass("fixed-logo");
            }
        });

        $(window).scroll(function() {    
            var scroll = $(window).scrollTop();

            if (scroll >= 650) {
                  $("#splash-lookbook").addClass("splash-lookbook-rel");
                  $("#lookbook").addClass("lookbook-position");
                  $("#lookbook-blurb").addClass("hide-blurb");            

            } else {
                $("#splash-lookbook").removeClass("splash-lookbook-rel");
                  $("#lookbook").removeClass("lookbook-position");
                  $("#lookbook-blurb").removeClass("hide-blurb");             
            }
        });

        $(document).scroll(function () {
        var contact = $(this).scrollTop();
        if (contact > 300) {
            $('#contact-side').fadeIn(200);
        } else {
            $('#contact-side').fadeOut(200);
        }
    });
    </script>

    {{ else }}

      <script>

         $(".small-logo").addClass("dark-fill");
         $("#nav-full-width").addClass("fixed-header")

      </script>

    {{ endif }}

    {{ if text_color == "dark" }}

      <script>

         $("#nav-full-width a").addClass("dark");
         $(".small-logo").addClass("super-dark-fill");

      </script>

    {{ endif }}

    <script type="text/javascript">

    var userFeed = new Instafeed({
            get: 'user',
    /*         limit: "1", */
            resolution: 'standard_resolution',
            userId: 1273814685,
            accessToken: '1273814685.467ede5.2aed82d632884f0e8a41e1a2c09a0516',
            filter: function(image) {
                return image.tags.indexOf('captve') >= 0;
            }
        });
        userFeed.run();     

    $('.grid').masonry({
      // set itemSelector so .grid-sizer is not used in layout
      itemSelector: '.grid-item',
      columnWidth: '.grid-sizer',
      gutter: '.gutter-sizer',
      percentPosition: true
    })


    $(window).load(function(){

    var $grid = $('#shop-container').isotope({
      itemSelector: '.item',
      layoutMode: 'fitRows'
    });

    $grid.imagesLoaded().progress( function() {
      $grid.masonry('layout');
    });         

    // store filter for each group
    var filters = {};

    $('.filters').on( 'click', '.button', function() {
      var $this = $(this);
      // get group key
      var $buttonGroup = $this.parents('.button-group');
      var filterGroup = $buttonGroup.attr('data-filter-group');
      // set filter for group
      filters[ filterGroup ] = $this.attr('data-filter');
      // combine filters
      var filterValue = concatValues( filters );
      // set filter for Isotope
      $grid.isotope({ filter: filterValue });
    });

    // change is-checked class on buttons
    $('.button-group').each( function( i, buttonGroup ) {
      var $buttonGroup = $( buttonGroup );
      $buttonGroup.on( 'click', 'button', function() {
        $buttonGroup.find('.is-checked').removeClass('is-checked');
        $( this ).addClass('is-checked');
      });
    });

    // flatten object by concatting values
    function concatValues( obj ) {
      var value = '';
      for ( var prop in obj ) {
        value += obj[ prop ];
      }
      return value;
    }
    });    

    (function($) {
        var $window = $(window),
            $html = $('html');

        $window.resize(function resize() {
            if ($window.height() < 600) {
                return $("#product-detail").addClass('product-detail-adjust');
            }

            $("#product-detail").removeClass('product-detail-adjust');
        }).trigger('resize');
    })(jQuery);

    $(document).foundation();

</script>
Improve this question
\$\endgroup\$
4
  • 5
    \$\begingroup\$ Welcome to Code Review. As we all want to improve our code on this site, please write a title that describes what your code does in a few words. Also, you said there are a few bugs due to plug-ins and such, is the code you posted working as intended, as far as you know? \$\endgroup\$
    – Phrancis
    Mar 5, 2016 at 20:09
  • \$\begingroup\$ the code is working as intended, except as I said a few small bugs, but at the moment I'm more concerned with getting all these call functions cleaned up, if the bugs persist I will look into specifics. Thanks \$\endgroup\$
    – fred
    Mar 6, 2016 at 0:02
  • 1
    \$\begingroup\$ Where have you read that you should have only one $(document).ready? If I know what you have read, maybe I can help debunk it. \$\endgroup\$
    – Roamer-1888
    Mar 7, 2016 at 14:23
  • \$\begingroup\$ I've re-read some of the StackOverflow threads and I guess I had misunderstood the issue. Some said multiple $(document).ready was messy, but I now understand that it is perfectly legal. \$\endgroup\$
    – fred
    Mar 9, 2016 at 1:41

2 Answers 2

Reset to default
1
\$\begingroup\$

Presumably statamic {{ if ... }} statements allow you to include/exclude statements within <script>...</script> tags.

If so, then you can write something like this in the document's <head>...</head> :

<script>
jQuery(function($) {
    var $window = $(window);
{{ if url == "/" or url == "/about" or segment_1 == "collections" or url == "/shop" or url == "/news" }}
    $window.scroll(function() {
        var gt100 = $window.scrollTop() >= 100;
        $(".small-logo").toggleClass("dark-fill", gt100);
        $("#nav-full-width").toggleClass("fixed-header", gt100);
        $("#main-logo").toggleClass("fixed-logo", gt100);
        $("#page-logo").toggleClass("fixed-logo", gt100);
        $(".fa-long-arrow-down").toggleClass("fixed-logo", gt100);
        var gt650 = $window.scrollTop() >= 650;
        $("#splash-lookbook").toggleClass("splash-lookbook-rel", gt650);
        $("#lookbook").toggleClass("lookbook-position", gt650);
        $("#lookbook-blurb").toggleClass("hide-blurb", gt650);
    });
    $(document).scroll(function () {
        $('#contact-side')[($(this).scrollTop() > 300) ? 'fadeIn' : 'fadeOut'](200);
    });
{{ else }}
    $(".small-logo").addClass("dark-fill");
    $("#nav-full-width").addClass("fixed-header")
{{ endif }}
{{ if text_color == "dark" }}
    $(".small-logo").addClass("super-dark-fill");
    $("#nav-full-width a").addClass("dark");
{{ endif }}
    var userFeed = new Instafeed({
        get: 'user',
        /* limit: "1", */
        resolution: 'standard_resolution',
        userId: 1273814685,
        accessToken: '1273814685.467ede5.2aed82d632884f0e8a41e1a2c09a0516',
        filter: function(image) {
            return image.tags.indexOf('captve') >= 0;
        }
    });
    userFeed.run();

    $('.grid').masonry({
        // set itemSelector so .grid-sizer is not used in layout
        itemSelector: '.grid-item',
        columnWidth: '.grid-sizer',
        gutter: '.gutter-sizer',
        percentPosition: true
    });

    var $grid = $('#shop-container').isotope({
        itemSelector: '.item',
        layoutMode: 'fitRows'
    });

    $grid.imagesLoaded().progress(function() {
        $grid.masonry('layout');
    });         

    $('.filters').on('click', '.button', function() {
        $(this).parents('.button-group').toggleClass('active');
        var filters = $('.button-group.active').map(function(btnGrp) {
            return $(btnGrp).attr('data-filter');
        }).join('');
        $grid.isotope({ filter: filters });
    });

    $('.button-group').on('click', 'button', function() {
        $(this).addClass('is-checked').closest('.button-group').find('button').not(this).removeClass('is-checked');
    });

    $window.resize(function resize() {
        $("#product-detail")[($window.height() < 600) ? 'addClass' : 'removeClass']('product-detail-adjust');
    }).trigger('resize');

    $(document).foundation();
});
</script>

This will give very conventional looking javascript/jQuery in its served form.

Improve this answer
\$\endgroup\$
2
  • \$\begingroup\$ This looks more like the type of thing I'm looking for. The problem is now with this revised code nothing is working anymore, the toggle classes aren't working, niether is isotope, masonry or instafeed. The good news though is that Statamic tags do work in the <script> tags, thanks. \$\endgroup\$
    – fred
    Mar 10, 2016 at 21:27
  • \$\begingroup\$ Whoops, syntax error. I just made an edit. \$\endgroup\$
    – Roamer-1888
    Mar 10, 2016 at 22:08
0
\$\begingroup\$

You can move you javascript code into 4 files. This will make your code more readable and is easier to maintain. Then you can put the necessary code into

jQuery(function(){
   //dom ready codes
});

or

$(document).ready(function(){});

The files will look something like this:

{{ if url == "/" or url == "/about" or segment_1 == "collections" or url == "/shop" or url == "/news" }} 
  <script src="file1.js"></script>  
{{ else }}
  <script src="file2.js"></script>
{{ endif }}

{{ if text_color == "dark" }}
  <script src="file3.js"></script>
{{ endif }}
<script src="file4.js"></script>

file1.js

$(window).scroll(function() {    
    var scroll = $(window).scrollTop();

    if (scroll >= 100) {
          $(".small-logo").addClass("dark-fill");
        $("#nav-full-width").addClass("fixed-header");
        $("#main-logo").addClass("fixed-logo");
        $("#page-logo").addClass("fixed-logo");
        $(".fa-long-arrow-down").addClass("fixed-logo");
    } else {
        $(".small-logo").removeClass("dark-fill");
        $("#nav-full-width").removeClass("fixed-header");
          $("#main-logo").removeClass("fixed-logo");
          $("#page-logo").removeClass("fixed-logo");
          $(".fa-long-arrow-down").removeClass("fixed-logo");
    }
});

$(window).scroll(function() {    
    var scroll = $(window).scrollTop();

    if (scroll >= 650) {
          $("#splash-lookbook").addClass("splash-lookbook-rel");
          $("#lookbook").addClass("lookbook-position");
          $("#lookbook-blurb").addClass("hide-blurb");            

    } else {
        $("#splash-lookbook").removeClass("splash-lookbook-rel");
          $("#lookbook").removeClass("lookbook-position");
          $("#lookbook-blurb").removeClass("hide-blurb");             
    }
});

$(document).scroll(function () {
  var contact = $(this).scrollTop();
  if (contact > 300) {
      $('#contact-side').fadeIn(200);
  } else {
      $('#contact-side').fadeOut(200);
  }
});

file2.js

$(".small-logo").addClass("dark-fill");
$("#nav-full-width").addClass("fixed-header");

file3.js

$("#nav-full-width a").addClass("dark");
$(".small-logo").addClass("super-dark-fill");

file4.js

var userFeed = new Instafeed({
        get: 'user',
/*         limit: "1", */
        resolution: 'standard_resolution',
        userId: 1273814685,
        accessToken: '1273814685.467ede5.2aed82d632884f0e8a41e1a2c09a0516',
        filter: function(image) {
            return image.tags.indexOf('captve') >= 0;
        }
    });
    userFeed.run();     

$('.grid').masonry({
  // set itemSelector so .grid-sizer is not used in layout
  itemSelector: '.grid-item',
  columnWidth: '.grid-sizer',
  gutter: '.gutter-sizer',
  percentPosition: true
})

$(window).load(function(){

var $grid = $('#shop-container').isotope({
  itemSelector: '.item',
  layoutMode: 'fitRows'
});

$grid.imagesLoaded().progress( function() {
  $grid.masonry('layout');
});         

// store filter for each group
var filters = {};

$('.filters').on( 'click', '.button', function() {
  var $this = $(this);
  // get group key
  var $buttonGroup = $this.parents('.button-group');
  var filterGroup = $buttonGroup.attr('data-filter-group');
  // set filter for group
  filters[ filterGroup ] = $this.attr('data-filter');
  // combine filters
  var filterValue = concatValues( filters );
  // set filter for Isotope
  $grid.isotope({ filter: filterValue });
});

// change is-checked class on buttons
$('.button-group').each( function( i, buttonGroup ) {
  var $buttonGroup = $( buttonGroup );
  $buttonGroup.on( 'click', 'button', function() {
    $buttonGroup.find('.is-checked').removeClass('is-checked');
    $( this ).addClass('is-checked');
  });
});

// flatten object by concatting values
function concatValues( obj ) {
  var value = '';
  for ( var prop in obj ) {
    value += obj[ prop ];
  }
  return value;
}
});    

(function($) {
    var $window = $(window),
        $html = $('html');

    $window.resize(function resize() {
        if ($window.height() < 600) {
            return $("#product-detail").addClass('product-detail-adjust');
        }

        $("#product-detail").removeClass('product-detail-adjust');
    }).trigger('resize');
})(jQuery);

$(document).foundation();

Hope this will help you.

Edit:

You can have multiple document ready functions on your page:

Improve this answer
\$\endgroup\$
3
  • \$\begingroup\$ There is no need to use more files. More files means more Internet calls, slower loading times, and more consumed bandwidth \$\endgroup\$
    – Keith M
    Mar 8, 2016 at 7:10
  • \$\begingroup\$ It is easier to read and if your webserver uses HTTP/2 it doesn't matter. In this case you have max 3 files which need to load. \$\endgroup\$
    – Marvin Caspar
    Mar 8, 2016 at 7:36
  • \$\begingroup\$ this is a nice solution to get the page cleaned up a bit, although I was expecting there to be some syntax errors or just some things not properly formatted in the jQuery itself, my knowledge of jQuery is limited so honestly maybe my concern doesn't even make sense, but I guess my question is overall does the code seem ok? \$\endgroup\$
    – fred
    Mar 9, 2016 at 1:36

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.