4
\$\begingroup\$

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();
    });
\$\endgroup\$
2
\$\begingroup\$

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;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<div class="container">
  <div class="dept-filters" data-dept="numbers">
    <div class="boxes">
      <label>
        <input type="checkbox" data-rel="filter1" />
        <span>Filter 1</span>
      </label>
      <label>
        <input type="checkbox" data-rel="filter2" />
        <span>Filter 2</span>
      </label>
      <label>
        <input type="checkbox" data-rel="filter3" />
        <span>Filter 3</span>
      </label>
    </div>
    <a class="refinar-btn">Refine</a>
    <output></output>
  </div>
  <div class="dept-filters" data-dept="letters">
    <div class="boxes">
      <label>
        <input type="checkbox" data-rel="filterA" />
        <span>Filter A</span>
      </label>
      <label>
        <input type="checkbox" data-rel="filterB" />
        <span>Filter B</span>
      </label>
      <label>
        <input type="checkbox" data-rel="filterC" />
        <span>Filter C</span>
      </label>
    </div>
    <a class="refinar-btn">Refine</a>
    <output></output>
  </div>
</div>

  • 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: ...;
}
\$\endgroup\$
  • \$\begingroup\$ All the tips will seem to add up beautifully to my code, but I have one doubt: will this css solution work for ie8? It seems pretty good and I would have saved a few lines of code latter on. Thanks for all the help! \$\endgroup\$ – Edson Júnior Jul 8 '15 at 16:36
  • \$\begingroup\$ @EdsonJúnior If your structure permits it, it should; MDN says it's supported from IE7. \$\endgroup\$ – Schism Jul 8 '15 at 16:54
  • \$\begingroup\$ @EdsonJúnior Never mind. Turns out :checked is only supported from IE9. \$\endgroup\$ – Schism Jul 8 '15 at 16:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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