patternjavascriptMinor
Minimalist accordion implementation using vanilla JavaScript
Viewed 0 times
javascriptaccordionvanillausingimplementationminimalist
Problem
The following example is an accordion script in JavaScript that does not use jQuery. When I started looking for a simple solution to this requirement I did not accept the idea of having to use a powerful and robust library for a simple implementation. I didn't like the idea of use a machine gun when what I really need is a slingshot. So as a jQuery detox process I decide create my own solution. So far the code is simple enough and I’m planning using it in production as a custom widget in my blog running Wordpress. I’m very enthusiast of minimalist development and I have some concerns with this code.
Any suggestions about how I can achieve a simpler and elegant solution? I was thinking in implementing the
Here is a live demo of the accordion script.
HTML
CSS
`.widget
{
width:60%;
margin:50px auto 0 auto;
border:4px solid rgba(10,10,10,0.05);
padding:10px;
border-radius:10px;
-webkit-box-shadow: 10px 10px 45px -20px rgba(0,0,0,0.75);
-moz-box-shadow: 10px 10px 45px -20px rgba(0,0,0,0.75);
box-shadow: 10px 10px 45px -20px rgba(0,0,0,0.75);
}
.widget-title
{
font-weight:bold;
text-transform:uppercase;
color:#636363;
padding-bottom:10px
}
.widget a
{
color:#9b9b9b;
text-decoration:none;
padding:10px 0 10px 10px;
display:inline-block;
}
.widget a + a
{
padding:10px 16px;
float:right;
line-height:1.5;
}
.widget a + a:hover
{
background-color:#ddd;
color:#fff;
}
.widget a:hover
{
color:#444
}
.widget ul
{
list-style-type:none;
margin:0;
padding:0;
background-color:#eee;
}
.widget ul
{
line-height:1.5;
}
.widget li
{
border-top
Any suggestions about how I can achieve a simpler and elegant solution? I was thinking in implementing the
element.classList properties.Here is a live demo of the accordion script.
HTML
Archivos
- 2015
- febrero 2015
- enero 2015
- 2014
- diciembre 2014
- noviembre 2014
- marzo 2014
- febrero 2014
- enero 2014
- 2013
- diciembre 2013
- septiembre 2013
CSS
`.widget
{
width:60%;
margin:50px auto 0 auto;
border:4px solid rgba(10,10,10,0.05);
padding:10px;
border-radius:10px;
-webkit-box-shadow: 10px 10px 45px -20px rgba(0,0,0,0.75);
-moz-box-shadow: 10px 10px 45px -20px rgba(0,0,0,0.75);
box-shadow: 10px 10px 45px -20px rgba(0,0,0,0.75);
}
.widget-title
{
font-weight:bold;
text-transform:uppercase;
color:#636363;
padding-bottom:10px
}
.widget a
{
color:#9b9b9b;
text-decoration:none;
padding:10px 0 10px 10px;
display:inline-block;
}
.widget a + a
{
padding:10px 16px;
float:right;
line-height:1.5;
}
.widget a + a:hover
{
background-color:#ddd;
color:#fff;
}
.widget a:hover
{
color:#444
}
.widget ul
{
list-style-type:none;
margin:0;
padding:0;
background-color:#eee;
}
.widget ul
{
line-height:1.5;
}
.widget li
{
border-top
Solution
Yes, you should use
Of course,
I also wonder why you use
Also, please don't mix languages in your code. I know it's only two variable names, but especially because it's only two variable names there's no reason to switch language. The DOM API is in English, and even all your CSS classes are in English, so it's simpler and cleaner to just stick to that (by the way, I'm not a native English speaker myself, I just prefer code to be consistent.)
Lastly, I always recommend that you use brace-on-same-line style in JavaScript. There are some assumptions about style in JS interpreters, and brace-on-same-line is one.
classList. Right now you overwrite the entire className attribute of the elements. It works, but it also means that you can never use any other class name on the elements. So it's just not a very precise or robust way of doing things; it only works as long as you don't add other classes. And if you do need to add another class to the elements at some point, your script will mess up because the comparison with "" will always fail, and then any and all class names will get summarily removed by your script.Of course,
classList isn't supported everywhere, so you may have to use a shim/polyfill. The nextElementSibling property isn't supported everywhere either, by the way (which, incidentally is why jQuery exists; because browser compatibility is not simple).I also wonder why you use
querySelector in one line, but elementsByClassName in the next line. In both cases you want to find element(s) with a given class name. Of course, in the first case, you want exactly 1 element, and in the other you want a collection. However, if you want 1 particular element, you should probably add an id attribute to that element - not a class - and then use the good old document.getElementById. In other words: Using a class selector and expecting it to only exist on one element is backwards. Classes are meant to be used on an arbitrary number of elements. If you want unique identification, use an id instead.Also, please don't mix languages in your code. I know it's only two variable names, but especially because it's only two variable names there's no reason to switch language. The DOM API is in English, and even all your CSS classes are in English, so it's simpler and cleaner to just stick to that (by the way, I'm not a native English speaker myself, I just prefer code to be consistent.)
Lastly, I always recommend that you use brace-on-same-line style in JavaScript. There are some assumptions about style in JS interpreters, and brace-on-same-line is one.
Context
StackExchange Code Review Q#83940, answer score: 2
Revisions (0)
No revisions yet.