Grid Cards Challenge

Community feedback

• 

  Daniel•260

  @FlashdanielPosted 4 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.

  Marked as helpful

  1

  ---

  

  kendo-desu•70

  @kendo-desuPosted 4 months ago

  @Flashdaniel Thank you for your comment and all the tips, I find them very helpful as I'm a complete beginner who has just started learning to code last month. I understand all the points you raised except the repeat(auto-fill, minmax(your, 1fr); part. Is it possible for you to elaborate more on that?

  0

  

  P

  Matt Pahuta•580

  @MattPahutaPosted about 2 months ago

  @kendo-desu

  I'm no Grid guru but I have found Kevin Powell's content immensely useful for tackling most every layout challenge I've come across. He has a lot of good stuff on Grid and flexbox, but here's a quick and simple video that I think will send you on the right path. Cheers!

  1
• 

  martbudr•90

  @martbudrPosted 4 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.

  1
• 

  P

  Matt Pahuta•580

  @MattPahutaPosted about 2 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!

  0