I have the following javascript that switches the different views. All works fine however it is extremely repetitive.
I would like to clean it up to be optimally usable, but not to sure how I would go about it.
Any help with this would be highly appreciated.
Below is my code that i wrote in order to get it to work. It works well but seems very bloated... don't know what more to say about it but red box telling me to add more text is just not going away, so not to sure how much more i am suppose to type in order to get some assistance with my question at had here.
function switchView () {
var viewContainer = document.getElementById('content-wrapper');
var tileBtn = document.getElementById('tile-btn');
var listBtn = document.getElementById('list-btn');
var swipeBtn = document.getElementById('swipe-btn');
tileBtn.addEventListener( 'click', function(e){
if (viewContainer.classList.contains('list-view') ||
viewContainer.classList.contains('swipe-view') ) {
e.preventDefault();
viewContainer.classList.remove('list-view');
viewContainer.classList.remove('swipe-view');
viewContainer.classList.add('tile-view');
}
});
listBtn.addEventListener( 'click', function(e){
if (viewContainer.classList.contains('tile-view') ||
viewContainer.classList.contains('swipe-view') ) {
e.preventDefault();
viewContainer.classList.remove('tile-view');
viewContainer.classList.remove('swipe-view');
viewContainer.classList.add('list-view');
}
});
swipeBtn.addEventListener( 'click', function(e){
if (viewContainer.classList.contains('list-view') ||
viewContainer.classList.contains('tile-view') ) {
e.preventDefault();
viewContainer.classList.remove('list-view');
viewContainer.classList.remove('tile-view');
viewContainer.classList.add('swipe-view');
}
});
};
switchView();
-
1\$\begingroup\$ It would be easier to help you if you provided the HTML on which this code acts. There are likely changes you could make there that would make this code less repetitive. \$\endgroup\$Heretic Monkey– Heretic Monkey2017年07月18日 14:57:16 +00:00Commented Jul 18, 2017 at 14:57
-
\$\begingroup\$ You are probably doing too much in JavaScript. It's hard to advise you properly, though, given the lack of context and detail in this question. \$\endgroup\$200_success– 200_success2017年09月16日 17:36:23 +00:00Commented Sep 16, 2017 at 17:36
2 Answers 2
Generally, when you have a lot of duplicated code you should try to take out the parts that are different and put them in an array, then loop the values and perform the operation based on the values.
Here's a quick re-factor I threw together of your code with that in mind. It could probably be improved upon using JS6 stuff like let
instead of the closure but at least this should illustrate the point.
function switchView () {
var viewContainer = document.getElementById('content-wrapper');
var buttons = [
{id: 'tile-btn', reqdClasses: ['list-view', 'swipe-view'], newClass: 'tile-view'},
{id: 'list-btn', reqdClasses: ['tile-view', 'swipe-view'], newClass: 'list-view'},
{id: 'swipe-btn', reqdClasses: ['list-view', 'tile-view'], newClass: 'swipe-view'}
];
for(var i=buttons.length; i--;)(function(i){
var button = document.getElementById(buttons[i].id);
button.addEventListener('click', function(e){
for(var n = buttons[i].reqdClasses.length; n--;){
if(!button.classList.contains(buttons[i].reqdClasses[n])) continue;
e.preventDefault();
for(var n = buttons[i].reqdClasses.length; n--;) button.classList.remove(buttons[i].reqdClasses[n]);
button.classList.add(buttons[i].newClass);
break;
}
});
})(i);
}
In these kinds of situations, it's often easier to just remove all of the classes, then add back the one you want. This generally happens fast enough that the user doesn't notice any flashing.
function viewSwitch(which) {
viewContainer.classList.remove('list-view','swipe-view','tile-view');
viewContainer.add(which);
}
function switchView() {
var viewContainer = document.getElementById('content-wrapper');
var tileBtn = document.getElementById('tile-btn');
var listBtn = document.getElementById('list-btn');
var swipeBtn = document.getElementById('swipe-btn');
tileBtn.addEventListener('click', function(e) {
e.preventDefault();
viewSwitch('tile-view');
});
listBtn.addEventListener('click', function(e) {
e.preventDefault();
viewSwitch('list-view');
});
swipeBtn.addEventListener('click', function(e) {
e.preventDefault();
viewSwitch('swipe-view');
});
};
switchView();