A Very Simple Javascript Accordion Like Animation

[faizza007]

hi,

I am a newbie in programming...recently i have developed a very simple (probably too simple!!!) javascript code that trims strings and shows animation....the code should be self explanatory....and i have included comments in the snippet so everyone understands what i did....any comments appreciated....just save the txt file as .html and double click to see the output in browser...

[Taywin]

It's nice that you are trying to learn more. From looking at your code, here is my comment...

1) I believe you are coding it on IE only. One obvious difference that this code won't work on FF is that the value of style pixel does not have 'px' at the end.
2) Your variable names do not explain themselves. This would be difficult for others who want to read your code.
3) Even though you said you have included comments, your comments are too brief and not really helpful. Furthermore, some functions have no comment at all. Documenting your code means anyone who looks at your code can understand what and how your function works without going through the function.
4) You could merge all your animation functions into one or two. Your animation function is not useful and will need a lot more if you want more steps in it.
5) Your code contains too many hard-coded variables -- looks for specific div IDs. This won't be extensible and/or flexible enough for other purposes in the future.
6) You use a div innerHTML to trigger the animation which is very weird to me. You could use a button to trigger the animation function instead.

PS: I do not know why you need to keep hiding & showing your display?

Below is a sample of what I would attempt to merge all your animation functions into one. Just a sample anyway... You could modify it the way you want it to your purpose...

// your code
function animateOpen1() {
  document.getElementById('cont1').style.height=50;
  document.getElementById('cont1').style.display='';
  setTimeout('animateOpen2()',100);
}

function animateOpen2() {
  document.getElementById('cont1').style.height=100;
  setTimeout('animateOpen3()',100);
}

function animateOpen3() {
  document.getElementById('cont1').style.height=150;
  document.getElementById('div1').innerText='Open';
}

function animateClose1() {
  document.getElementById('cont1').style.height=100;
  //document.getElementById('cont1').style.display='';
  setTimeout('animateClose2()',100);
}

function animateClose2() {
  document.getElementById('cont1').style.height=50;
  //document.getElementById('cont1').style.display='';
  setTimeout('animateClose3()',100);
}

function animateClose3() {
  document.getElementById('cont1').style.height=0;
  document.getElementById('cont1').style.display='none';
  document.getElementById('div1').innerText='Close';
}

// =======================

// sample code
// Display an animation of growing style size of an element from 0px
// up to 150px, and each step is 50px. Ignore if the
// incoming element is not found.
//
// @param  contentDivId  string which is the target element for
//                       changing style size
// @param  height  integer which is the assigned style size
function animationOpen(contentDivId, height) {
  var el = document.getElementById(contentDivId)
  if (el) {  // only do it if the element exists
    height = parseInt(height, 10)  // ensure integer because of calling from setTimeout()
    el.style.height = height+"px"
    height += 50
    if (height>150) {
      height = 150
      window.setTimeout("animationClose('"+contentDivId+"', "+height+")", 10)
    }
    else {
      window.setTimeout("animationOpen('"+contentDivId+"', "+height+")", 100)
    }
  }
}  // animationOpen(string, int)

// Display an animation of shrinking style size of an element from 150px
// down to 0px, and each step is 50px. Ignore if the
// incoming element is not found.
//
// @param  contentDivId  string which is the target element for
//                       changing style size
// @param  height  integer which is the assigned style size
function animationClose(contentDivId, height) {
  var el = document.getElementById(contentDivId)
  if (el) {  // only do it if the element exists
    height = parseInt(height, 10)  // ensure integer because of calling from setTimeout()
    el.style.height = height+"px"
    height -= 50
    if (height<0) {
      height = 0
      window.setTimeout("animationOpen('"+contentDivId+"', "+height+")", 10)
    }
    else {
      window.setTimeout("animationClose('"+contentDivId+"', "+height+")", 100)
    }
  }
}  // animationClose(string, int)

// when you use it, just put the function onclick on a button in HTML as...
// onclick="animationOpen('cont1', 50)"

[faizza007]

Hey..thank you for your comment...actually i just wanted to see if i am able to do some coding that works or not. i agree i should have used meaningfull variable naming. my animation function is too naieve(i know!!). i thought if i could animate one div based on another div click..the next step would b to make a custom accordion.(which i tried to use in C# and it does not work very well.)...any way thanks for ur comments and the revised code....