patternjavascriptMinor
jQuery responsive tabs to accordion
Viewed 0 times
responsivejqueryaccordiontabs
Problem
I wrote a script that adapts HTML tabs for different screen resolutions. On a small screen it's an Accordion and on wider ones they're Tabs. The script is for a production site, and I'm not so good in JavaScript yet, so show me any weaknesses.
`(function ($) {
// html structure
// div - selector
// // ul
// // // li.class - tabs
$.fn.tabslight = function () {
var classes = {
tabsTrigger: '.tab__trigger',
tabTypeAccordeon: 'tab-type-accordeon',
tabTypeTab: 'tab-type-tabs',
activeClass: 'current',
tabList: 'tab-list',
tabListItem: 'tab-list__item',
tabContent: 'tab-content',
tabContentItem: 'tab-content__item',
tabContent: 'tab__inner'
},
slideDuration = 300, //ms
container = $(this),
contentItems = container.find('.' + classes.tabContentItem),
isAccordeon = true,
tabsUpdate = function() {
var items = $('.' + classes.tabContentItem),
itemsTriger = $('.' + classes.tabListItem),
itemsContent = container.find('.' + classes.tabContent);
isAccordeon = ( $(window).width() ' + item + '';
});
string += '';
container.prepend(string);// Prepend list tabs for desktop statemenm
tabsUpdate(); // First run tabs update function.
}(),
triggers = container.find(classes.tabsTrigger);
// Listen for resize, and update tabs type
window.addEventListener('resize', tabsUpdate);
triggers.on('click', function (e) {
e.preventDefault(); // Disable the default action on clicking item
if ( isAccordeon ) { // if accordeon
var item = $(this).parent(),
itemContent = item.find('.' + classes.tabContent),
items = item.siblings();
itemsContent = items.find('.' + classes.tabContent);
if ( ! item.hasClass( classes.activeClass ) ) { // if elem don't has current class
items.removeClass(classes.activeClass);
item.addClass(classes.activeClass);
itemsContent.stop(true, true).slideUp(slideDuration); // hide siblings content
itemContent.stop(true
`(function ($) {
// html structure
// div - selector
// // ul
// // // li.class - tabs
$.fn.tabslight = function () {
var classes = {
tabsTrigger: '.tab__trigger',
tabTypeAccordeon: 'tab-type-accordeon',
tabTypeTab: 'tab-type-tabs',
activeClass: 'current',
tabList: 'tab-list',
tabListItem: 'tab-list__item',
tabContent: 'tab-content',
tabContentItem: 'tab-content__item',
tabContent: 'tab__inner'
},
slideDuration = 300, //ms
container = $(this),
contentItems = container.find('.' + classes.tabContentItem),
isAccordeon = true,
tabsUpdate = function() {
var items = $('.' + classes.tabContentItem),
itemsTriger = $('.' + classes.tabListItem),
itemsContent = container.find('.' + classes.tabContent);
isAccordeon = ( $(window).width() ' + item + '';
});
string += '';
container.prepend(string);// Prepend list tabs for desktop statemenm
tabsUpdate(); // First run tabs update function.
}(),
triggers = container.find(classes.tabsTrigger);
// Listen for resize, and update tabs type
window.addEventListener('resize', tabsUpdate);
triggers.on('click', function (e) {
e.preventDefault(); // Disable the default action on clicking item
if ( isAccordeon ) { // if accordeon
var item = $(this).parent(),
itemContent = item.find('.' + classes.tabContent),
items = item.siblings();
itemsContent = items.find('.' + classes.tabContent);
if ( ! item.hasClass( classes.activeClass ) ) { // if elem don't has current class
items.removeClass(classes.activeClass);
item.addClass(classes.activeClass);
itemsContent.stop(true, true).slideUp(slideDuration); // hide siblings content
itemContent.stop(true
Solution
This is a partial review
Comma operator
Don't use the comma operator. Besides that it is a maintenance hell, and increases the amount of variables that are accidentally dropped in the global namespace, it serves no practical purpose here. On top of that, you even use it outside
Your use of the comma operator also messes with the indentation you are using, making it harder to read your code.
In the code above you have a semicolon after defining all variables. Then have two more statements seperated by a comma, with a semicolon. Then another statement with a semicolon. Why? Just use the semicolon (
Naming
Decide on a consistent naming scheme for your classes. It makes little sense to use both the
Comma operator
Don't use the comma operator. Besides that it is a maintenance hell, and increases the amount of variables that are accidentally dropped in the global namespace, it serves no practical purpose here. On top of that, you even use it outside
var statements, creating something that is even more incomprehensible to review.Your use of the comma operator also messes with the indentation you are using, making it harder to read your code.
var itemTrigger = $(this).parent(),
itemTriggerSiblings = itemTrigger.siblings(),
itemTriggerNumber = itemTrigger.index(),
item = container.find('.' + classes.tabContentItem).eq(itemTriggerNumber);
itemContent = item.find('.' + classes.tabContent),
items = item.siblings();
itemsContent = items.find('.' + classes.tabContent);In the code above you have a semicolon after defining all variables. Then have two more statements seperated by a comma, with a semicolon. Then another statement with a semicolon. Why? Just use the semicolon (
;) after each statement.Naming
Decide on a consistent naming scheme for your classes. It makes little sense to use both the
- and the __ separator in those class names.Code Snippets
var itemTrigger = $(this).parent(),
itemTriggerSiblings = itemTrigger.siblings(),
itemTriggerNumber = itemTrigger.index(),
item = container.find('.' + classes.tabContentItem).eq(itemTriggerNumber);
itemContent = item.find('.' + classes.tabContent),
items = item.siblings();
itemsContent = items.find('.' + classes.tabContent);Context
StackExchange Code Review Q#149114, answer score: 2
Revisions (0)
No revisions yet.