Four card feature

Community feedback

• 

  T

  Grace•31,090

  @grace-snowPosted over 2 years ago

  Hello

  This looks good

  Here are some small suggestions:

  /* style.css | https://giulo25.github.io/Four-card-feature-section/style.css */
  
  main {
    /* padding: 8rem 16rem; */
    padding: 1rem;
  }
  
  .text {
    /* max-width: 50rem; */
    max-width: 50ch;
    note: good to put max width in ch for text content like this where the purpose is to prevent line length getting too long for readability
  }
  
  .cards {
    /* grid-template-rows: 1fr 1fr; */
    /* max-width: 120rem; */
    grid-template-rows: auto;
    max-width: 100rem; // or whatever the grid max width should be
    margin: auto;
  }
  
  .img img {
    note: decorative images should have empty alt value;
    note: img in html should have width and height attributes for better performance;
  }
  
  

  Marked as helpful

  0

  ---

  

  Simone•170

  @Giulo25Posted over 2 years ago

  @grace-snow

  Hi Grace, thanks for your feedback. I didn't understand why I have to set padding 1rem in the main. With padding 1rem it's different from the design.

  I set vertical padding 8rem now and it looks ok to me.

  0

  

  T

  Grace•31,090

  @grace-snowPosted over 2 years ago

  @Giulo25 ah I also removed the 62.5% html font size hack as I hate introducing potential accessibility issues like that

  0