CODEWITHSUNDEEP
javascripthtmlcssdomecmascript-6
Sumit Ghewade
Sumit Ghewade

Reputation: 483

Can somebody please fix my Star rating created using HTML CSS Vanilla Javscript (Star rating will only highlight previous elements)

I have created a star rating app using CSS HTML and Javascript. when I click on a star it doesn't get highlighted as expected. Suppose I click the third star then only two stars before it gets highlighted.

I'm not getting how I'm suppose to handle the css property which will make the current clicked star highlighted.

let stars = document.querySelectorAll('.star');
document.querySelector('.star-container').addEventListener('click', starRating);
let rating = document.querySelector('.rating');

function starRating(e) {
	if (e.target.classList[0] == 'star') {
		stars.forEach((star) => star.classList.remove('star__checked'));
		for (i = stars.length - 1, j = 0; i >= 0; i--, j++) {
			if (stars[i] !== e.target) {
				stars[i].classList.add('star__checked');
			} else {
				stars[i].classList.add('star__checked');
				rating.textContent = `${j + 1}/5`;
				break;
			}
		}
	} else {
		stars.forEach((star) => star.classList.remove('star__checked'));
		rating.textContent = `${0}/5`;
	}
}
.star-container {
	display: flex;
	width: 350px;
	flex-direction: row-reverse;
}

.star {
}

.star:before {
	content: '\f005';
	font-family: fontAwesome;
	font-size: 60px;
	position: relative;
	display: block;
	color: pink;
}

.star:after {
	content: '\f005';
	font-family: fontAwesome;
	position: absolute;
	top: 7px;
	font-size: 60px;
	color: gold;
	opacity: 0;
}
.star:hover:after,
.star:hover ~ .star:after {
	opacity: 1;
}

.star__checked ~ .star:after {
	opacity: 1;
}
<html lang="en">
	<head>

		<link rel="stylesheet" href="./styles.css" />
		<link
			rel="stylesheet"
			href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/4.7.0/css/font-awesome.min.css"
		/>
		<title>Document</title>
	</head>
	<body>
		<div class="star-container">
			<div class="star"></div>
			<div class="star"></div>
			<div class="star"></div>
			<div class="star"></div>
			<div class="star"></div>
		</div>
		<h1 class="rating">0/5</h1>
		<script src="app.js"></script>
	</body>
</html>

Upvotes: 1

Views: 663

Answers (3)

Tamas Szoke
Tamas Szoke

Reputation: 5542

I made a couple of improvements:

  • No need for another loop, just use e.target
  • You can use the data-val attribute for the values
  • Add star__checked:after to work with the last element

Here's the working code:

let stars = document.querySelectorAll('.star');
document.querySelector('.star-container').addEventListener('click', starRating);
let rating = document.querySelector('.rating');

function starRating(e) {
  const star = e.target;
  const val = star.getAttribute('data-val');

  stars.forEach((star) => star.classList.remove('star__checked'));

  star.classList.add('star__checked');
  rating.textContent = `${val}/5`;
}
.star-container {
  display: flex;
  width: 350px;
  flex-direction: row-reverse;
}

.star:before {
  content: '\f005';
  font-family: fontAwesome;
  font-size: 60px;
  position: relative;
  display: block;
  color: pink;
}

.star:after {
  content: '\f005';
  font-family: fontAwesome;
  position: absolute;
  top: 7px;
  font-size: 60px;
  color: gold;
  opacity: 0;
}

.star:hover:after,
.star:hover~.star:after {
  opacity: 1;
}

.star__checked~.star:after,
.star.star__checked:after {
  opacity: 1;
}
<link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/4.7.0/css/font-awesome.min.css" />

<div class="star-container">
  <div class="star" data-val="5"></div>
  <div class="star" data-val="4"></div>
  <div class="star" data-val="3"></div>
  <div class="star" data-val="2"></div>
  <div class="star" data-val="1"></div>
</div>
<h1 class="rating">0/5</h1>

Upvotes: 1

SMAKSS
SMAKSS

Reputation: 10520

Actually your issue lies behind this line:

.star__checked~.star:after {
  opacity: 1;
}

What's happening here?

While you applying star__checked to each desired element there is no need to apply style for successor siblings. ~ is for general successor sibling, so whenever you define such a style it would only select the successor elements (Since your using flex-direction: row-reverse; your element will begin from right to left, so then it will only highlight your successor elements). Thus you should just apply the opacity: 1; for elements with star__checked class.

How to fix it?

If you want to fix your current code, you just have to replace the mentioned part with the following and you will be fine:

.star__checked.star:after {
  opacity: 1;
}

Your final code would be something like this:

let stars = document.querySelectorAll('.star');
document.querySelector('.star-container').addEventListener('click', starRating);
let rating = document.querySelector('.rating');

