2
\$\begingroup\$

I have this code currently written to help me create a menu. I was just wondering if there is a way to tidy this so that there is less code needed to achieve the same thing.

The code is designed to create large dropdowns which have divs inside which will contain most of the content. I want to hide parts of the menu so that the page doesn't become crowded so Its set to hide the other dropdown containers.

$(document).ready(function() {

  $("#1_open").click(function() {
    $("#background_container").slideToggle((500, 'easeInOutExpo'), function() {
      var background = $(this).css('display');
      if (background == 'none') {
        $('#1_arrow').attr('src', './resources/images/icons/right.png');
        $('#background, #colourscheme , #description , #logo , #products , #links , #contact').css('display', 'block');
      } else {
        $('#1_arrow').attr('src', './resources/images/icons/left.png');
        $('#colourscheme , #description , #logo , #products , #links , #contact').css('display', 'none');
      }
    });
  });
  $("#2_open").click(function() {
    $("#colour_container").slideToggle((500, 'easeInOutExpo'), function() {
      var colour = $(this).css('display');
      if (colour == 'none') {
        $('#2_arrow').attr('src', './resources/images/icons/right.png');
        $('#background, #description , #logo , #products , #links , #contact').css('display', 'block');
      } else {
        $('#2_arrow').attr('src', './resources/images/icons/left.png');
        $('#background, #description , #logo , #products , #links , #contact').css('display', 'none');
      }
    });
  });
  $("#3_open").click(function() {
    $("#description_container").slideToggle((500, 'easeInOutExpo'), function() {
      var description = $(this).css('display');
      if (description == 'none') {
        $('#3_arrow').attr('src', './resources/images/icons/right.png');
        $('#background, #colourscheme , #logo , #products , #links , #contact').css('display', 'block');
      } else {
        $('#3_arrow').attr('src', './resources/images/icons/left.png');
        $('#background, #colourscheme , #logo , #products , #links , #contact').css('display', 'none');
      }
    });
  });
  $("#4_open").click(function() {
    $("#logo_container").slideToggle((500, 'easeInOutExpo'), function() {
      var logo = $(this).css('display');
      if (logo == 'none') {
        $('#4_arrow').attr('src', './resources/images/icons/right.png');
        $('#background, #colourscheme , #description , #products , #links , #contact').css('display', 'block');
      } else {
        $('#4_arrow').attr('src', './resources/images/icons/left.png');
        $('#background, #colourscheme , #description , #products , #links , #contact').css('display', 'none');
      }
    });
  });
  $("#5_open").click(function() {
    $("#products_container").slideToggle((500, 'easeInOutExpo'), function() {
      var products = $(this).css('display');
      if (products == 'none') {
        $('#5_arrow').attr('src', './resources/images/icons/right.png');
        $('#background, #colourscheme , #description , #logo , #links , #contact').css('display', 'block');
      } else {
        $('#5_arrow').attr('src', './resources/images/icons/left.png');
        $('#background, #colourscheme , #description , #logo , #links , #contact').css('display', 'none');
      }
    });
  });
  $("#6_open").click(function() {
    $("#contact_container").slideToggle((500, 'easeInOutExpo'), function() {
      var contact = $(this).css('display');
      if (contact == 'none') {
        $('#6_arrow').attr('src', './resources/images/icons/right.png');
        $('#background, #colourscheme , #description , #logo , #products , #links').css('display', 'block');
      } else {
        $('#6_arrow').attr('src', './resources/images/icons/left.png');
        $('#background, #colourscheme , #description , #logo , #products , #links').css('display', 'none');
      }
    });
  });
  $("#7_open").click(function() {
    $("#links_container").slideToggle((500, 'easeInOutExpo'), function() {
      var links = $(this).css('display');
      if (links == 'none') {
        $('#7_arrow').attr('src', './resources/images/icons/right.png');
        $('#background, #colourscheme , #description , #logo , #products , #contact').css('display', 'block');
      } else {
        $('#7_arrow').attr('src', './resources/images/icons/left.png');
        $('#background, #colourscheme , #description , #logo , #products , #contact').css('display', 'none');
      }
    });
  });


});
<div id="background">
  <div class="header" id="1_open">
    <img src="./resources/images/icons/right.png" class="arrow" id="1_arrow">Background</div>
  <div id="background_container" class="containers" style="display:none;"></div>
</div>

<div id="colourscheme">
  <div class="header" id="2_open">
    <img src="./resources/images/icons/right.png" class="arrow" id="2_arrow">Colour Scheme</div>
  <div id="colour_container" class="containers" style="display:none;"></div>
</div>
<div id="description">
  <div class="header" id="3_open">
    <img src="./resources/images/icons/right.png" class="arrow" id="3_arrow">Description</div>
  <div id="description_container" class="containers" style="display:none;"></div>
</div>
<div id="logo">
  <div class="header" id="4_open">
    <img src="./resources/images/icons/right.png" class="arrow" id="4_arrow">Logo and Icon</div>
  <div id="logo_container" class="containers" style="display:none;"></div>
</div>

<div id="products">
  <div class="header" id="5_open">
    <img src="./resources/images/icons/right.png" class="arrow" id="5_arrow">Products</div>
  <div id="products_container" class="containers" style="display:none;"></div>
</div>

<div id="contact">
  <div class="header" id="6_open">
    <img src="./resources/images/icons/right.png" class="arrow" id="6_arrow">Contact Information</div>
  <div id="contact_container" class="containers" style="display:none;"></div>
</div>

<div id="links">
  <div class="header" id="7_open">
    <img src="./resources/images/icons/right.png" class="arrow" id="7_arrow">Links</div>
  <div id="links_container" class="containers" style="display:none;"></div>
</div>

\$\endgroup\$
1
  • \$\begingroup\$ have you tried using css? \$\endgroup\$
    – kainlite
    Commented Jun 22, 2015 at 0:36

2 Answers 2

2
\$\begingroup\$

You are repeating yourself too much. Not DRY at all. Read about Dry Principals

This snippet here could easily be extracted into a function, that then could be reused everywhere.

var background = $(this).css('display');
  if (background == 'none') {
    $('#1_arrow').attr('src', './resources/images/icons/right.png');
    $('#background, #colourscheme , #description , #logo , #products , #links , #contact').css('display', 'block');
  } else {
    $('#1_arrow').attr('src', './resources/images/icons/left.png');
    $('#colourscheme , #description , #logo , #products , #links , #contact').css('display', 'none');
  }

You could create a function that would take as parameters everything that differ from each usage. Example-

function doStuff(selector, image1, image2, otherParam) {
    var background = $(this).css('display');
  if (background == 'none') {
    $(selector).attr('src', image1);
    $('#background, #colourscheme , #description , #logo , #products , #links , #contact').css('display', 'block');
  } else {
    $(selector).attr('src', image2);
    $('#colourscheme , #description , #logo , #products , #links , #contact').css('display', 'none');
  }
}
\$\endgroup\$
0
\$\begingroup\$

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.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.