CSS Flexbox and JavaScript

Community feedback

• 

  Yari Morcus•500

  @YariMorcusPosted over 2 years ago

  I took a look at your solution, and have a couple of feedback points and tips I want to share with you. I do need to say up front that it might be a lot to digest, but I am doing this to not only help you out, but also to share information about things I learned from others.

  Feedback points

  1. I am able to submit a rating, even if I did not select any number. In a 'normal' rating system this would not be allowed, because normally you cannot submit your review form, without not giving up a rating number. Unless the system doesn't require you to supply a rating number, but in this challenge it does (or at least, that is how I read it in the challenge description).

  2. I always learned not to write a word in uppercase, when you want to show it in uppercase to your users. This has more to do with Web Accessibility because screenreaders might read it in the wrong way. My advice is to always use text-transform: uppercase;.

  A couple of tips

  I do have a few other tips for you as well:

  1. Always supply a raster image as a fallback for your vector graphics, in the case the user disabled images, or the browser does not support an SVG. Otherwise, nothing will be shown to the user. I noticed that you did not supply text within the alt attribute, and that is good (because it is decorative)!

  2. Try to use a little more of the semantic HTML elements HTML5 offers you for structuring your document. You can think of <article>, <section> and <header>. When I did this challenge, I used <article> for both the rating card and thank you card, <section> to indicate it is a specific section within article (section offers the same functionality), and <header> to enclose any titles or introduction content. When you start using these semantic HTML elements more, you make your component even more accessible for people who use assistive technologies (such as screenreaders), because theses HTML elements are recognized better than using the common 'div'.

  3. Do not use of @import statement within your stylesheets. They create a new HTTP request, which in return results in slowing down the speed of your website (when the user requests to open your webpage). I do need to say that in this case, it doesn't really matter, because it is just one of them, and the website doesn't use a lot of resources. But I do recommend not to use them when you use more of them, especially when creating a big website. Just as a sidenote: speed is an important metric for search engine optimization (SEO), and also user experience.

  4. Don't try to repeat your CSS (follow the DRY principle). It makes it harder to maintain, and unnecessarily increases the size of your file (again, this has to do with the speed of your website).

  I saw that body main, .star, .rating-star-component and .thankyou-page-wrapper use the following CSS declarations:

  display: flex;
  justify-content: center;
  align-items: center;
  

  In this case, I recommend you to group all of the above named selectors together, and apply above CSS declarations to it:

  body,
  main,
  .star,
  .rating-star-component,
  thankyou-page-wrapper {
      display: flex;
      justify-content: center;
      align-items: center;
  }
  

  When you have to update one of the CSS declarations, it will all be adjusted automatically for the other selectors (you spend less time repeating the same action).

  Other things I want to say

  The responsiveness of your interactive rating component is something I tested as well, and that seems to be fine (which is very important nowadays).

  I hope you can do something with the feedback points and tips I gave you, but don't see it as something you need to do, but rather something for later (you are always free to correct your solution if you want).

  If I made a mistake somewhere in this post, feel free to correct me and keep building awesome things :D.

  Marked as helpful

  1

  ---

  

  Abdullah Ajayi•70

  @AbdullahAjayiPosted over 2 years ago

  @YariMorcus Thanks so much bro for taking your time to view my work. It really mean a lot to me. Accessibility isn't sth I've been incorporating into my small parctice projects, but will start looking forward to that.

  Thanks a ton. I learnt a lot from your reply. Also, sorry for the late response. I couldn't help it.

  0