Skip to main content
5 of 5
added comments to the code
kosmos
  • 156
  • 2
  • 5

If you see a little your code, you will see that there are excessive repeated selectors, which (in most cases) means something goes wrong. Thinking how to improve it, finally I rewrote the entire code. The result (I hope it works):

$(function(){
    var targets = ['background', 'colour', 'description', 'logo', 'products', 'links', 'contact']; // containers
    var $elements = $('#background, #colourscheme, #description, #logo, #products, #links, #contact'); // jQuery elements
    var i, x; // we use `i` for the array and `x` for the event selector
    for( i = 0, x = 1; i < targets.length; i++ ){
        $('#' + x + '_open').on('click', function(){
            $('#' + targets[i] + '_container').slideToggle((500, 'easeInOutExpo'), function(){
                var visible; // will store the name of the method (show/hide)
                if( !$(this).is(':visible') ){
                    $('#' + x + '_arrow').attr('src', './resources/images/icons/right.png');
                    visible = 'show';
                }
                else{
                    $('#' + x + '_arrow').attr('src', './resources/images/icons/left.png');
                    visible = 'hide';
                }
                switch( x ){ // since the code is (almost) the same to all elements, we finally use the method which name was stored in `visible` variable
                    case 1: visible == 'show' ? $elements.show() : $elements.not('#background').hide(); break; // this case is the only one that missmatch, so we use directly the method to show/hide
                    case 2: $elements.not('#colourscheme')[visible](); break; // the rest of cases had the same syntax so it is easy to solve
                    case 3: $elements.not('#description')[visible](); break;
                    case 4: $elements.not('#logo')[visible](); break;
                    case 5: $elements.not('#products')[visible](); break;
                    case 6: $elements.not('#contact')[visible](); break;
                    case 7: $elements.not('#links')[visible](); break;
                }
            });
        });
        x++;
    }
});

The first thing was to assign the names of the targets containers into an array and the elements into a jQuery object. Since we are working with an "ordered" identifiers (#1_open, #2_open...) we can assign the onclick event with a loop easily and work into it.

Since the code is almost the same, we just need to identify our current target x and proceed at convenience. There are more ways to do it, but well, this is just another one.

kosmos
  • 156
  • 2
  • 5