Javascript Dom, position property of csss and flexbox
Design comparison
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
- @alleycaaatPosted over 1 year ago
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, thespan
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 theSubmit button
. They don't have a way to pick a rating. Yousubmit button
is great because it's already abutton
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 havespan.style.backgroundColor = 'hsl(217, 12%, 63%)';
you could putspan.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 yourdiv
s, too. Right now you have.container-after-submisson
withz-index: -1
, tucking it behind your.container div
. And it works, but messing withz-index
is a little more effort. Since you used flexbox, you can simply give.container
a style ofdisplay: flex
anddiv .container-after-submisson
another class ofhidden { 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 helpful1@BABAR1532Posted over 1 year ago@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 GitHubJoin 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