function starRating(e) {
  if (e.target.classList[0] == 'star') {
    stars.forEach((star) => star.classList.remove('star__checked'));
    for (i = stars.length - 1, j = 0; i >= 0; i--, j++) {
      if (stars[i] !== e.target) {
        stars[i].classList.add('star__checked');
      } else if (stars[i] === e.target) {
        stars[i].classList.add('star__checked');
        rating.textContent = `${j + 1}/5`;
        break;
      }
    }
  } else {
    stars.forEach((star) => star.classList.remove('star__checked'));
    rating.textContent = `${0}/5`;
  }
}
.star-container {
  display: flex;
  width: 350px;
  flex-direction: row-reverse;
}

.star:before {
  content: '\f005';
  font-family: fontAwesome;
  font-size: 60px;
  position: relative;
  display: block;
  color: pink;
}

.star:after {
  content: '\f005';
  font-family: fontAwesome;
  position: absolute;
  top: 7px;
  font-size: 60px;
  color: gold;
  opacity: 0;
}

.star:hover:after,
.star:hover~.star:after {
  opacity: 1;
}

.star__checked.star:after {
  opacity: 1;
}
<html lang="en">

<head>

  <link rel="stylesheet" href="./styles.css" />
  <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/4.7.0/css/font-awesome.min.css" />
  <title>Document</title>
</head>

<body>
  <div class="star-container">
    <div class="star"></div>
    <div class="star"></div>
    <div class="star"></div>
    <div class="star"></div>
    <div class="star"></div>
  </div>
  <h1 class="rating">0/5</h1>
  <script src="app.js"></script>
</body>

</html>

But if you looking for best practices, you have to do some further implementation and some magic with previousElementSibling or indexOf(), which cost tones of work to achieve, but its highly recommended.

I will give it a touch with indexOf(), where this method will give us the index of the currently selected element in the array. Then we highlight it as usual and after that, with the ~ we will select all successor siblings as well. But keep in mind the star__checked class will only apply to the selected element.

So it will be something like this:

const stars = document.querySelectorAll('.star');
document.querySelector('.star-container').addEventListener('click', starRating);
const rating = document.querySelector('.rating');

function starRating(e) {
  stars.forEach((star) => star.classList.remove('star__checked'));
  const i = [...stars].indexOf(e.target);
  if (i > -1) {
    stars[i].classList.add('star__checked');
    rating.textContent = `${stars.length - i}/5`;
  } else {
    star.classList.remove('star__checked');
    rating.textContent = `0/5`;
  }
}
.star-container {
  display: flex;
  width: 350px;
  flex-direction: row-reverse;
}

.star:before {
  content: '\f005';
  font-family: fontAwesome;
  font-size: 60px;
  position: relative;
  display: block;
  color: pink;
}

.star:after {
  content: '\f005';
  font-family: fontAwesome;
  position: absolute;
  top: 7px;
  font-size: 60px;
  color: gold;
  opacity: 0;
}

.star:hover:after,
.star:hover~.star:after {
  opacity: 1;
}

.star.star__checked~.star:after,
.star.star__checked:after {
  opacity: 1;
}
<html lang="en">

<head>

  <link rel="stylesheet" href="./styles.css" />
  <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/4.7.0/css/font-awesome.min.css" />
  <title>Document</title>
</head>

<body>
  <div class="star-container">
    <div class="star"></div>
    <div class="star"></div>
    <div class="star"></div>
    <div class="star"></div>
    <div class="star"></div>
  </div>
  <h1 class="rating">0/5</h1>
  <script src="app.js"></script>
</body>

</html>

Upvotes: 5

Sebastian B.
Sebastian B.

Reputation: 2191

Another simplified version:

let stars = document.querySelectorAll('.star');
document.querySelector('.star-container').addEventListener('click', starRating);
let rating = document.querySelector('.rating');

function starRating(e) {
  stars.forEach((star) => star.classList.remove('star__checked'));
  let i = [].indexOf.call(stars, e.target);
  if (i > -1) {
    stars[i].classList.add('star__checked');
    rating.textContent = `${stars.length - i}/5`;
  } else {
    rating.textContent = `${0}/5`;
  }
}
.star-container {
  display: flex;
  width: 350px;
  flex-direction: row-reverse;
}

.star {}

.star:after {
  content: '\f005';
  font-family: fontAwesome;
  font-size: 60px;
  display: block;
  color: pink;
}

.star.star__checked ~ .star:after,
.star.star__checked:after,
.star:hover ~ .star:after,
.star:hover:after {
  color: gold;
}
<html lang="en">

<head>

  <link rel="stylesheet" href="./styles.css" />
  <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/4.7.0/css/font-awesome.min.css" />
  <title>Document</title>
</head>

<body>
  <div class="star-container">
    <div class="star"></div>
    <div class="star"></div>
    <div class="star"></div>
    <div class="star"></div>
    <div class="star"></div>
  </div>
  <h1 class="rating">0/5</h1>
  <script src="app.js"></script>
</body>

</html>

Upvotes: 1

Related Questions