HTML CSS (Grid) Flexbox

Community feedback

• 

  Milan Dordevic•165

  @milan93djordjevicPosted over 4 years ago

  Visually looks good. But visual aspect is not the only one you should care for. Look more into accessibility issues and dive into semantic markup for starter.

  Btw I finished this same project few hours back and looking trough your code I realized my grid has significantly more columns which I now realized is absolutely unnecessary.. Good job on that grid, simple and sufficient. I like it. Good job!

  Milan D

  2
• 

  T

  Matt Studdert•13,611

  @mattstuddertPosted over 4 years ago

  Nice work on this challenge and congrats on submitting your first solution! 🙂

  It's great to see you using CSS Grid. I'd recommend reviewing the responsiveness of your solution one more time. You've got a single max-width: 375px media query, which means at 376px up to a large tablet size the content doesn't fit the screen.

  Also, have you ever tried using min-width media queries instead of max-width? It's quite a common workflow with front-end developers to use them and work mobile-first. It can often lead to less CSS code and has the benefit of loading in fewer styles for mobile users, which can be a nice performance gain.

  Keep up the great work!

  1
• 

  Bgnichol•60

  @bgnicholPosted over 4 years ago

  Thanks so much for the feedback, Matt. I am just dipping my toe into media queries, so these are just the sort of pointers I need. I'll use the min-width from now on. Thanks again!

  0
• 

  Bgnichol•60

  @bgnicholPosted over 4 years ago

  Thanks so much, Milan. I appreciate it. I also realized after posting that my images did not come across on Github. I will look into that too. I look forward to checking out your version.

  0