Layout and Responsive using Grid

Community feedback

• 

  Vanza Setia•27,775

  @vanzasetiaPosted over 2 years ago

  Hello, Jean! 👋

  Congratulations on finishing this challenge! 🎉

  I have some feedback on this solution:

  • Accessibility
    • Good job on using landmark elements! 👍
    • For the div with a class of numbers, swap the div with ul and wrap each item with li.
    • <h2>10k+</h2> swap the h2 with span. It's not a heading. The content below it is too small. You might find it helpful if you think of a heading like a title on a document.
    • About the nbsp in the footer. I don't think it's a necessary thing to do. I think a normal space will just work.
  • Styling
    • Changing the layout from two columns into one column in 375px is too late. I would recommend adjusting the breakpoint of your media query.
    • Keep in mind that those sizes on the style-guide.md have nothing to do with the media queries. They are telling you that "this is how your website should look like at these screen sizes". You should keep making your website looks good in between those screen sizes.
    • Also, I would recommend writing the styling using the mobile-first approach. It often makes me write less code.

  I hope this information is useful! Happy coding! 😁

  Marked as helpful

  1
• 

  Deepak Parmar•490

  @deepak-parmarPosted over 2 years ago

  Your path to the image should be images/image-header-mobile.jpg

  ../ is causing it to look for the image out of stats-preview-grid folder.

  1

  ---

  

  Jean Ruiz•590

  @jeancarlosruizPosted over 2 years ago

  @deepak-parmar Thank you!

  1
• 

  Hamdi Mohamud•100

  @hamdi2008Posted over 2 years ago

  Nice work. I will just add a top margin to the mobile view of the image so that image is not stuck to the top.

  0