All comments

• 

  T-Gomziakov•10

  @T-GomziakovSubmitted about 1 year ago

  Hello! This is my first solution and would appreciate any feedback people have.

  

  QR container component created using HTML and CSS

  1

  50
  

  MatejBumbera•70

  @MatejBumberaPosted about 1 year ago

  Hi, first of all, congratulations on your solution! You did a great job in matching it to the design and I really like your code too. I just think you have some excessive code there.

  • The excessive code I mean is assigning classes to main, h1 and p tags. I think you could easily just style directly the main, h1 and p tags.

  • Another thing I noticed is that you used picture tag around img, and I'd say that's not necessary since the picture tag is used when you want to put there multiple images that change in response to screen width. You can look up more about the usage of picture tag here: https://www.w3schools.com/html/html_images_picture.asp

  • And one tiny thing I noticed is u used article tag, which is very good since it's semantic, but I wouldn't really use it here since article should be used on part of a webpage that should be independent, like u could just put it on a different webpage and it would make sense. I would replace it with section, which is also semantic. You can check the difference at: https://www.w3schools.com/html/html5_semantic_elements.asp

  Overall, you did a great job and I wish you happy coding! I hope you find my review helpful :)

  Marked as helpful

  1

  This comment has received 2 repliesShow
• 

  Levi Ribeiro•140

  @ribeiroLeviSubmitted over 1 year ago

  I am having a realy bad time trying to match the resolution of my version to the resolution of the design. any tips?

  

  3 column preview card

  2

  71
  

  MatejBumbera•70

  @MatejBumberaPosted over 1 year ago

  Hi, I don't have the answer to your question, but I have some tips regarding your solution. First of all, congratulations on your solution! You did a great job in matching it to the design. Now to the tips:

  • I noticed some repetition in your code, mainly defining flexbox and its properties to both the container class and space id (+ assigning the same id to more elements is a bad practice).
  • You could use semantic HTML in your code. That means using tags like <main>, <section>, <header>, <footer> etc. instead of <div> tags. They are more descriptive so your code is more clear and it helps assistive technologies and search engines. You can study it more here: https://www.w3schools.com/html/html5_semantic_elements.asp
  • The website doesn't change in response to smaller web screen (for example on mobile phone). You could do this by using media queries, or in your case, you could use flex-wrap: wrap; declaration in a container rule and don't define the flexbox container width, define only flex items width. What happens is that when the items don't fit on the screen, they move on to the next line.

  But overall, good job and keep coding! I hope you find my review helpful :D

  0