jQuery accordion assistance

Question

I have come across this code that I have tried to add to but I am afraid that I have made of mess of it. I am not very experienced with Jquery or web design and I need help finding what I have done wrong and how I can be more efficient.

Is there a better/more efficient way to write this code?

It has some bugs...

• Displays all divs upon load. I would like it to just show one that I can select.
  
• When it animates the text jumps around for some reason.
  
• Sometimes the divs will animate up but not down.

Here is a link to what it looks like right now: http://dl.dropbox.com/u/14080718/Final/NeedHelp.html

    <script>
    $(document).ready(function() {
        // the currently loaded section
        var curLoaded = 'about';

        // navigation trigger
        $('#navbar a').each(function() {
            var $this = $(this)
            var target = $this.attr('href').split('#')[1];
            var $contentContainer = $('#contentContainer');
            var oldPos = 0;
            var newPos = 200;

            // add a click handler to each A tag
            $this.click(function(){
                // if the container isn't open, then open it
                if ($contentContainer.css('height') == '') {
                    // trigger the animation
                    $contentContainer.animate({
                        height: newPos
                    },"slow", function(){
                        // fade in the content
                        $('#' + target).fadeIn();
                    });
                } else {
                    if (curLoaded == target) {
                        $contentContainer.animate({
                            height: oldPos
                        },"slow", function(){
                            $('#content div').hide();
                        });
                    } else {
                        $contentContainer.animate({
                            height: oldPos
                        },"slow", function(){
                            $('#content div').hide();
                            $contentContainer.animate({
                                height: newPos
                            },"slow", function(){
                                $('#' + target).fadeIn();
                            });
                        });
                    }
                }
                curLoaded = target;

                return false;
            });
        });
    });
    </script>

Answer 1

I've taken a look at your page and took a quick stab at addressing some of the buggy behaviors.

For one thing, it looks like you were handling the case where the user clicks a nav link for a second time, by sliding the content div closed, but you were not handling if the user clicks the nav link a third time. As a user, I would expect that the div would slide open or closed no matter how many times I clicked the link.

In order to fix that, I've added a check for whether or not the target element is visible, and I use that to decide whether to slide the content open or closed. I've also added calls to the show and hide functions for the target and curloaded elements to ensure that the check for whether the element is visible will provide the expected result.

My revised code is below. It's possible that the logic for some of the cases could be consolidated in order to simplify the code.

// navigation trigger 
$('#navbar a').each(function() {
    var $this = $(this);
    var target = $this.attr('href').split('#')[1];
    var $contentContainer = $('#contentContainer');
    var oldPos = 0;
    var newPos = 200;

    // add a click handler to each A tag 
    $this.click(function() {
        // if the container isn't open, then open it 
        if ($contentContainer.css('height') === '') {
            // trigger the animation 
            $contentContainer.animate({
                    height: newPos
                }, "slow", function() {
                    // fade in the content 
                    $('#' + target).show().fadeIn();
                });
        } else {
            if (curLoaded == target) {
                if ($('#' + target).is(':visible')) {
                    $contentContainer.animate({
                            height: oldPos
                        }, "slow", function() {
                            $('#' + curLoaded).hide();
                            $('#content div').hide();
                        });
                } else {
                    $contentContainer.animate({
                            height: newPos
                        }, "slow", function() {
                            // fade in the content 
                            $('#' + target).show().fadeIn();
                        });
                }
            } else {
                $contentContainer.animate({
                        height: oldPos
                    }, "slow", function() {
                        $('#' + curLoaded).hide();
                        $('#content div').hide();
                        $contentContainer.animate({
                                height: newPos
                            }, "slow", function() {
                                $('#' + target).fadeIn();
                            });
                    });
            }
        }

        curLoaded = target;

        return false;
    });

});

Answer 2

The css on this fiddler will help with all the divs initially showing up. Can you explain the other 2 errors in more detail (I don't seem to notice them)

Fiddler Code