Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found

Submitted

interactive-rating-component

Eugi 270

@EugiSs

Desktop design screenshot for the Interactive rating component coding challenge

This is a solution for...

  • HTML
  • CSS
  • JS
1newbie
View challenge

Design comparison


SolutionDesign

Solution retrospective


I am learning to write JS. I will be glad to comments :)

Community feedback

P

@GregLyons

Posted

It looks like all of your HTML, CSS, and JavaScript is pretty solid. Nice!

The one thing I'd suggest changing would be to put the rating numbers in <button> elements. Right now, the user can only fill out your form with a mouse, since they can't focus the ratings to select them. While you could make the <li> elements themselves focusable, you'd have to worry about other things like making sure the Enter key also selects the rating using another event listener, and so on. Instead, the <button> element is focusable by default, and pressing Enter activates the click event, so it's a lot easier to work with. Doing this would make your website more accessible, not just for users who can't use a mouse, but also for users who prefer to use a mouse.

As I mentioned before your JavaScript is good, but since you said that you're learning to write JS, let me give you a tip that you may want to consider for future projects:

If, instead of passing an anonymous callback function into addEventListener, you first define a function beforehand and then pass in that function to addEventListener, it then makes it easier for you to remove the event listener in the future when necessary, like so:

const handler = function(e) {
  ...
};

document.addEventListener("...", handler);

...

document.removeEventListener("...", handler);

In other words, you can just pass in the name of the function to removeEventListener. See Adam Heath's answer on this StackOverflow post for more details.

Obviously you don't need to do that in this challenge, but you'll likely find that you need to remove an event listener at some point in the future.

Marked as helpful

0

Eugi 270

@EugiSs

Posted

@GregLyons You're right, using a <button> would be much better if you thought about it, thanks. I will definitely keep this in mind in the future. I have been researching about removing an event listener, but here I decided to write an anonymous function since nothing needs to be removed. But anyway, thanks for the reminder. And thanks for the rating and advice :)

1

@Kamasah-Dickson

Posted

I really like your solution and also the rotation you added. But your card is not responsive on smaller devices, there is this overflow.

So I suggest you use max width on your cards instead of just width.

Besides good job there👍 Happy coding👍

0

Eugi 270

@EugiSs

Posted

@Kamasah-Dickson Yes, I really didn't notice that the card drops out on smaller devices. I checked the functionality and forgot about it. Thanks for your comment and rating!

0

@Kamasah-Dickson

Posted

@EvgiSs you are always welcome👍

0

Please log in to post a comment

Log in with GitHub
Discord logo

Join our Discord community

Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!

Join our Discord