Grid Cards Challenge

Community feedback

• 

  Daniel•260

  @Flashdaniel

  Posted 12 months ago

  Nice work. I like to point a few things:

  1. Use headings h1, h2 ... h6 orderly, not h2 then h2 again like in your code.
  2. box-sizing property is not inherited, so adding it to the body element only affects the body. if your thought was to all the elements. Do this

  *,
  *::before,
  *::after {
    box-sizing: border-box;
  }
  

  You can make your site more responsive by changing the .parent { grid-template-columns: 1fr; } to .parent { grid-template-columns: repeat(auto-fill, minmax( your, 1fr); }.

  Learn to make use of the repeat() function, instead of grid-template-columns: 1fr 1fr 1fr; Do grid-template-columns: repeat(3, 1fr);. And don't use grid-template-rows if it's not necessary like in your case, allow Auto-placement in grid to make that decision for you.

  12

  Marked as helpful
• 

  martbudr•90

  @martbudr

  Posted 12 months ago

  The solution works both on desktop and mobile. The layout looks good on different screen sizes. The code is written in a readable way. The solution looks very similar to the design.

  10
• 

  P

  Matt Pahuta•670

  @MattPahuta

  Posted 10 months ago

  Hi there. Great job on completing this challenge! You've matched the design comp quite well and achieved the layout using CSS Grid, which is the technique this challenge is designed to highlight. I also appreciate you correctly identified the icon images as being purely decorative and gave them appropriate alt values (empty values).

  As a previous reviewer mentioned, headings should always go in order to communicate the structure of the content correctly to assistive technology and bots. This design calls for one h1 tag and four h2 tags.

  Here are a few other things I flagged for you:

  1. Back to the main page heading, this should be one h1 split over two lines. You can wrap half in a strong tag or styled span with display set to block.
  2. You're using a lot of IDs in your html where you should be using classes. It's generally best practice to style elements using classes rather than IDs or by element tag. IDs are better suited for targeting elements via JavaScript, page navigation, linking labels with inputs, and a few other use cases. There is no need for IDs in this particular challenge.
  3. The cards don't need an extra wrapping div. One div within your main tag is plenty. You should always strive to keep your html as simple as possible.
  4. Get in the habit of using a modern CSS reset for all your projects. Andy Bell and Josh Comeau both have excellent resets you can google and use. Place it at the beginning of your styles file.
  5. Font sizes should always be set using rem units, not pixels. You're using a mix of both here, so you'll want to be consistent throughout your code.
  6. Media queries should also be defined using rem or em units, not pixels.

  Again, nice work here and good luck moving forward!

  00