Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I've written a simple accordion using zepto js lib.

Looking for advice on how to improve this better.

HTML

<div class="box">
   <span class="learn-more"><a href="#">Learn more</a></span>
   <div class="more">
     blah<br>
     blah<br>
     blah<br>
     <span class="close"><a href="#">close</a></span>
   </div>
</div>
<div class="box">
   <span class="learn-more"><a href="#">Learn more</a></span>
   <div class="more">
     blah<br>
     blah<br>
     blah<br>
     <span class="close"><a href="#">close</a></span>
   </div>
</div>

Javascript

$('.more').addClass('hide');
var learnMore = $('.learn-more'),
    close = $('.close');
learnMore.click(function() {
    $(this).hide().next('div').toggleClass('hide');
});
close.click(function() {
    $(this).parent().toggleClass('hide');
    $(this).parent().prev().show();
});

Working demo

http://jsfiddle.net/s5x9A/

How can I prevent the page from jumping when I click on the empty anchor tags?

share|improve this question
2  
You last question is more of a javascript question that belongs on StackOverflow right! I don't see a problem with you code. But you never know with edge cases. –  andho Nov 29 '12 at 3:59
1  
But the simple answer is you prevent the default click event. $('.box .learn-more a, .box .close a').click(function(e){e.preventDefault();}); –  andho Nov 29 '12 at 4:01

1 Answer 1

There are a couple of inconsistencies which you could tidy up. You're toggling classes to show/hide the div but using show/hide directly on the triggers. You're using chaining in one onClick but not in the other. And you're using the class to reference the accordion contents in one case, but the element name in the other.

$('.more').hide();

var learnMore = $('.learn-more'),
    close = $('.close');

learnMore.click(function() {
   $(this).hide().next('.more').show();
});

close.click(function() {
    $(this).parent().hide().prev('.learn-more').show();
});

Fiddle

is one possible way of making everything consistent, although you may prefer to always toggle classes instead (or to add animations).

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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