basic html - css

Community feedback

• 

  P

  Øystein Håberg•12,740

  @Islandstone89Posted 24 days ago

  Hey, well done.

  Here are some tips to improve your solution even further - good luck :)

  HTML:

  • Don't use words like "Image" in the alt text. Screen readers start announcing images with "image", so your alt text, "Card Image" would be read like this: "image, Card Image". The alt text must also say where it leads(the frontendmentor website). A good alt text would be "QR code leading to the Frontend Mentor website."

  • "Improve your front-end skills by building projects" is a heading, not a paragraph. I would change it to a <h2> - a page should only have one <h1>, reserved for the main heading. As this is a card heading, it would likely not be the main heading on a page with several components.

  CSS:

  • Make a habit of including a modern CSS Reset at the top of the stylesheet.

  • I recommend adding a bit of padding, for example 16px, on the body, to ensure the card doesn't touch the edges on small screens.

  • On the body, change height to min-height: 100svh - this way, the content will not get cut off if it grows beneath the viewport.

  • Remove all widths and heights in px. We rarely want to give a component a fixed size, as we need it to grow and shrink according to the screen size.

  • We do want to limit the width of the card, so it doesn't get too wide on larger screens. To solve this issue, give the card a max-width of around 20rem.

  • font-size must never be in px. This is a big accessibility issue, as it prevents the font size from scaling with the user's default setting in the browser. Use rem instead.

  • letter-spacing must also not be in px. You can use em, where 1em equals the element's font size.

  • Paragraphs have a default value of font-weight: 400, so there is no need to declare it.

  • To create the space between the image and the edge of the card, set padding on all 4 sides of the card: padding: 16px;.

  • On the image, remove margin-top. Add display: block, height: auto and max-width: 100% - the max-width prevents it from overflowing its container. Without this, an image would overflow if its intrinsic size is wider than the container. max-width: 100% makes the image shrink to fit inside its container.

  Marked as helpful

  1

  ---

  

  Ahmet Turkoglu•30

  @koygocuren881Posted 23 days ago

  @Islandstone89 Hello! Thank you very much for your feedback and recommendations. I reviewed each point in your comment and tried to implement them. SVH, REM, and EM are new units for me. Using decimal values like 31.1875 REM is a bit intimidating, but everything seems to be working well. I understand that there are times when rounding values up or down is necessary, but it’s not always required. thanks again for your time and effort.

  1

  

  P

  Øystein Håberg•12,740

  @Islandstone89Posted 23 days ago

  @koygocuren881 Good job!

  You're almost there, just a few more things to correct:

  • Every webpage needs a <main> that wraps all of the content, except for <header> and footer>. This is vital for accessibility, as it helps screen readers identify a page's "main" content. Wrap the card in a <main>. I think you removed the <main> in your update, since I didn't mention it in my first post.

  • On body, you have set the value for padding twice. Since padding: 0 comes after padding: 16px, that declaration wins. We do want a little bit of padding, so remove padding: 0.

  • On the card, remove:

    • min-height, it is not needed.
    • box-sizing: border-box, as that is already set on all elements with the universal selector *.
    • position: relative - we don't need any positioning properties for this challenge.

  • letter-spacing: 0px should be written as letter-spacing: 0 - whenever the value is zero, we don't need to include any units.

  • On the image, replace width: 100% and max-width: 18rem with max-width: 100%.

  0
• 

  Aditya Narayan•40

  @AdityanaraynPosted 24 days ago

  This is nice

  0