When you see that you're basically copy-pasting the code and changing a few values, you can usually encapsulate all the common stuff inside a function. Here:
var registerStuff = function(category, active, prod, select) {
$(category).click(function(){
var CategoryID = $(this).data('category');
$(category).removeClass(active);
$(this).addClass(active);
$(prod).each(function(){
if(($(this).hasClass(CategoryID)) == false){
$(this).css({'display':'none'});
};
});
$('.'+CategoryID).fadeIn();
});
$(select).change(function(){
var CategoryID = $(this).find('option:selected').data('category');
$(prod).each(function(){
if(($(this).hasClass(CategoryID)) == false){
$(this).css({'display':'none'});
};
});
$('.'+CategoryID).fadeIn();
});
};
registerStuff(".category-menu ul li", 'cat-active', '.prod-cnt', '.category-menu select');
registerStuff(".category-menu2 ul li", 'cat-active2', '.prod-cnt2', '.category-menu2 select');
We can now clean up the inside of that function further. For example, some things could be cached, and the second $(category)
is uneccessary. The == false
is a code smell, rather use a negation. The jQuery hide()
method is better than spelling out the css()
. Blending out the prods of the wrong category is common code, and can be factored out as well.
var registerStuff = function(category, active, prod, select) {
var displayOnlyWantedCategory = function($prods, categoryId) {
$prods.each(function() {
var $this = $(this);
if (! $this.hasClass(categoryId)) {
$this.hide();
}
});
$('.' + categoryId).fadeIn();
};
var $category = $(category);
$category.click(function() {
var $this = $(this);
$category.removeClass(active);
$this.addClass(active);
displayOnlyWantedCategory($(prod), $this.data('category'));
});
$(select).change(function() {
displayOnlyWantedCategory($(prod), $(this).find('option:selected').data('category'));
});
};
The next thing to consider is your usage of classes. Your category-menu
and category-menu2
are visually and functionally identical, they're only two instances of the same idea. What happens when you want to have fifteen category-menu
s on the same page? I wouldn't want to see the CSS for that:
.category-menu,
.category-menu2,
.category-menu3,
...
.category-menu14,
.category-menu15 {
...
}
Your solution does not scale. Instead, let's think about a product-collection
element, which contains a few buttons to select a category, and a few items which can be displayed. We want to limit the effect of selecting a category to the current collection only.
The resulting HTML might look like:
<div class="product-collection">
<ul>
<li class="product-collection__selector
product-collection__selector--active"
data-product-collection__category="">All</li>
<li class="product-collection__selector"
data-product-collection__category="cat1">Category 1</li>
<li class="product-collection__selector"
data-product-collection__category="cat2">Category 2</li>
</ul>
<ul>
<li class="product-collection__item"
data-product-collection__category="cat1">Item 1 [cat 1]</li>
<li class="product-collection__item"
data-product-collection__category="cat2">Item 2 [cat 2]</li>
<li class="product-collection__item"
data-product-collection__category="cat1">Item 3 [cat 1]</li>
</ul>
</div>
The jQuery initialization must take care to install the handlers only in the elements of the current collection:
$(function() {
$('.product-collection').each(function() {
var $collection = $(this);
var $selectors = $collection.find('.product-collection__selector');
var $items = $collection.find('.product-collection__item');
$selectors.click(function() {
var $selector = $(this);
var cat = $selector.data('product-collection__category');
$selectors.removeClass('product-collection__selector--active');
$selector.addClass('product-collection__selector--active');
if (cat) {
$items.each(function() {
var $item = $(this);
if ($item.data('product-collection__category') == cat)
$item.fadeIn();
else
$item.hide();
});
}
else {
$items.fadeIn();
}
});
});
});
See it in action here: http://jsfiddle.net/Qv6QE/3/
My class names may seem excessively verbose, but they avoid namespace conflicts by following the Block-Element-Modifier naming scheme.