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

Javascript Dom, position property of csss and flexbox

@BABAR1532

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


This is my first javascript project. If you have time please check my javascript code quality and suggest to me if it can be improved

Community feedback

AC 🍀 340

@alleycaaat

Posted

Good job completing the project!

I took a look at your code and have a couple of comments. Technically your HTML is fine and works, but for accessibility there are a couple of things that could improve. For the rating options you used a span tag, which doesn't tell users with screen readers what they, the span element, are. A user could infer it based on context, but there are steps we can take to make things clearer. For instance, making the rating options buttons or radio inputs will let the user know it's something they can interact with. Here's an article on accessible buttons if you want to read more. Another reason this is really important is for keyboard users, they can visually see that the numbers are something to interact with, but if you press tab while on the page, the focus goes to the Submit button. They don't have a way to pick a rating. You submit button is great because it's already a button element and the text on it will tell the user what it does when pressed. :)

Your JavaScript looks good! One way it could be a little cleaner is to add or remove a class from the clicked element versus changing the actual styling in your JS. Where you have span.style.backgroundColor = 'hsl(217, 12%, 63%)'; you could put span.classList.add('selected') and that style from your stylesheet would change the background-color and text color. Here's the MDN article on classList. There's nothing technically wrong with your code now, classList is just a handy property to access. :)

You could also use classList to hide or display your divs, too. Right now you have .container-after-submisson with z-index: -1, tucking it behind your .container div. And it works, but messing with z-index is a little more effort. Since you used flexbox, you can simply give .container a style of display: flex and div .container-after-submisson another class of hidden { display: none !important}. When you want to display the results page, remove .hidden from .container-after-submisson and then apply it to .container.

Overall, it's a really nice project! The JS comments just clean things up and make for easier coding, but I do really recommend making your rating option elements more accessible. Let me know if you have any questions or I was confusing :)

Cheers!

Marked as helpful

1

@BABAR1532

Posted

@alleycaaat Thank you very much for commenting on my project. These small mistakes that I did, next time definitely take care of it and also Thank you for sharing the article to improve my knowledge regarding semantic markup in HTML. People like you who always try to help new learners are great motivation for me to also help other fellows and make something great! Cheers! Good night! or good morning! maybe Bye::

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