2
\$\begingroup\$

This seems a little bit excessive, and was wondering if I can refactor this jQuery. I'm not sure if you need anymore information, or if it's possible just by looking at this script.

  var getActiveState;
  $(".modal-review__rating-order-wrap span").hover(function(){
    $(this).parent().removeClass("is-active");
    var thisStar = $(this).parent().find("[data-rating-value="+getActiveState+"]")
    thisStar.removeClass("is-active");
  }, function(){
    var thisStar = $(this).parent().find("[data-rating-value="+getActiveState+"]")
    if(getActiveState){
      $(this).parent().addClass("is-active");
      thisStar.addClass("is-active");
    }
  });
  $(".modal-review__rating-order-wrap span").on("click",function(){
    $(this).parent().addClass("is-active");
    $(".modal-review__rating-order-wrap span").not(this).removeClass("is-active");
    $(this).addClass("is-active");
    getActiveState = $(this).data("rating-value");
    return getActiveState;
  });

This is applied to a 5 star system, wanted to show what the jQuery is affecting. When jQuery clicks on the star, it adds an .is-active class to the span, and also to the wrap.

<div class="modal-review__rating-order-wrap">
  <span class="icon-star" data-rating-value="1">
    <svg>star 1</svg>
  </span>
  <span class="icon-star" data-rating-value="2">
    <svg>star 2</svg>
  </span>
  <span class="icon-star" data-rating-value="3">
    <svg>star 3</svg>
  </span>
  <span class="icon-star" data-rating-value="4">
    <svg>star 4</svg>
  </span>
  <span class="icon-star" data-rating-value="5">
    <svg>star 5</svg>
  </span>
</div>
\$\endgroup\$
2
\$\begingroup\$

I couldn't see the Unicode stars in 200_success's answer on Chrome, so I made a version of their snippet with SVG stars from an old meta.SO post of mine:

$('.modal-review__rating-order-wrap > span').click(function() {
    $(this).addClass('active').siblings().removeClass('active');
    $(this).parent().attr('data-rating-value', $(this).data('rating-value'));
});
/* Initial state */
div.modal-review__rating-order-wrap > span {
    display: block; float: left;
    height: 30px; width: 40px;
    background-image: url("data:image/svg+xml,%3Csvg%20xmlns='http://www.w3.org/2000/svg'%20width='80'%20height='30'%3E%3Cpath%20d='M17.5,12.5h-8.5l6.8,5-2.6,8.1,6.8-5,6.8,5-2.6-8.1,6.8-5h-8.5l-2.6-8.1z'%20fill='%23c0c0c0'%20stroke='%23c0c0c0'/%3E%3Cpath%20d='M57.5,12.5h-8.5l6.8,5-2.6,8.1,6.8-5,6.8,5-2.6-8.1,6.8-5h-8.5l-2.6-8.1z'%20fill='%23ffd83d'%20stroke='%23eac328'/%3E%3C/svg%3E");
    background-position: 0px 0px;    /* gray star */
}

/* Persistent state */
div.modal-review__rating-order-wrap[data-rating-value] > span {
    background-position: -40px 0px;  /* gold star */
}
div.modal-review__rating-order-wrap > span.active ~ span {
    background-position: 0px 0px;    /* gray star */
}

/* Hover state */
div.modal-review__rating-order-wrap[class]:hover > span {
    background-position: -40px 0px;  /* gold star */
}
div.modal-review__rating-order-wrap[class] > span:hover ~ span {
    background-position: 0px 0px;    /* gray star */
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>

<div class="modal-review__rating-order-wrap">
  <span data-rating-value="1"></span>
  <span data-rating-value="2"></span>
  <span data-rating-value="3"></span>
  <span data-rating-value="4"></span>
  <span data-rating-value="5"></span>
</div>

Note that I had to add [class] as a specificity hack to the last two CSS selectors, since otherwise the third selector (with .active) would override them. (In 200_success's version, the :after pseudo-classes effectively serve the same function, but my version doesn't use them.)

I also made the inner spans into floating blocks to eliminate any whitespace gaps between them, since those cause annoying "blinking" effects when moving the mouse over the gap. Flex layout on the outer div could be used to achieve the same effect.

Note that I've marked this "answer" as Community Wiki, since it's basically just an addendum to another answer. I'd have posted it as a comment, but you can't include snippets in comments. I didn't feel presumptuous enough to edit this into their answer directly, but if 200_success wants to do so, I'm happy to delete this "meta-answer".

\$\endgroup\$
1
\$\begingroup\$

You should avoid excessive reliance on jQuery. All of the ephemeral hovering behaviour can be handled using CSS alone, and I would recommend doing it that way, instead of fiddling around with the is-active class.

jQuery is only needed to set the persistent state when clicked. In the click handler, you have a bug here:

$(".modal-review__rating-order-wrap span").not(this).removeClass("is-active");

If you have multiple five-star widgets on the page, you would reset is-active on all of them. Instead, you should be using $(this).siblings().removeClass("is-active").

I find getActiveState to be a weird name for a variable: it sounds like a name for a getter function. The use of a variable also prevents you from having multiple widgets on a page. I recommend storing the state in the DOM instead.

I've made a demo using ⭑ and ⭒ characters.

$('.modal-review__rating-order-wrap > span').click(function() {
    $(this).addClass('active').siblings().removeClass('active');
    $(this).parent().attr('data-rating-value', $(this).data('rating-value'));
});
/* Initial state */
div.modal-review__rating-order-wrap {
    color: orange;
}
div.modal-review__rating-order-wrap > span:after {
    content: '\2b52';  /* ⭒ */
}

/* Persistent state */
div.modal-review__rating-order-wrap[data-rating-value] > span:after {
    content: '\2b51';  /* ⭑ */
}
div.modal-review__rating-order-wrap > span.active ~ :after {
    content: '\2b52';  /* ⭒ */
}

/* Hover state */
div.modal-review__rating-order-wrap:hover > span:after {
    content: '\2b51';  /* ⭑ */
}
div.modal-review__rating-order-wrap > span:hover ~ span:after {
    content: '\2b52';  /* ⭒ */
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>

<div class="modal-review__rating-order-wrap">
  <span data-rating-value="1"></span>
  <span data-rating-value="2"></span>
  <span data-rating-value="3"></span>
  <span data-rating-value="4"></span>
  <span data-rating-value="5"></span>
</div>

\$\endgroup\$
0
\$\begingroup\$

One thing I would suggest is caching your jQuery variables so you don't need to keep traversing the DOM to find them:

var $myModal =  $(".modal-review__rating-order-wrap span");
var $parent = $(this).parent();

Also, the application and removal of the is-active class is occuring a fair bit. Is it possible to lift the class up and apply it to an ancestor element higher up the tree, and have its descendents react to that instead?

\$\endgroup\$
  • \$\begingroup\$ This code is applying to a 5 star rating system. I have the .modal-review__rating-order-wrap (as a div around the spans - which are the stars), so .is-active is being applied to both the wrapper, and the individual stars. \$\endgroup\$ – hellomello Jun 5 '17 at 14:26

Your Answer

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

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