2
\$\begingroup\$

I have the following code which switches between the states of hide() and show() of show prices button.I am aware about .toggle() in jQuery.But I couldn't find a workaround of it in this instance. When I click the button the <p></p> element is appended to .msg class in the div and when I click it once again it hides it state by removing the <p></p> element from the DOM (which is in the else block).

I have managed to come up with a solution which works well using If..else, yet it doesn't feel right or the best way to do it. I find this solution naive and want to know what I can do to further optimize this code.

Following is the code:

(function() {
  //$('.not-interested').hide();
  $('.msg').hide();

  var showPrice = function(e) {
    e.stopPropagation();

    var vacation = $(this).closest('.vacation');
    var button = vacation.find('button');
    if ((!vacation.hasClass('present'))) {
      var price = +vacation.data('price');
      var vacation_place = vacation.find('h3').text();
      var msg = $("<p style='text-align:center;line-height:24px;font-size:13px;'>Price for " + vacation_place + " is: " + (3 * price) + " </p>");
      vacation.find('.msg').prepend(msg).show();
      vacation.addClass('present');
    } else if ((e.type == 'click' && e.toElement.localName == 'button') || e.type == 'showAll') {

      //console.log(e.toElement.localName);
      //vacation.on('click','button')){
      //console.log(e.toElement);
      //console.log(vacation.on('click','button').context);
      //console.log(button);
      vacation.removeClass('present');
      vacation.find('.msg').hide().find('p').remove();
    }

  };

  var removePrice = function(e) {
    e.stopPropagation();
    var vacation = $(this).closest('.vacation');
    vacation.find('div.msg').hide();
    //vacation.on('click.price','button',showPrice);
  };

  $('.vacation').on('click.show', 'button', showPrice);
  $('.vacation').on('showAll.price', showPrice); // creating a custom event

  $('.show-all-price').on('click', function(e) {
    e.preventDefault();
    $('.vacation').trigger('showAll.price'); // firing a custom event on click of an anchor
  });
})();
body,
ul {
  font-size: 100%;
  margin: 0;
  padding: 0;
  font-family: "sans-serif";
  color: #fff;
}
.show-all {
  width: 100px;
  background: #597C80;
  margin-top: 25px;
  margin-left: 25px;
  border: 1px solid #2A3F41;
}
.show-all a {
  display: block;
  text-decoration: none;
  color: #333;
  text-align: center;
  padding: 10px;
  font-size: 13px;
}
ul {
  list-style: none;
  margin-top: 20px;
  margin-left: 20px;
  float: left;
}
li {
  float: left;
  display: block;
  padding: 10px;
  background: #2A3F41;
  margin-right: 10px;
  padding-bottom: 25px;
}
li h3 {
  text-align: center;
}
li > div {
  width: 80%;
  margin: 0 auto;
}
li button {
  width: 100%;
  background: #377C37;
  color: #333;
  border: none;
  outline: 0;
  cursor: pointer;
  padding: 5px 9px;
}
button:active {
  position: relative;
  top: 2px;
  /* padding: 8px 13px 6px;*/
}
li a {
  display: block;
  margin-top: 10px;
  text-align: center;
  text-decoration: none;
  color: #597C80;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.0.2/jquery.min.js"></script>

<body>
  <div class="show-all">
    <a href="#" class="show-all-price">Show all</a>
  </div>
  <ul id="vacation-list">
    <li class="vacation" data-price="395">
      <h3>Hawaiian Vacation</h3>
      <div>
        <button>Show all prices</button>
        <div class="msg">
          <a href="#" class="not-interested">Not Interested</a>
        </div>
      </div>

    </li>
    <li class="vacation" data-price="315">
      <h3>American Vacation</h3>
      <div>
        <button>Show all prices</button>
        <div class="msg">
          <a href="#" class="not-interested">Not Interested</a>
        </div>
      </div>

    </li>
    <li class="vacation" data-price="420">
      <h3>French Vacation</h3>
      <div>
        <button>Show all prices</button>
        <div class="msg">
          <a href="#" class="not-interested">Not Interested</a>
        </div>
      </div>

    </li>
  </ul>
  <script type="text/javascript" src="js/jquery.min.js"></script>
  <script src="js/app.js"></script>
</body>

I've also tried using .on() and .off() with name spacing but couldn't figured how to do it appropriately.

Here's the code with .on() and .off() switching:

(function(){
//$('.not-interested').hide();
$('.msg').hide();

var showPrice = function(e){
    e.stopPropagation();

    var vacation = $(this).closest('.vacation');
    var button = vacation.find('button');

    var price = +vacation.data('price');
    var vacation_place = vacation.find('h3').text();
    var msg = $("<p style='text-align:center;line-height:24px;font-size:13px;'>Price for " + vacation_place + " is: " + (3 * price) + " </p>");
    vacation.find('.msg').prepend(msg).show();

    vacation.on('click.remove','button',removePrice);


};

   var removePrice = function(e){
     e.stopPropagation();
     var vacation = $(this).closest('.vacation');
     vacation.find('div.msg').hide().find('p').remove();
     vacation.on('click.price','button',showPrice);
   };

   $('.vacation').on('click.price','button',showPrice);
   $('.vacation').on('showAll.price',showPrice); // creating a custom event

   $('.show-all-price').on('click',function(e){
     e.preventDefault();
     $('.vacation').trigger('showAll.price'); // firing a custom event on click of an anchor
   });
})();

Note: ,e.toElement.localName == 'button' doesn't work in IE.

\$\endgroup\$
  • 3
    \$\begingroup\$ Welcome to Code Review! Please declare your cross-posts. \$\endgroup\$ – 200_success Dec 24 '15 at 7:02
  • \$\begingroup\$ Clicking on "Show all" toggles all of the displays, so that messages that were shown become hidden, and messages that were hidden become shown. That's not the intended behaviour, is it? \$\endgroup\$ – 200_success Dec 24 '15 at 7:11
  • \$\begingroup\$ Suggest you hard-code the messages, and simply toggle them. \$\endgroup\$ – Roamer-1888 Dec 24 '15 at 9:45
  • \$\begingroup\$ @200_success that's the intended behaviour and it works fine until few vacations are shown and then when "show all" is clicked it hides down the shown and shows the hidden which is not intended. Also I want to re-factor this code as I don't think this is the best way to do.. \$\endgroup\$ – Mustafa Ujjainwala Dec 24 '15 at 10:11

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.