Design comparison
Solution retrospective
I don't know if I did the javascript part right, it is my first project of javascript. More exactly this part of code: submit.addEventListener("click", function(){ document.querySelector(".selector").innerHTML = "You selected " + stars + " out of 5"; document.querySelector(".div-boss.first-div").classList.add("hidden"); document.querySelector(".div-boss.thank-you").classList.remove("hidden"); }), where I should display the thank you page.
Community feedback
- @ErayBarslanPosted about 2 years ago
Hey there and nice work! JS looks alright but there can be minor adjustments. You can use
focus
selector with CSS so you won't need a nested loop and.active
class..f-btn:focus { background-color:hsl(25, 97%, 53%); color:white; } for (var i = 0; i < buttons.length; i++) { buttons[i].addEventListener("click", function () { stars = this.value; }) }
Rest of JS is fine. Also you can set the background-color to body and position card to center with the help of grid:
html { height: 100%; } body { min-height: 100%; font-family: 'Overpass', sans-serif; margin: 0; background-color: hsl(216, 12%, 8%); display: grid; justify-content: center; align-items: center; } .attribution { position: absolute; left: 50%; transform: translate(-50%, 0); top: 95%; }
Without absolute on attribution, our card wouldn't be centered properly. Lastly for semantic html, your elements should be contained landmarks. You can change
<section> to <main>
and<div class=".attribution"> to <footer>
Also you use a<h1>
in your page but by default it's hidden. In this case you can change<h2>
to<h1>
heading for better SEO result. This makes the last of my suggestions. Happy coding :)Marked as helpful0 - @sinjin25Posted about 2 years ago
The js seems fine to me at least.
I think that your script itself could be improved. I would recommend putting it in the head then using the
defer
attribute. This is non-blocking (according to https://www.w3schools.com/tags/att_script_defer.asp) and waits until the dom content is loaded to execute, so no need to put it in the body anymore.Your loop readability might be improved by using a
.forEach
but that is up to personal style.Good work on the project :)
Marked as helpful0
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