REST Countries API with color theme switcher solution

Community feedback

• 

  Travolgi 🍕•31,400

  @denieldenPosted over 2 years ago

  Hi Vinit, good job! I took some time to look at your code and have some ideas for improving it:

  • add more descriptive text in the alt attribute of the image
  • if you write the width attribute in the img element you don't have to specify the px size, just the number
  • remove all unnecessary code, the less you write the better as well as being clearer: for example the div container of image
  • img element must have an alt attribute in the details page, it's very important!
  • to make it look as close to the design as possible add to contain-all-details class:

  display: flex;
  flex-direction: row;
  gap: 5rem;
  flex-wrap: wrap;
  

  • instead of using px try to use relative units of measurement -> read here
  • I don't understand why you have inserted all those data- attributes in the div of the cards of the countries in the home
  • instead of using innerHTML use textContent because it is more secure
  • I would also add a query reset button, I find it very convenient
  • to make all flag images the same height use the object-fit: cover and aspect-ratio: 3/2 property

  Overall you did well 😉

  Hope this help and happy coding!

  Marked as helpful

  0

  ---

  

  VINIT CHURI•160

  @vinit-churiPosted over 2 years ago

  @denielden thanks for the feedback, very helpful

  1