Responsive landing page using Grid

Community feedback

• 

  P

  Jairo Valderrama•420

  @jairovg

  Posted almost 3 years ago

  Hey @mgallegoa, here are some feedback on your solution

  • You're using for background-size a contain value which stretches the image to its container height, but it won't do it to its width. Try to check the dev tools for different breakpoints like 425px or 768px and see how this value affects the page. Look for the possible values you may use in the MDN docs.
  • You have multiple a11y issues; probably solving one, in particular, may solve others; this is page must have one main landmark. You can read more about this issue on the Deque University Axe rules page.
  • The navigation elements are a list with a hover state but are not interactive elements. So I suggest you add an anchor element inside each li and handle the different element's states here.
  • The same comment above applies to the logo and all the page elements where you handle the hover state with non-interactive elements.
  • You have a ul wrapped by a div in the footer. Think about these elements and what would be the better semantic way to handle them.
  • I've noticed you also are using some empty div elements with the goal of handling background images with images at different breakpoints. Think if all these images are decorative or if some of them are content images. Here I'm sharing an entry about handling responsive images with HTML elements, in case not all are decorative or need to be addressed, not as background-images.
  • Finally, I think client testimonials may be considered a list. If you see the documentation, list allows having flow content elements.

  10

  Marked as helpful