6
\$\begingroup\$

Is there a way to improve the Javascript that I wrote in this code? I made a dropdown box that toggles between showing and hiding the textbox. I then realize that the Jquery/Javscript code I was writing began to repeat itself.

Just want to know if there is a way to simplify the Javascript/JQuery that I wrote.

Is there a way to improve the Javascript that I wrote in this code? I made a dropdown box that toggles between showing and hiding the textbox. I then realize that the Jquery/Javscript code I was writing began to repeat itself.

Just want to know if there is a way to simplify the Javascript/JQuery that I wrote.

$(document).ready(function() {
  $("body").css("background-color", "yellow");

  $("#dropdown1").on('click', function() {
    $('#menu1').slideToggle();
    var arrow = $("div.arrow");
    arrow.toggleClass("right");
    arrow.toggleClass("down");
  });
  $("#dropdown2").on('click', function() {
    $('#menu2').slideToggle();
    var arrow = $("div.arrow2");
    arrow.toggleClass("right2");
    arrow.toggleClass("down2");
  });
});
body {
  background-color: #333;
}

.wrapper {
  max-width: 1024px;
  display: block;
  margin: 0 auto;
  background-color: #ffffff;
  padding: 5px 20px;
}

div.container {
  max-width: 90%;
}

.arrow,
.arrow2,
.arrow3 {
  /*    float: left;*/
  display: inline-block;
  border: solid black;
  border-width: 0 3px 3px 0;
  padding: 4px 4px 4px 4px;
  margin: 0 0 0 10px;
}

.down,
.down2 {
  transform: rotate(45deg);
  -webkit-transform: rotate(45deg);
  margin: 0 0 2px 10px;
}

.up {
  transform: rotate(-135deg);
  -webkit-transform: rotate(-135deg);
  margin: 0 0 -4px 10px;
}

.right,
.right2,
.right3 {
  transform: rotate(-45deg);
  -webkit-transform: rotate(-45deg);
  margin: 0 0 0px 10px;
}

#menu1,
#menu2,
#menu3 {
  display: none;
}

.title {
  padding: 5px 0 5px 10px;
  display: inline-block;
}
<!DOCTYPE html>
<html>

<head>
  <link href="style.css" rel="stylesheet" type="text/css">
  <title></title>
</head>

<body>
  <main class="wrapper">
    <div class="container">
      <div id="dropdown1">
        <div class="arrow right"></div>
        <h3 class="title">Dropdown 1</h3>
        <div id="menu1">
          <p>Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute
            irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</p>
        </div>
      </div>
      <div id="dropdown2">
        <div class="arrow2 right2"></div>
        <h3 class="title">Dropdown 2</h3>
        <div id="menu2">
          <p>Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute
            irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</p>
        </div>
      </div>
    </div>
  </main>
  <script src="https://ajax.googleapis.com/ajax/libs/jquery/3.2.1/jquery.min.js"></script>
  <script type="text/javascript" src="script.js"></script>
</body>

</html>

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Well, you certainly have repeated yourself in this question. =) \$\endgroup\$
    – 200_success
    Commented Jul 18, 2017 at 18:48

1 Answer 1

Reset to default
1
\$\begingroup\$

In this case the answer is very straight-forward if you want to keep things in plain ol' jQuery. Instead of selecting on ids, use classes and rely upon a well structured dom (which you already have):

$(document).ready(function() {
  $("body").css("background-color", "yellow");

  $(".dropdown_with_toggle").on('click', function() {
    var el = $(this);
    $('.menu', el).slideToggle();
    var arrow = $(".arrow", el);
    arrow.toggleClass("right");
    arrow.toggleClass("down");
  });
});

Then adjust your HTML slightly:

<!DOCTYPE html>
<html>

<head>
  <link href="style.css" rel="stylesheet" type="text/css">
  <title></title>
</head>

<body>
  <main class="wrapper">
    <div class="container">
      <div class="dropdown_with_toggle">
        <div class="arrow right"></div>
        <h3 class="title">Dropdown 1</h3>
        <div class="menu">
          <p>Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute
            irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</p>
        </div>
      </div>
      <div class="dropdown_with_target">
        <div class="arrow right"></div>
        <h3 class="title">Dropdown 2</h3>
        <div class="menu">
          <p>Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute
            irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</p>
        </div>
      </div>
    </div>
  </main>
  <script src="https://ajax.googleapis.com/ajax/libs/jquery/3.2.1/jquery.min.js"></script>
  <script type="text/javascript" src="script.js"></script>
</body>

</html>

They key is that the class dropdown_with_toggle is not being used for styling but rather to guide your javascript. You listen for click events on all elements with that class, and then you take action not by finding elements by id, but by finding children of your main element by class. That is what the $('.menu', el) syntax does: it finds the selector but only looks for descendants of a given element. It's effectively just an alternate syntax for the jQuery .find() method, which would work equally well here.

Just because: the next step is to not attach your listeners directly to the elements but instead use deferred listeners on a mutual parent:

$('.container').on( 'click', '.dropdown_with_toggle', function(){ } )

The rest of your code doesn't change, but the advantage this notation has is that if you add or remove elements dynamically, you don't have to re-register any listeners (as long as you are only modifying elements inside your container div, and the container div isn't removed). I used to use this trick often with dynamic pages that are loaded/reloaded via ajax.

\$\endgroup\$
1
  • \$\begingroup\$ Thank you. I'm new to Javascript and this helped me out a lot. \$\endgroup\$
    – DJ Chris Fusion
    Commented Jul 18, 2017 at 18:44

Your Answer

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

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