Interactive-Card

Community feedback

• 

  Anton•515

  @antaryaPosted about 2 years ago

  Hi 👋,

  Nice job 🚀. I noticed a couple of things that can be improved.

  HTML

  [1. Do not use main tag as a wrapper of card, instead add parent div or section tag for your card <main><div class='panel'>...</div></main> as a child of main tag. main tag specifies the main content of a document https://www.w3schools.com/html/html5_semantic_elements.asp

  [2. Rate-related elements with submit button can be recreated and wrapped with the form, which is a better way to handle this case. Instead of a button for each rate, you can use radio input as it matches the required logic choosing one from several. You will learn a lot by checking other solutions after your attempt, e.g. check the solution for the same challenge of @Elaine https://www.frontendmentor.io/solutions/responsive-interactive-rating-component-SeBo-aR4gB it is implemented using form and radio inputs; it has consistent names and implemented well.

  CSS

  [3. The main tag used as a card wrapper has a width: 25vw without restriction on huge screens; your card will be pretty wide. Instead, you can set the width to 100%, but restrict it not to be greater than 400px using max-width: 400px.

  [4. It looks like you are using BEM for naming classes; if yes, I think it is not used correctly. For example .Card__starbox--img - img is not a modifier but an element, instead you can use something like card__sticker, same for .Card__Submitbox--submit. .Card__Rating---rate has three dashes, and .second-image is not part of the card - consistency will make your code easy to read and maintain.

  • BEM - https://getbem.com/naming/
  • CUBE CSS - https://cube.fyi/
  • challenge created following CUBE CSS - https://www.youtube.com/watch?v=NanhQvnvbR8

  JS

  [5. You don't need to apply styles from javascript in this case. Define them in CSS and use the class name to reference those styles.

  [6. For a map of a similar key/value in the getRate function, why not use Amount directly? 

  [7. While there is nothing wrong with string concatenation, you can also check template literals.

  • https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Template_literals.

  [8. Getting the answer value can be moved into submit click listener, as it is needed only on submit, not before.

  General

  [9. Use reset to have a good starting point, e.g. https://piccalil.li/blog/a-modern-css-reset/.

  [10. There is only one commit which might be fine for a small project, but even on a small project, train yourself and commit often with a meaningful message when specific work is completed. For a consistent format of commit messages, you can use (https://www.conventionalcommits.org/en/v1.0.0/).

  Let me know if you have questions. I hope this will be helpful.

  Keep up the good work 🚀. Cheers!

  1
• 

  Hyron•5,870

  @hyrongennikePosted about 2 years ago

  Hi @Andrusik1,

  Congrats on completing the challenge

  I would suggest just disabling the button until a rating is selected this can be done by adding the disabled attribute on the button and removing it when the rating is clicked currently it's "a" when you click submit without selecting a rating.

  Hope this is helpful

  1