
Design comparison
Solution retrospective
Please look at the code and give me feedback on better accessibility techniques
Community feedback
- @grace-snowPosted 3 months ago
I'm afraid this needs rewriting. You must learn to use appropriate html, especially on forms.
This is a form with a fieldset of radios. That fieldset must have a legend (or it can be a role radiogroup and use aria-labelledby or aria-label instead of legend).
There should be no, click listeners anywhere in this. Just a form submit listener.
Once that's all changed you'll need to change the styles and js. The good news is the javascript should be much shorter. It only needs a line to grab the rating value from the form and a line to hide/show the panels.
Make sure the thank you is wrapped in an aria-live region or focus moves to it.
And make sure decorative images have empty alt attributes.
Marked as helpful0@grace-snowPosted 3 months agoWith the styling you are also over complicating this. It doesn't need a media query or clamped width. A max width in rem should do for the sizing, and a little padding on the body so the component can't hit screen edges.
The column gap between options can be small but use justify-content space-between.
The submit button shouldn't have a height, just padding.
The card padding can be clamped or use a min() function like min(20px, [whatever % value]
1@Raymond023Posted 3 months ago@grace-snow I've made all the corrections for better accessibility and performance, I even got rid of jQuery. Thank you so much, I really can't say how uch I've learnt and improved in my skills from your feedbacks. I look forward to more of your feedback. Cheers.
0@grace-snowPosted 3 months ago@Raymond023 well done. There are still some problems here. Inputs can't be hidden like that with opacity 0. That makes them inaccessible. You must hide elements in an accessible way.
Note there was no need to remove the img elements. Empty alt makes them decorative. Personally I hate seeing empty elements in html but that is just personal preference.
It's also not recommended to change inline styles with javascript. Use js to toggle classes or attributes and let css handle all styling.
I'm pretty sure the form method should be get and not post.
In the js you can get the rating value in a much simpler way. It's right there in the form data. So all you need inside that submit function is this.rating.value(or form.rating.value)
0@Raymond023Posted 3 months ago@grace-snow quick questions
- So using opacity 1 is good practice in hiding the radio?
- I thought using get is for a case where you want to retrieve data from a server, and I felt using post would be better as the user would be sending their review?
0@grace-snowPosted 3 months ago@Raymond023 Look up how to accessibly hide elements.
Get and post are just about security. This is not sensitive data.
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