3
\$\begingroup\$

I built a simple product filtering feature that works at least in the browsers and platforms that I have tried using it on, but I am unsure about a couple of aspects of it. I built this on a Bigcommerce site but I think my questions could still apply broadly (some):

  1. Is this a solid approach?  Conventional? If not, why, and how could/should I approach it differently?   

  2. Should I expect to have any problems with browser compatibility?  If so, would they be different problems than any problems I might experience if I was using the stock Bigcommerce faceted search option, or even some other random search feature from another site?

Assume an out of the box faceted search option is not an available. jQuery is loaded twice in HTMLhead.html (head template) as 1.7.2 and 2.2.2

The following example is a simplified version of what I created.  In this example there are only 4 products and 2 categories present.  We can also assume that no products belong in more than one category:

HTML - (Two checkboxes, one for each category)

<fieldset>
    <div class="col-md-6 test-one"><label><input id="example-category-1" type="checkbox" value="first-checkbox" />CHOOSE CATEGORY 1</label></div>
    <div class="col-md-6 test-one"><label><input id="example-category-2" type="checkbox" value="second-checkbox" />CHOOSE CATEGORY 2</label></div>
</fieldset>

HTML - fiddle with expanded li showing the structure of a random example product.

JS:

Page loads, all products present, no checkboxes clicked. When a checkbox is clicked for the first time, all products are hidden (rather, their parent lis are)

    var test = true;
    $(".test-one").change(function() {
        if(test){           
            $('div[data-product]').parents("li").hide();
            //hide every product after first click on any checkbox 
            
            if( $('#example-category-1').is(':checked') ) {
            //check for checkbox activation and show products if true
                $('div[data-product="1802"]').parents("li").show();
                $('div[data-product="1781"]').parents("li").show();
            }
            if( $('#category-2').is(':checked') ) {
            //check for checkbox activation and show products if true
                $('div[data-product="1348"]').parents("li").show();
                $('div[data-product="1347"]').parents("li").show();
            }
            test =false;
            //set to false so we don't enter this if statement again
        }
    });

For the rest of the time, this is what is handling the filtering (would like to exclude the following code from the first click, but seems to work okay as is):

$('#example-category-1').click(function() {
        if( $(this).is(':checked') ) {
            $('div[data-product="1802"]').parents("li").show();
            $('div[data-product="1781"]').parents("li").show();
        } else {
            $('div[data-product="1802"]').parents("li").hide();
            $('div[data-product="1781"]').parents("li").hide();
        }
    });
    $('#example-category-2').click(function() {
        if( $(this).is(':checked') ) {
            $('div[data-product="1348"]').parents("li").show();
            $('div[data-product="1347"]').parents("li").show();
        } else {
            $('div[data-product="1348"]').parents("li").hide();
            $('div[data-product="1347"]').parents("li").hide();
        }
    });

Final note: I was told that I can manipulate the product object to achieve goals like this one - I kind of know in theory what the product object is, but I don't know how to access it / act on it - any input, test suggestions, or criticism would be greatly appreciated.

\$\endgroup\$
1
\$\begingroup\$

I don't see how this is scalable if you have to hard code product id's into your jQuery code and duplicate handlers for every category you may want to add.

I also see no need to have separate logic for "first time" execution vs. every other time.

I would consider putting a data- attribute on each checkbox that can indicate the target category the checkbox filter is to work against. And then put a data- attribute on each product to indicate what category it belongs to.

So let's assume your products are like this:

<div class="product" data-category="1">...</div>
<div class="product" data-category="2">...</div>

And your category checkboxes are like:

<input class="category_filter" data-category-target="1" type="checkbox" value="first-checkbox" />
<input class="category_filter" data-category-target="2" type="checkbox" value="second-checkbox" />

Then your jQuery might be simplified down to something like:

// cache references to DOM elements to eliminate need to requery
var $categoryFilters = $('.category_filter');
var $allProducts = $('.product');
// build out selectors for each category
var productsByCategory = {};
$categoryFilters.each(function()) {
     var category = $(this).data('category-target');
     productsByCategory[category] = $allProducts.filter('[data-category=' + category + ']');
});

// now attach change handler to filters
$categoryFilters.on('change', function() {
    $allProducts.hide();
    $categoryFilters.each(function() {
        var target = $(this).data('category-target');      
        if($(this).is(':checked')) {
            productsByCategory[target].show();
        }
    }
});

Why are you loading two versions of jQuery? If this is absolutely unavoidable, you should be aliasing these versions appropriately (i.e. to something other than $) so that you can write code against whichever version is pertinent to that section of code.


Anytime you find yourself duplicating code for cases like you have with #example-category-*, this should be an immediate red flag to you that have established a coding antipattern. For this specific example, what you really have is class level behavior (all of the category checkboxes exhibit similar behavior in this case). That typically mean you should be writing code against the class of elements, not against individual unique id's.

\$\endgroup\$
  • \$\begingroup\$ Unfortunately I am unable to assign attributes to each particular product. This is on the Bigcommerce platform, so the products are generated to the page from a product object (?). You can see the link to my fiddle which shows what they look like after they are generated to the page. Support have told me that there is no way to assign an attribute like this, which I find very hard to believe, but they are not always correct. \$\endgroup\$ – Tron Jan 25 '17 at 18:33
  • \$\begingroup\$ Re: Antipattern, I haven't heard that term used before but I appreciate the info and I would like to know more - I'm not sure what EXACTLY you are referring to as the antipattern that I am creating, and also what exactly do you mean "against the class of elements, not against individ......" I'm finding different explanations of the term. \$\endgroup\$ – Tron Jan 25 '17 at 18:35
  • \$\begingroup\$ Re: jQuery - It was someone else who added it in and I don't know why so I am trying to contact them before doing anything myself. I wonder if one version is being loaded for a different section or purpose than the other? What kinds of serious issues should I be aware of? \$\endgroup\$ – Tron Jan 25 '17 at 18:37
  • \$\begingroup\$ @Tron RE: data attributes. Somewhere you MUST hold the knowledge of the mapping of products to categories. Even if you have to manually create the relationship (like you are doing now), you would be better served to build a javascript configuration object or similar that holds that mapping that your can be used by this portion of code. The goal should be to eliminate hard-coded branching in your jQuery function that is, in essence related to category configuration. \$\endgroup\$ – Mike Brant Jan 25 '17 at 19:28
  • \$\begingroup\$ @Tron By antipattern, I mean frequent patterns one see in software development that should be avoided. In your case, I am specifically referring to code that is repeated over and over and over with hardcoded [some element]-[some number] type of namings. This is a bad development pattern that you should learn to recognize and resolve. In this case, you resolve it by treating your checkboxes as a class of elements that all have similar behavior rather than a series of individual elements with duplicated behavior attached to them. So attache behavior to a class not a unique id. \$\endgroup\$ – Mike Brant Jan 25 '17 at 19:33

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.