2
\$\begingroup\$

I have a handful of images working as jQuery toggle switches - Showing or hiding an element on a click event.

The code below mostly works as intended, but I'm fairly certain I'm using a lot of unnecessary stuff to make it happen. Any suggestions on streamlining?

Also, any suggestions on closing (toggle to hide) any visible elements that had already been toggled to visible? As it works now, if the user clicks on the first image and then the second image, it stacks the newly visible elements on top of each other. Ideally, I'd like to close (hide) the element from the first toggle when the second toggle is clicked and/or close any open elements on a scroll event.

The code below is abridged. There's actually 10+ toggles which can get a little unwieldy if the user has opened a bunch of them.

$("#show-wine").click(function(){  
  $(".wine-text").toggle();
});
$("#show-wine2").click(function(){  
  $(".wine-text2").toggle();
});
$("#show-wine3").click(function(){ 
  $(".wine-text3").toggle();
});
.wine-text, .wine-text2, .wine-text3, .wine-text4, .wine-text5, .wine-text6, .wine-text7, .wine-text8, .wine-text9 {
  display:none;
  position:fixed;
  top:3em;
  left:1.5em;
  width:35em;
  padding:1em;
  height:auto;
  min-height:30em;
  background: rgba(0,0,0,0.95);
  color:#FFF;
  border-radius:4px;
  line-height:1.25em;
  text-align:left;
}
.wine-tiles li img:hover {
  cursor:pointer;
  opacity:.7;
}
.wine-tiles li img .unhide:hover .wine-text {
  display:block;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<div id="column1">  
  <ul class="wine-tiles">									
    <li>
      <img src="images/labels/image.jpg" id="show-wine" class="unhide bounce" alt="Img 1"/>									
      <span class="wine-text">
        <span class="wine-text-title centered">Wine#1</span><br>
        <span class="wine-text-winery centered">Big White House</span><br><br>
        <span class="wine-text-copy">Lorem Ipsum</span>
        <br/><br/>
        <button id="hide-wine" class="button">Close</button>
      </span>	
      <h2>Title</h2>
    </li>
    <li>
      <img src="images/labels/image.jpg" id="show-wine2" class="unhide bounce" alt="Img 2"/>									
      <span class="wine-text2">
        <span class="wine-text-title centered">Wine#2</span><br>
        <span class="wine-text-winery centered">Big White House</span><br><br>
        <span class="wine-text-copy">Lorem Ipsum</span>
        <br/><br/>
        <button id="hide-wine" class="button">Close</button>
      </span>	
      <h2>Title</h2>
    </li>
    <li>
      <img src="images/labels/image.jpg" id="show-wine3" class="unhide bounce" alt="Img 3"/>									
      <span class="wine-text3">
        <span class="wine-text-title centered">Wine#3</span><br>
        <span class="wine-text-winery centered">Big White House</span><br><br>
        <span class="wine-text-copy">Lorem Ipsum</span>
        <br/><br/>
        <button id="hide-wine" class="button">Close</button>
      </span>	
      <h2>Title</h2>
    </li>
  </ul>
</div>
<div id="column2">
  <ul class="wine-tiles">									
    <li>
      <img src="images/labels/image.jpg" id="show-wine" class="unhide bounce" alt="Img 1"/>									
      <span class="wine-text">
      <span class="wine-text-title centered">Wine#4</span><br>
      <span class="wine-text-winery centered">Big White House</span><br><br>
      <span class="wine-text-copy">Lorem Ipsum</span>
      <br/><br/>
      <button id="hide-wine" class="button">Close</button>
    </span>	
    <h2>Title</h2>
  </li>
  <li>
    <img src="images/labels/image.jpg" id="show-wine2" class="unhide bounce" alt="Img 2"/>									
    <span class="wine-text2">
      <span class="wine-text-title centered">Wine#5</span><br>
      <span class="wine-text-winery centered">Big White House</span><br><br>
      <span class="wine-text-copy">Lorem Ipsum</span>
      <br/><br/>
      <button id="hide-wine" class="button">Close</button>
    </span>	
    <h2>Title</h2>
    </li>
    <li>
      <img src="images/labels/image.jpg" id="show-wine3" class="unhide bounce" alt="Img 3"/>									
      <span class="wine-text3">
        <span class="wine-text-title centered">Wine#6</span><br>
        <span class="wine-text-winery centered">Big White House</span><br><br>
        <span class="wine-text-copy">Lorem Ipsum</span>
        <br/><br/>
        <button id="hide-wine" class="button">Close</button>
      </span>	
      <h2>Title</h2>
    </li>
  </ul>
</div>

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

The code below mostly works as intended, ...

Does it really? The HTML has some errors:

  • h2 tag incorrectly closed with </p>, for example: <h2>Title</p>
  • Duplicated id="hide-wine"

It's good to validate your HTML on http://validator.w3.org/check. When your HTML is invalid, you can have nasty bugs and inexplicable behavior.

You're using both <br> and <br/>, which are both correct in HTML5, but for the sake of consistency I recommend to pick a writing style and stick to it.

Any suggestions on streamlining?

If you mean reducing the duplicated HTML code, you will need to use a framework that allows loops.

I'm fairly certain I'm using a lot of unnecessary stuff to make it happen.

The duplicated JavaScript is easier to fix than the HTML. Instead of this:

  $("#show-wine").click(function(){  
    $(".wine-text").toggle();
  });
  $("#show-wine2").click(function(){  
    $(".wine-text2").toggle();
  });
  $("#show-wine3").click(function(){ 
    $(".wine-text3").toggle();
  });

You could:

  • add a common show-wine class for all the #show-wine*
  • and a common wine-text class for all the .wine-text*
  • rely on the fact that the show-wine-n toggles and wine-text-n have a common parent

This would allow to simplify the above code to:

  $(".show-wine").click(function() {
    $(this).parent().find(".wine-text").toggle();
  });
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Unless this is XHTML (Transitional or Strict), <br> is perfectly valid. If this is HTML5, then both <br> and <br /> are valid. \$\endgroup\$
    – cimmanon
    Sep 25, 2014 at 13:42

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.