Skip to content
Submitted over 1 year ago

Remove and add a display class when submitting the form using JS.

Michael Degorre•160
@MDEGORRE
A solution to the Interactive rating component challenge
Preview (opens in new tab)Code (opens in new tab)

Solution retrospective

Hello, this is my first challenge submitted.

My process:

  • get the input checked value when form is submitted.
  • replace the content adding a display: none property.

Feel free to give me any advices please !

Code

Please log in to post a comment

Log in

Community feedback

  • Grace•32,130
    @grace-snow

    Looks good overall but there is room for improvement:

    1. All content should be contained within landmarks. That means both form panels should be in main, and attribution in footer.
    2. Decorative icons should have empty alt
    3. The radios should be in a fieldset with a legend (or visually-hidden legend). It's like the question is not associated to the answers in any way right now.
    4. In js don't get the selected value from a quest selector! Get it from the form data. The chosen value is already available to you with this.rating.value (ie form.rating.value where "rating" is the name of the field whose value you want)
    5. Consider using the hidden attribute for showing and hiding instead of a class (optional)
    6. The thank you panel should be wrapped in an extra div that has aria-live on it and is always present in the DOM (I.e. Is not display none). This means the thank you content would be announced to assistive tech users when it appears.
    7. Never limit the height of elements that contain text, including the body. Min-height is fine to use, height is not.
    8. Do not set an explicit width on the component. Use max-width instead.
    9. Never set font size in px, not even once
    10. The radios must be inclusively hidden! That means using a visually-hidden class not setting opacity to 0, which can lead to elements being hidden from some screen readers.
    11. This challenge does not need a media query. But if it did (note for future challenges) that media query would need to be min width in a mobile first layout (99% of styling will be mobile first) and would need to be defined in rem or em not px.
    12. Make sure the component has a little margin on all sides or a wrapping element (eg body in this case) has a little padding on all sides, so the component cannot hit screen edges for any users. It can currently hit screen edges on the top and the bottom on mobile and on all sides on larger screens for users with larger text sizes
    Marked as helpful

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