Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found

Submitted

Interactive-rating-component

@subhamengine

Desktop design screenshot for the Interactive rating component coding challenge

This is a solution for...

  • HTML
  • CSS
  • JS
1newbie
View challenge

Design comparison


SolutionDesign

Solution retrospective


Feedback is highly appreciated.

Community feedback

P

@GregLyons

Posted

Looks like everything's working properly, and I like how you make the submission button disabled until the user selects a rating; I don't think that was in the challenge itself, so it's a good catch. I have a few suggestions as well:

1) You should be using semantic HTML (<h1>-<h6> tags, <p>, <button>) instead of <div>'s whenever applicable. This makes your website easier to understand for screen readers and SEO. One example is that your rating buttons should actually be <button> elements instead of <div>'s. Another benefit of this is that users can then use "Tab" and "Shift + Tab" to go through the ratings with a keyboard. Some people physically have trouble using computer mice and need to use a keyboard, whereas others (like myself sometimes) just prefer to use keyboards sometimes for filling out forms (like tabbing between input fields). This behavior is built into <button> and <input> elements and so doesn't require much more work on your end (as long as you remember to do it), but it would be quite hard to code for <div> elements. Using semantic HTML lets you leverage a lot of built-in stuff to make your websites even better without doing more than choosing the correct element.

2) I believe display: none; applies not only to the element, but to its children as well. This would help you not have to apply your .non-display class to elements one-by-one like you do in your JavaScript. For this to work, you could take your .summary, .thakyou-title, and .thankyou-para elements in wrap them in a <div>, with a class like .thankyou-wrapper. Then, you could wrap the other elements (your initial component with the rating feature) in another <div>, with a class like .rating-wrapper. Then you can just add/remove the .non-display class on those two wrapper <div>'s.

3) You can read more about display: none; here. Near the middle of the page, you can see that with display: none;, your page will be displayed as if the element isn't there. Thus, these rules in your .non-display class:

margin: none;
border: none;
width: none;
height: none;

aren't necessary/don't do anything, as your element isn't taking up space when it has display: none;. They would be relevant if you used visibility: hidden; instead, in which case the element disappears but still takes up space, but in that case you may as well just use display: none; in the first place.

Hope this helps!

Marked as helpful

0
Ivan 2,630

@isprutfromua

Posted

Hi there. You did a good job 😎

keep improving your programming skills🛠️

your solution looks great, however, if you want to improve it, you can follow these steps:

✅ Fix navigating through page via TAB key

Disallow css fonts @import. CSS font @import prevents parallel downloads, use <link> instead.

✅ Use relative units for font size, and witdth/height such as ems or rems. While modern browsers can smoothly zoom pixel-based layouts, sizing type in relative units ensures an entire layout can be scaled up or down by simply updating the font-size of the body element.

I hope my feedback will be helpful. You can mark it as useful if so 👍 it is not difficult for you, but I understand that my efforts have been appreciated

Good luck and fun coding 🤝⌨️

Marked as helpful

0

@subhamengine

Posted

Thanks for your feedback, it's very very helpful. Next time ill keep all these points in my mind.

1

P

@GregLyons

Posted

@subhamengine Sure thing, glad I could help!

0

Please log in to post a comment

Log in with GitHub
Discord logo

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