Responsive Interactive Rating Component

Community feedback

• 

  Richard Ogujawa 👨‍💻•170

  @RichardOgujawaPosted almost 2 years ago

  Hi Carlos!

  Great job with the code, love what you've done man. Just had a few recommendations but feel free to disagree.

  In the HTML:

  • To write more semantic HTML, I would recommend using an article tag instead of a div for the rating component because it's a self-contained composition as it doesn't require other information around it to be understood. More on HTML articles here: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/article
  • I appreciate the use of BEM man
  • I know that more than likely the heading in this component wouldn't be a h1 level heading if it was included in a website, but given that it's the only content on the page it should be regarded as the main heading on the page, and therefore does deserve to be written using a h1. The h1 is simply for the most important text on the page, the introduction, and what is the page about. Every page should have one. h2, h3, etc. are for headings of lesser importance, and for them to be of lesser importance, there must first be a heading of greater importance. If the h1 tag appears to be inappropriate it's probably because of its appearance, but don't let appearance prevent you from making your HTML as semantic as possible. You can always fix the appearance later with the CSS.
  • I'd recommend using the picture tag, it's better than just using the img tag. More on that here: https://blog.bitsrc.io/why-you-should-use-picture-tag-instead-of-img-tag-b9841e86bf8b

  In the CSS:

  • I'd recommend not using the hsl function in your color variables. You're better off just mentioning the values (i.e. '--soft-blue: 215, 51%, 70%' instead of '--soft-blue: hsl(215, 51%, 70%)'). This will allow you to later add an alpha value if needs be. For example, if you wanted the soft-blue colour to be slightly transparent, but used the variables the way you've written it, you wouldn't have that flexibility to do so. However, if you only used the values, then you could do something like this. 'color: hsl(var(--soft-blue), 50%)'
  • I'd recommend separating the different properties in your CSS based on what they do, for example having display properties separate from the dimension properties separate from the color properties, etc. And by separate I simply meaning grouping them together with a space between each group. It makes the code a lot easier to read. However, I would recommend putting anything that has to do with the dimension of the elements in the same group though, for example, instead of separating width from padding, I'd put them in the same group, because they all contribute to the overall size of the element. Things like margins, and borders also contribute to the overall size of an element. I also include font-size when I write my CSS, but it's a bit more debatable I guess.
  • I like how you used radio inputs instead of buttons, it was something that I had in mind to do but wasn't able to execute at the time and so just settled on doing buttons. But, after seeing what you did I decided to go back and give it a try again and got it so thank you! I would also recommend using a fieldset around the radio buttons to group them together and a legend with an sr-only class on the fieldset too.

  Hope this helps, and sorry for any typos if there are any:)

  Marked as helpful

  1

  ---

  

  Carlos•110

  @carlosmndzgPosted almost 2 years ago

  @RichardOgujawa Hello!

  First of all thanks for your comment.

  It is true that I could use an article instead of a div, which is more semantic I didn't noticed. With the HSL colors, I didn't know about the problems related to the alpha values, so that is very helpful! I appreciate all your advice!

  Thank you a lot for your help, have a nice day!

  0

  

  Richard Ogujawa 👨‍💻•170

  @RichardOgujawaPosted almost 2 years ago

  @carlosmndzg Happy to help man. Thanks for taking the time to read it, and I'm looking forward to checking out what you do next, my code isn't perfect also, but I'd like to believe it's evolving with each challenge, but if you do see anything feel free to let me know too.

  Always happy to learn.

  Have a great day:)

  1