HTML + CSS

Community feedback

• 

  Hassia Issah•50,650

  @HassiaiPosted over 1 year ago

  Replace <div class="container"> with the main tag and <p class="card-heading"> with <h1> to fix the accessibility issues. click here for more on web-accessibility and semantic html

  To center .card on the page using grid, replace the height in the body with min-height: 100vh.

  for a responsive content which wont require a media query for this challenge, replace the width in .card with max-width and reduce it value. max-width: 320px. Give the img a max-width of 100% instead of a width and height value.

  Give .text a margin value for all the sides, text-align: center and a font-size of 15px which is 0.9375rem, this will be the font-size of both p and h1.

  Use relative units like rem or em as unit for the padding, margin, width values and preferably rem for the font-size values, instead of using px which is an absolute unit. For more on CSS units Click here

  Hope am helpful.

  Well done for completing this challenge. HAPPY CODING

  Marked as helpful

  1

  ---

  

  Pfigdev•10

  @pfigzPosted over 1 year ago

  @Hassiai Yes, I did not pay attention to accessibility with my markup, thank you for pointing that out and providing links. Good suggestions as well, I will incorporate them.

  1
• 

  Pipow•500

  @PipouwPieuwPosted over 1 year ago

  Hey there,

  You did a good job! It looks neat. :)

  A few suggestions:

  • To make sizing easier, you can apply box-sizing: border-box on everything like so :

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

  This makes padding and stuff included in the width / height calculations. You may have noticed that your .card is 280px wide even if you gave it a width of 250px. This is because its padding of 15px is then added to the width. border-box prevents this.

  • Instead of giving a fixed width: 250px to your card it would be better to give it max-width: 250px. max-width makes elements more responsive as it allows them to shrink if necessary. In this case I also had to add width: 100% so the card would size correctly, I'm not sure why. It may be because you used display: grid.

  • You can then get rid of the breakpoint of min-width: 300px you used, because the card will never get larger than the screen now.

  • I also would set min-height: 100vh instead of height: 100vh on the body. This way it would be at least as big as the screen but could get bigger if needed.

  • HTML landmarks are important. You should include a <main> tag around your content and use headings. For exemple the text Improve your front-end skills by building projects could be wrapped inside a <h1>.

  Overall, well done! You did a good job :)

  Marked as helpful

  1

  ---

  

  Pfigdev•10

  @pfigzPosted over 1 year ago

  @PipouwPieuw Excellent feedback - thank you for the suggestions, especially the border-box. Is this something you incorporate into your styles as a default? I feel like I will going forward.

  0

  

  Pipow•500

  @PipouwPieuwPosted over 1 year ago

  @pfigz You're welcome :) Yes, I always include this in my reset file, on every project. It just makes life much easier!

  Marked as helpful

  1