snippetjavascriptMinor
Filter by vendor-feature with jQuery & Isotope
Viewed 0 times
withvendorisotopefilterjqueryfeature
Problem
Task description:
A set of product-thumbnails is shown. The user can filter the shown products by vendor(s). Done by clicking one or multiple buttons.
I used jQuery together with the Isotope-plugin: http://isotope.metafizzy.co/
First I had the idea of using iteration for assembling the filter-criterium string:
`
Apple
Garmin
Fitbit
Polar
A set of product-thumbnails is shown. The user can filter the shown products by vendor(s). Done by clicking one or multiple buttons.
I used jQuery together with the Isotope-plugin: http://isotope.metafizzy.co/
First I had the idea of using iteration for assembling the filter-criterium string:
$(function() {
$products = $('.products');
$products.isotope({
itemSelector: '.product',
layoutMode: 'fitRows'
})
$('.vendor').click(function() {
let filterCrit = '';
$(this).toggleClass('active');
// Iterate over the vendor-buttons ...
$('.vendor').each(function(i, product) {
// ... if button is "active" => Add vendor-name to filter-criterium.
if ($(this).hasClass('active')) {
filterCrit += ', .' + $(this).data('vendor');
}
});
// Remove the trailing ", " and use the string as filter-criterium.
$products.isotope({
filter: filterCrit.slice(2)
});
});
});
* {
padding: 0;
margin: 0;
}
body {
background-color: #282828;
color: #efefef;
}
.products {
width: 100%;
}
.product {
margin-left: 5px;
margin-bottom: 5px;
float: left;
width: 100px;
}
.wrap {
max-width: 1300px;
width: 100%;
margin: 30px auto;
padding: 10px 30px;
}
nav {
padding: 10px 20px;
overflow: hidden;
display: flex;
justify-content: space-between;
flex-wrap: wrap;
width: 100%;
}
a.vendor {
text-decoration: none;
text-transform: uppercase;
font-size: 1.4rem;
padding: 20px 10px;
border: 3px solid white;
display: inline-block;
width: 20%;
min-width: 125px;
text-align: center;
color: #efefef;
margin-bottom: 5px;
}
a.vendor:hover {
color: #101010;
background-color: #d7d7d7;
}
a.vendor.active {
background-color: silver;
color: #010101;
box-shadow: inset 1px 1px 2px #555;
}
`
Apple
Garmin
Fitbit
Polar
Solution
Dropping variables in the global scope
Creating a string with a common delimiter
In the first snippet you use a somewhat ugly way of creating a string with a common delimiter. It is not very readable, and produces that "trailing comma" that you later have to remove. Instead, use
Note: If you didn't need the string, but instead could use a jQuery collection of elements, you could use
Semicolons
Both your first and second snippet are missing a semicolon. Don't rely on automatic semicolon insertion (ASI), as it will more likely bite you than save you.
Function with side-effect
In the second snippet,
I am unsure why you have a closure inside an anonymous function. Sure, it hides
In the second snippet, use
You are using
Arrow functions
Arrow functions are not supported in Internet Explorer. If you are using something like babel to convert newer syntax to older syntax that is more widely supported you have nothing to worry about.
In the end, I would suggest to use the first version, as it is clearer. The second version might give a better performance if you have an incredible large number of categories, but your application will be unusable from a consumer standpoint if that is the case. In any case, the number of product elements selected will always overshadow the number of categories selected.
$products is not declared. You are dropping it into the global scope... Consider using strict mode ("use strict";) to cause a SyntaxError rather than allowing this to happen.Creating a string with a common delimiter
In the first snippet you use a somewhat ugly way of creating a string with a common delimiter. It is not very readable, and produces that "trailing comma" that you later have to remove. Instead, use
Array.prototype.join():$('.vendor').click(function() {
let filterVendors = [];
$(this).toggleClass('active');
// Iterate over the vendor-buttons ...
$('.vendor').each(function(i, product) {
// ... if button is "active" => Add vendor-name to filter-criterium.
if ($(this).hasClass('active')) {
filterVendors.push('.' + $(this).data('vendor'));
}
});
$products.isotope({
filter: filterVendors.join(', ')
});
});Note: If you didn't need the string, but instead could use a jQuery collection of elements, you could use
let a = $('') and a.add('another selector') to build the collection. This would be cleaner than dynamically creating one big selector, but is not possible in this specific case, because you need the selector string.Semicolons
Both your first and second snippet are missing a semicolon. Don't rely on automatic semicolon insertion (ASI), as it will more likely bite you than save you.
Function with side-effect
In the second snippet,
getCurrent(..) is a function with a side-effect. It both toggles the current selection and returns a selector. Try to avoid this.I am unsure why you have a closure inside an anonymous function. Sure, it hides
active from the rest of the function... but it requires you to have that function with side-effect. Just pull it out into the anonymous function, and make two functions..attr(data- instead of .data(In the second snippet, use
.data(..) instead of .attr(..) to benefit from (future) optimizations related to data storage on DOM elements.let vs varYou are using
var in the second snippet. While it is not wrong to use var, be consistent. Decide for which version of Ecmascript you are writing, and which browsers need to be supported, and use that version consistently. In this case, use let for both of those variables.Arrow functions
Arrow functions are not supported in Internet Explorer. If you are using something like babel to convert newer syntax to older syntax that is more widely supported you have nothing to worry about.
In the end, I would suggest to use the first version, as it is clearer. The second version might give a better performance if you have an incredible large number of categories, but your application will be unusable from a consumer standpoint if that is the case. In any case, the number of product elements selected will always overshadow the number of categories selected.
$(function() {
let $products = $('.products');
$products.isotope({
itemSelector: '.product',
layoutMode: 'fitRows'
});
$('.vendor').click(function() {
let filterVendors = [];
$(this).toggleClass('active');
// Iterate over the vendor-buttons ...
$('.vendor').each(function(i, product) {
// ... if button is "active" => Add vendor-name to filter-criterium.
if ($(this).hasClass('active')) {
filterVendors.push($(this).data('vendor'));
}
});
// Remove the trailing ", " and use the string as filter-criterium.
$products.isotope({
filter: filterVendors.join(', ')
});
});
});Code Snippets
$('.vendor').click(function() {
let filterVendors = [];
$(this).toggleClass('active');
// Iterate over the vendor-buttons ...
$('.vendor').each(function(i, product) {
// ... if button is "active" => Add vendor-name to filter-criterium.
if ($(this).hasClass('active')) {
filterVendors.push('.' + $(this).data('vendor'));
}
});
$products.isotope({
filter: filterVendors.join(', ')
});
});$(function() {
let $products = $('.products');
$products.isotope({
itemSelector: '.product',
layoutMode: 'fitRows'
});
$('.vendor').click(function() {
let filterVendors = [];
$(this).toggleClass('active');
// Iterate over the vendor-buttons ...
$('.vendor').each(function(i, product) {
// ... if button is "active" => Add vendor-name to filter-criterium.
if ($(this).hasClass('active')) {
filterVendors.push($(this).data('vendor'));
}
});
// Remove the trailing ", " and use the string as filter-criterium.
$products.isotope({
filter: filterVendors.join(', ')
});
});
});Context
StackExchange Code Review Q#139830, answer score: 2
Revisions (0)
No revisions yet.