5
\$\begingroup\$

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 element.classList properties.

Here is a live demo of the accordion script.

HTML

<aside id="archives" class="widget widget_archive"><h3 class="widget-title">Archivos</h3>
  <ul>
    <li><a href="#">2015</a>
      <a href="#"<i class="fa fa-angle-down"></i></a>
      <ul>
        <li><a href="#">febrero 2015</a></li>
        <li><a href="#">enero 2015</a></li>
      </ul>
    </li>
    <li><a href="#">2014</a>
      <a href="#"<i class="fa fa-angle-down"></i></a>
      <ul class="hide">
        <li><a href="#">diciembre 2014</a></li>
        <li><a href="#">noviembre 2014</a></li>
        <li><a href="#">marzo 2014</a></li>
        <li><a href="#">febrero 2014</a></li>
        <li><a href="#">enero 2014</a></li>
      </ul>
    </li>
    <li><a href="#">2013</a>
      <a href="#"<i class="fa fa-angle-down"></i></a>
      <ul class="hide">
        <li><a href="#">diciembre 2013</a></li>
        <li><a href="#">septiembre 2013</a></li>
      </ul>
    </li>
  </ul>
</aside>

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:1px solid #ccc;
}

.widget li li 
{
  line-height:1.5;
  border-top:1px solid #eaeaea;
  background-color:#fff;
}

.widget > ul li:first-child 
li a:first-child {
  display:inline-block;
}

.hide {
  display:none;
}

JS

"use strict";

var elementum = document.querySelector(".widget_archive");
var gestus = elementum.getElementsByClassName("fa");

for (var i = 0; i < gestus.length; i++) {
  gestus[i].addEventListener("click", toogle);
}

function toogle()
{
  if (this.nextElementSibling.className === "")
  {
    this.nextElementSibling.className = "hide";
  }
  else
  {
    this.nextElementSibling.className = ""
  }
}
\$\endgroup\$
  • \$\begingroup\$ Big fan of minimalist code. I once upon a time used jQuery also. Life is much better without it. I applaud your efforts. \$\endgroup\$ – Misunderstood Mar 20 '15 at 20:59
  • \$\begingroup\$ Thanks for your comment and support @Misunderstood. I firmly believe that we must know our tools and know where we can use them. I believe in jQuery and in other frameworks like Velocity.js and Underscore.js to name a few. But there is no greater satisfaction than knowing what problems frameworks solve and when to use them in our projects. \$\endgroup\$ – James Mar 21 '15 at 1:16
2
\$\begingroup\$

Yes, you should use 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.

\$\endgroup\$
  • \$\begingroup\$ Hey @Flambino a got a few comments about your answer, do you like to discuss it in a chat? \$\endgroup\$ – James Mar 13 '15 at 20:17
  • \$\begingroup\$ @James Sorry, been away. But feel free to ask here in the comments. I'll get to it, when I have the chance. \$\endgroup\$ – Flambino Mar 14 '15 at 22:24
  • \$\begingroup\$ The action to overwrite the className attribute is to avoid an initialization function in the js script. By this I mean a function that only display the current year panel leaving the others hides. I decided that I can achieve this by attaching the .hide class to the elements containing past years (this is donde in the backend side). Since I don't perceive using any class on those elements I decided to overwrite it. Also if I decide to edit the style I can easily target them with some css selectors. \$\endgroup\$ – James Mar 16 '15 at 14:50
  • \$\begingroup\$ I don’t clearly understand why comparison with "" will always fail, I use it for the toggling, but maybe I’m missing something. \$\endgroup\$ – James Mar 16 '15 at 14:50
  • \$\begingroup\$ The option to use a shim is basically a fact if I want to increase browser compatibility and for this little script will be the best bet. \$\endgroup\$ – James Mar 16 '15 at 14:51
1
\$\begingroup\$

The way I see it there are two accordion styles.

  1. Expand and Contract (ec) the one Section where multiple Sections can be Expanded or Contracted.
  2. Only one Section is Expanded at any given time. (ecp ec paired)

HTML

<button class="link" type="button" onclick ="ec('s1')">Button</button>
<div id="s1" class="hide">

</div>

<button class="link" type="button" onclick ="ec('s2')">Button</button>
<div id="s2" class="hide">

</div>
<div id="s999"></div>  

JS

I have two functions one for each the two EC methods.
function ec() is for EC method 1
functions ecp() is for EC method 2, pairs the current selected section with the previous.
Normally obe one of these functions is used on a web page.

The div id="s999" is for the initial value for prev so there is no null value to prev.

<script type="text/javascript"> //<![CDATA[
toggle[''] = 'block';
toggle['none'] = 'block';
toggle['block'] = 'none';
function ec(id){
  div = document.getElementById(id);
  div.style.display=toggle[div.style.display];
  var y=div.offsetTop;window.scrollTo(0, y-64);
}



var prev = document.getElementById('s999');
function ecp(id){ 
  div = document.getElementById(id);
  var disp = div.style.display;
  prev.style.display='none';
  div.style.display=toggle[disp];
  prev=div;
  var y=div.offsetTop;window.scrollTo(0, y-64);
}

//]]>
</script>

The reason I have var disp = div.style.display; is for when the same section is selected as previously. I do not care if prev is expanded or contracted just contracting it is much quicker than an else if to check its display state. But I need to remember what I am doing with the passed id. If the same section is clicked and I toggle the display state, if it is being expanded the prev toggle would contract it again.

The line of code:

var y=div.offsetTop;window.scrollTo(0, y-64);

Takes the expanded section and puts it at the top of the Browser window. When all are contracted, it scrolls to the very top or the page.

The reason I use an array for the toggle is to eliminate the else if that would otherwise be required.

I review every line of code as to how it will be compiled to machine code and then executed in the microprocessor execution engine and micro code. This is a fallback to when I started out in this business designing integrated circuits

I keep myself up to date with each new Intel micro architecture by reading Intel's Intel's 64 and IA-32 Architectures Optimization Reference Manual

Intel 64 and IA-32 Architectures Optimization Reference Manual

All programmers should be familiar with this manual.

You don't have to read all 642 pages. At a minimum read Chapter 3

Chapter 3 General Optimization Guidelines

and and read the User/Source Coding Rules.
Search the document using this search phrase: "User/Source Coding Rule".

Button vs Anchor

I prefer to use a button rather than an <a> anchor. The button is so much easier to style. I almost look at button as an enhanced <div>. The button's default display style is inline-block. Not so much an issue today, but inline-block was problematic with Browser support.

Using <a> with an href value of "#" is a jQuery thing. Not a good thing either. If you need to use an <a> as an actual link a a button can still be used. Just wrap a form around it. (Not SEO friendly)

<form action="link to where ever"><button type="submit">Link Text</button></form>

It also makes it easy to make a link a post and include hidden inputs

A post link can remedy cache no-cache issues. No cache use post, cached use get.

to make it SEO friendly you would need to add the <a href> elsewhere.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.