Repeating JS, function required?

[Gmattgreenfield]

https://github.com/Gmattgreenfield/Filtered-Catalogue-WP-Plugin/blob/master/assets/js/filtered_catalogue_js.js#L17

Line 17 and 34 are basically the same thing but one is brand and the other category.

I'm wondering if its more efficient to pass category and brand in as parameters to a single function. I tried it but got a bit lost.....

[AidanThreadgold]

Try -

function toggleSelectState(type) {

    if(type != "category" && type != "brand")  {
        return; // stop the function.
    }

    // Get the class name of the clicked checkbox
    var className = $(this).attr("value");

    if(!className) {
        return;
    }

    // Show or Hide elements with the above ID as a class name
    $('.' + className).toggleClass( type + "-is-visable" );

    if ( $(".catalogue__product").hasClass( type + "-is-visable") ) {
        // if any [type] checkboxes are checked
        $('.catalogue__product').removeClass( type + "-not-selected" );
    } else {
        // if none are checked
        $('.catalogue__product').addClass( type + "-not-selected" );
    }
};

Very straight forward, just taken what you already did and removed reference to "category" or "brand" with the value of the type parameter. You spelt visable wrong by the way ;)

[Gmattgreenfield]

Yer I was do something like that but it wouldn't work.. https://github.com/Gmattgreenfield/Filtered-Catalogue-WP-Plugin/compare/javascript-function-improvements?diff=split&name=javascript-function-improvements

I did indeed spell visible wrong... you spelling it visable just confused me even more haha.

Will investigate, thanks again for the help!

[Gmattgreenfield]

Is it a problem with how I'm running that function? $( ".checkbox--category" ).change( toggleSelectState(category) );

I've create a branch with the above code called issue-4 Its a shame I cant set up a test site for you really...

[AidanThreadgold]

Just need to wrap category in quotes -

$( ".checkbox--category" ).change( toggleSelectState("category") );

Or (IE 9+):

var checkboxes = document.getElementsByClassName(".checkbox--category");
for(var i = 0; i < checkboxes.length; ++i ) {
  checkboxes[i].addEventListener("change", toggleSelectState("category")); 
}

Untested, not sure 100% if it'll work... Docs: https://developer.mozilla.org/en/docs/Web/API/EventTarget.addEventListener

You could try markup/js it in a jsbin to quickly test & share the code.