Design comparison
SolutionDesign
Solution retrospective
I'm studying javascript, please give feedback.. thanks
Community feedback
- @pikapikamartPosted about 3 years ago
Hi, the layout in desktop is good, for the mobile, it could use bit of paddings so that it won't touch both the sides of the screen.
The functionality works as well.
Suggestions would be that:
- Your choice of
radio
buttons are correct, but its state is not properly used. If you useddisplay: none
on an element, especially for interactive elements, screen reader won't have any idea on how to use it, in this case, your tip will not work properly for those people. What you could have done instead is that, do not use thedisplay: none
property, instead just use a sr-only class on theinput type="radio"
then, on your css, you could have just triggered ainput:focus-visible + label
selector, then just put a outline property on thelabel
so that keyboard functionality, as well as screen reader will work properly. I would remove remove as well thechecked
property that you declared on everyinput
. When usinginput type="radio"
checked
property will only work with one, as long as thename
property of theinput
is the same. - add a
event.preventDefault
on yourform
so that it won't refresh the page when a user presses the enter or click thereset
button - header tag here is not really well suited since your image does not have any
alt
, add a text on it so that your header will function properly via landmarks in screen reader. Thealt
text could be the text from the image. - add a
h1
tag on your solution. Thish1
should have the sr-class. When I say sr-class, I mean that it should have screen reader only text, you could search for that one. Yourh1
text should be the name of this app, then a little bit text to describe, maybe 4 words or so. - aria-live could be really helpful as well. This is addition if you want to really make your solution accessible.
Other than that, great work.
Marked as helpful0@dwi312Posted about 3 years ago@pikamart Thank you for the input, I will study your input
0 - Your choice of
- @hafizanadliPosted about 3 years ago
- for better folder structure, you can make styles folder for your css file and scripts folder for js file. Instead of putting that file inside assets folder. Assets folder is for static file.
- better make function handler for reset button, instead of reset by reloading whole page
0@dwi312Posted about 3 years ago@hafizanadli naaahhh... apparently this is what I missed, I haven't given the settings to the reset function
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