0

This filter works. Each time you select a category from one of the drop down menus (using art terminology as an example), the filtered results are displayed with animation. I just want to make sure there are no glaring code discrepancies or incompatibilities before I move on with the project. Or if you know a more elegant way to achieve the same, please reply. Thanks. Here's the fiddle:

http://jsfiddle.net/Earl1234/qm5Yj/2/

Earl1234
  • 37
  • 7

1 Answers1

1

Well, your #maindiv isn't using proper HTML syntax, and, when it does, it messes up the layout.

Shouldn't you jQuery be a bit more like this though?

Demo: http://jsfiddle.net/SO_AMK/yrwVZ/

jQuery:

$(document).ready(function() {
    $("#genre").change(function() {
        var selGenre = $(this).val();
        var selSize = $("#size").val();
        $("#paintings").slideUp(function() {
            $('.eaPaintDiv, .error').hide();
            $('.eaPaintDiv[class*=' + selGenre + '][class*=' + selSize + ']').show();
            if ($('.eaPaintDiv[class*=' + selGenre + '][class*=' + selSize + ']').length == 0) {
                $(".error").show();
            }
            $("#paintings").slideDown();
        });
    });
    $("#size").change(function() {
        var selGenre = $("#genre").val();
        var selSize = $(this).val();
        $("#paintings").slideUp(function() {
            $('.eaPaintDiv, .error').hide();
            $('.eaPaintDiv[class*=' + selSize + '][class*=' + selGenre + ']').show();
            if ($('.eaPaintDiv[class*=' + selSize + '][class*=' + selGenre + ']').length == 0) {
                $(".error").show();
            }
            $("#paintings").slideDown();
        });
    });
});​

I added the error for appearances sake. Your filtering is slightly redundant and could possibly mess up if you only hide the selected paintings initially. So, instead, you can just hide them all then show the correct ones.

P.S. You can use my float clearing method, instead of an empty DIV, like this:

#paintings:after {
    display: block;
    content: " ";
    height: 0;
    clear: both;
}
Community
  • 1
  • 1
A.M.K
  • 16,727
  • 4
  • 32
  • 64
  • Excellent. Thanks, A.M.K. Appreciate the tips. The mainDiv issue was a mistake. Shouldn't have even been in there. – Earl1234 Sep 20 '12 at 13:39
  • A.M.K, if you're up for another, I'm still trying to nail down this one: http://stackoverflow.com/questions/12487937/jquery-ui-effects-core-addclass-removeclass-on-one-element-with-multiple-selec – Earl1234 Sep 20 '12 at 13:41