Custom Fonts

Community feedback

• 

  P

  Øystein Håberg•10,710

  @Islandstone89Posted 7 months ago

  Hi! I have some suggestions you might find helpful :)

  HTML:

  • 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" section. Wrap the card in a <main>.

  • Always include an altattribute on images! Decorative images should have empty alt text. This image, though, has meaning, hence it must have proper alt text. Write something short and descriptive, without including words like "image" or "photo". Screen readers start announcing images with "image", so an alt text of "image of qr code" would be read like this: "image, image of qr code". The alt text must also say where it leads(frontendmentor website).

  • In general, you should use class instead of id. This article explains the use cases for the id attribute.

  • Other than the <div> holding the card content, you don't need any divs.

  • Do not use <br> to force text onto a new line. The text should flow naturally, and all styling, including space between elements, should be done in the CSS.

  CSS:

  • Including a CSS Reset at the top is good practice.

  • Add around 1rem of padding on the body, so the card doesn't touch the edges on small screens.

  • You should not use position or transform to center the card, so remove those. To center the card horizontally and vertically, I would use Flexbox on the body:

  display: flex;
  flex-direction: column;
  justify-content: center;
  align-items: center;
  min-height: 100svh;
  

  • Remove all widths and heights. In web design, we want our components to adjust to different screen sizes. Setting fixed sizes in px makes them unable to adapt, and it is something you should rarely do.

  • Add a max-width of around 20rem on the card, to prevent it from getting too wide on larger screens.

  • 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.

  • Since all of the text should be centered, you only need to set text-align: center on the body, and remove it elsewhere. The children will inherit the value.

  • Likewise, put the font-family on the body, and remove it elsewhere. Also, remember to specify a fallback font:font-family: 'Outfit', sans-serif;.

  • Remove the margin on the paragraph.

  • On the image, add display: block and max-width: 100% - the max-width prevents it from overflowing its container.

  Marked as helpful

  2

  ---

  

  P

  Mukarram Haq•270

  @MukarramHaqPosted 7 months ago

  @Islandstone89 You, my dear sir, is the GOAT.

  I knew I was doing a lot of things wrong in this code but I had no idea what they were.

  Thank you for pointing these out. I'll make sure to keep this in mind while building the next project as I don't want to spend too much time on this one.

  Just followed you on GitHub. I would love to build some genuine connections :)

  1

  

  P

  Øystein Håberg•10,710

  @Islandstone89Posted 7 months ago

  @MukarramHaq My pleasure! I followed you back on GitHub :)

  1

  

  P

  Mukarram Haq•270

  @MukarramHaqPosted 7 months ago

  Hi, @Islandstone89

  I have made some changes that you pointed out in my code. Just wanted to let you know and if you have time I would appreciate it if you could look at it. :)

  1

  

  P

  Øystein Håberg•10,710

  @Islandstone89Posted 7 months ago

  @MukarramHaq Good job! A few things to notice:

  HTML

  • Remember that you shouldn't use words like "image" in alt text, since screen readers will read it as an image by default.

  CSS

  • Remove the height on the card. The content should determine its height.

  • Usually display: block and max-width: 100% is placed on img, meaning it works on all images on a page.

  • I would highly recommend to add the following code at the beginning:

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

  This is a standard used in every web project. This article explains the difference between box-sizing: content-box and box-sizing: border-box.

  • Writing .card in front of the elements adds to the specificity of the selector, so that's something to be aware of. It's usual to give elements a class, which has a lower specificity than an descendant combinator.

  Hope this makes sense - keep up the good work :)

  Marked as helpful

  1

  

  P

  Mukarram Haq•270

  @MukarramHaqPosted 7 months ago

  @Islandstone89 Thank you so much for these helpful tips.

  I feel like my code is a lot cleaner now and it makes so much sense.

  Appreciate you taking your time out to look at my code! :)

  1

  

  P

  Øystein Håberg•10,710

  @Islandstone89Posted 7 months ago

  @MukarramHaq Happy to help :D

  1
• 

  P

  Koda👹•3,830

  @kodan96Posted 7 months ago

  Hi there! 🙌

  Usually there's no need to use id-s, especially not in small projects like this. The main reason for classes is to define a reusable chuck of code that you can easily apply to multiple elements.

  The problem with IDs is that one ID can only be applied to one element, so they are not reusable on the same page.

  There are use cases for them, but it's better to stick with classes most of the time.

  Hope this was helpful🙏

  Good luck and happy coding! 💪

  2

  ---

  

  P

  Mukarram Haq•270

  @MukarramHaqPosted 7 months ago

  @kodan96 Thank you for the feeback.

  I was wondering how and when to use IDs or classes. This pointed me to the right direction. Appreciate your help.

  I followed you on GitHub and Twitter. I would love to connect and from some genuine connections along the way :)

  2