0

I am working on a five star rating system. There are five star icons. When an icon is clicked, I want to change the value of an input to whichever star is clicked(e.g. if the fourth star is clicked, the input will have a value of 4). My problem is that the click method that I apply to the newly created icon does not work at all. What am I doing wrong? Here is a jsFiddle to demonstrate.

html

<div class="rating" style="float: none;clear: both;">
  <noscript>
    <label class="radio-inline">
      <input type="radio" name="stars" value="1" title="1 Star"> 1
    </label>
    <label class="radio-inline">
      <input type="radio" name="stars" value="2" title="2 Stars"> 2
    </label>
    <label class="radio-inline">
      <input type="radio" name="stars" value="3" title="3 Stars"> 3
    </label>
    <label class="radio-inline">
      <input type="radio" name="stars" value="4" title="4 Stars"> 4
    </label>
    <label class="radio-inline">
      <input type="radio" name="stars" value="5" title="5 Stars"> 5
    </label>
  </noscript>
</div>
<input type="text" name="stars" value="" id="stars">

Javascript

 $element = $('.rating');
  $element.empty();
  for (var i = 0; i < 5; i++) {
    var occurrence = i + 1;
    var newStar = $('<i class="fa fa-star-o" title="' + occurrence + ' stars" data-occurrence="' + occurrence + '"></i>');
    newStar.on('click', function() {
      $('#stars').val(occurrence + ' stars given.');
    });
    $element.append(newStar);
  }
  $element.each(function() {
    var _parent = $(this);
    var originalStars = _parent.html();
    $(_parent).on('mouseover', 'i[class*="fa-star"]', function() {
      var starOccurrence = $(this).prevAll().andSelf().length;
      $(this).prevAll().andSelf().removeClass('fa-star-o').addClass('fa-star');
    }).mouseout(function() {
      $(_parent).html(originalStars);
    });
  });
Roamer-1888
  • 19,138
  • 5
  • 33
  • 44
ShoeLace1291
  • 4,551
  • 12
  • 45
  • 81
  • 4
    Replacing the `innerHTML` gets rid of the internal state of all elements, including event listeners. Don't do that. – Oriol Feb 22 '16 at 23:51
  • ^ that, you keep getting the HTML as a string, and reinserting it, loosing all the event handlers and associated data for each element. – adeneo Feb 22 '16 at 23:53
  • So how else can I add elements without removing the event listeners? – ShoeLace1291 Feb 22 '16 at 23:54
  • 1
    Possible duplicate of [Direct vs. Delegated - jQuery .on()](http://stackoverflow.com/questions/8110934/direct-vs-delegated-jquery-on) – freedomn-m Feb 23 '16 at 00:01
  • Use event delegation - there's 100s of examples on SO. – freedomn-m Feb 23 '16 at 00:01
  • use event delegation if you must replace the html...although there are simple enough ways to do this with class changes and not replacing html – charlietfl Feb 23 '16 at 00:02

1 Answers1

0

2 problems. Mentioned in the comments, don't replace the inner html, just remove/add classes to reset the stars.

.mouseout(function() {
    $('i').removeClass('fa-star').addClass('fa-star-o');
  });

Second problem is you need to grab the occurrence value from the data attribute, and not the overwritten occurrence variable.

$('#stars').val($(this).data('occurrence') + ' stars given.');

jsFiddle: https://jsfiddle.net/9mLzLsw7/2/

Eric Guan
  • 15,474
  • 8
  • 50
  • 61
  • The first problem isn't an actual problem. The problem is within the for loop. The elements have to be created first before those removeClass and addClass can work. – ShoeLace1291 Feb 23 '16 at 00:04
  • You are creating the elements though, and the class adding/removing works in my fiddle. – Eric Guan Feb 23 '16 at 00:07
  • Ok, now I see what you are saying. And the issue wasn't the delegation at all like the others said. Thanks for your help! – ShoeLace1291 Feb 23 '16 at 00:13
  • Going a bit further, you can (1) avoid the need for `data-occurrence` by exploiting jQuery's `.index()`, (2) delegate 'click' handing to the `.rating` container, (3) append the `...` string directly, without first wrapping in jQuery. I would write something like [this](https://jsfiddle.net/9mLzLsw7/5/). – Roamer-1888 Feb 23 '16 at 11:28