Create URL using data obtained from filters

Unfortunately there is no URL where you can see the code working. I hope you all can figure out what the code does simply by looking at it.

GitHub

var FilteredNav = {
      wrapper: $(".dept-filters")
      , init: function() {
        FilteredNav.changeUrl();
        FilteredNav.createUrl();
      }
      , createUrl: function() {
        var url = null
          , urlItems = null;

        $.each(FilteredNav.wrapper, function() {
          // clean it all up before creating the url
          urlItems = [];

          var self = $(this)
            , checkedInput = self.find('input[type="checkbox"]:checked')
            , submitButton = self.find('.refinar-btn');

          // select only the relevant items
          $.each(checkedInput, function(i, item) {
            urlItems.push($(item).attr("rel"));
          });

          if (checkedInput.length) {
            // create the url
            url = document.location.protocol
            + "//"
            + document.location.host
            + "/"
            + self.attr("data-dept")
            + "/?"
            + urlItems.join("&");

            // add new url as href of 'refine' button
            submitButton.removeClass("hide").attr("href", url);
          } else {
            submitButton.addClass("hide");
          }
        });
      }
      , changeUrl: function() {
        FilteredNav.wrapper.find('input[type="checkbox"]').bind("change", FilteredNav.createUrl);
      }
    };

    // uncomment this line when testing on the console
    // FilteredNav.init();

    // comment this line when testing on the console
    $(document).ready(function() {
      FilteredNav.init();
    });

Your code looks pretty good! Here are a few comments:

-
Prefer to use .data('dept') over .attr('data-dept').

-
Prefer to use .on('change', ...) over .bind('change', ...).

-
rel is an invalid attribute on input elements. You can use data-rel instead.

-
It's a inefficient to create URLs for every FilteredNav.wrapper whenever any checkbox is changed, since it should only affect one of them. Consider having changeUrl only affect the enclosing .dept-filters. (This is as simple as just removing the .each() and setting self = $(this).closest(FilteredNav.wrapper);.) Then in init, use $.each(FilteredNav.wrapper, changeUrl);.

This might look like:



var FilteredNav = {
wrapper: $(".dept-filters")
, init: function() {
FilteredNav.changeUrl();
$.each(FilteredNav.wrapper, FilteredNav.createUrl);
}
, createUrl: function() {
var urlItems = [];

var self = $(this).closest(FilteredNav.wrapper)
, checkedInput = self.find('input[type="checkbox"]:checked')
, submitButton = self.find('.refinar-btn');

// select only the relevant items
$.each(checkedInput, function(i, item) {
urlItems.push($(item).data("rel"));
});

if (checkedInput.length) {
// create the url
var url = document.location.protocol
+ "//"
+ document.location.host
+ "/"
+ self.data("dept")
+ "/?"
+ urlItems.join("&");

// add new url as href of 'refine' button
submitButton.removeClass("hide").attr("href", url);
} else {
submitButton.addClass("hide");
}

// Just to demonstrate
submitButton.siblings('output').text(submitButton.attr('href'));
}
, changeUrl: function() {
FilteredNav.wrapper.find('input[type="checkbox"]').on("change", FilteredNav.createUrl);
}
};

// uncomment this line when testing on the console
// FilteredNav.init();

// comment this line when testing on the console
$(document).ready(function() {
FilteredNav.init();

// don't leave my Stack Snippet! :)
$('a').on('click', function() { return false; });
});
a {
text-decoration: none;
color: inherit;
}

.refinar-btn {
border: 1px solid black;
display: inline-block;
padding: 1ex;
}

.refinar-btn.hide {
background: #eee;
color: #777;
}






Filter 1



Filter 2



Filter 3


Refine






Filter A



Filter B



Filter C


Refine




• You can take this even further and only generate the URL when the submit button is actually clicked. You would then have to add or remove the .hide class in a different way. One solution is to have a very simple change handler for the checkboxes that just sets .hide if any are checked. Alternatively, you could move it entirely out of the script and use CSS similar to the following, which would be pretty dependent on your structure. For example, it wouldn't work in my snippet above.

.refinar-btn {
    /* hide styles, e.g.: */
    opacity: 0.5;
    box-shadow: 0;
}

input[type="checkbox"]:checked ~ .refinar-btn {
    /* show styles, e.g.: */
    opacity: 1;
    box-shadow: ...;
}

StackExchange Code Review Q#96143, answer score: 2