Just HTML and CSS

Community feedback

• 

  T

  Grace•31,230

  @grace-snowPosted 10 months ago

  Here is some feedback as promised

  1. Don't style on IDs. They have very specific purposes and cause unwelcome high CSS specificity that becomes a nightmare on large projects. Break that habit as soon as possible and try to stick to single class selectors in CSS as much as possible. Additionally in this case, this is a card component so there is the potential (liklihood) that it would be used multiple times on one page. But IDs must always be unique. So you will cause instant failures on things like accessibility scans if you style the ID like this. Here is a post all about what the ID attribute is really meant to be used for
  2. Alt text must be a proper description of the image when it is meaningful content like this. This image must say what the image is (QR code) and where it goes (to FrontEndMentor.io). Here is a post about how and when to write good alt text on images
  3. Related to point no 1, as this is a card component it would not contain the page title (h1). You should be using h2 or even h3 for this depending on where you think it would sit on a page. Heading order is extremely important for SEO and accessibility.
  4. Moving onto styling: Never limit the size of the body. It's fine to have min-height: 100vh; (or 100svh / 100dvh etc) but should never have its height or width limited.
  5. The component must not have a height or width. It's essential you never limit the height of elements that contain text within them somewhere as all that can do is cause overflow when editors change the length of content or users change their base font size settings. You should never set width on components like this either as that will also break when users have different font sizes. Instead, set one max-width in rem on the component and no height. Let the browser do its job and work out the necessary height for you. By setting max width in rem, you allow the component to shrink when needed (like on a small screen) and prevent it from growing too large, but users with a larger font size will also benefit by getting the exact same design experience as everyone else.
  6. Do not set large margins or paddings to try and create a layout. This is not how you center a component in the viewport! Instead, use min-height on the body as advised above, along with either flex or grid properties. Here is a guide to centering with CSS
  7. Once the above is fixed, the component will need a small amount of margin (or the body can have a small amount of padding) just so the component can never hit the screen edges (e.g. on small screens)
  8. The whole card should have padding. Then the items within it only need some vertical margins
  9. Always include a modern CSS reset at the very start of the styles in every project. Here is a great reset by Andy Bell. Make sure you read about it carefully and understand what it's doing. Occasionally you may want to change / add bits to your reset.
  10. The img must not have a width or height. A standard part of most CSS resets is setting img elements to display: block; max-width: 100%. That, plus some border radius is all this img needs. You can optionally add width: 100% to it, but it's not strictly necessary.
  11. You should not be using em so much. It's good you're trying to use a responsive unit, but em compounds so should not be the unit of choice unless you specifically want a CSS property to scale with the element's font size. For example, em is a good choice of unit for things like buttons or margin top on elements within a blog article, as in those cases you would want the spacing to scale with the text size. In most other cases you will want to use rem, or px if you never want a value to change. The important point for now is only use em very intentionally and if in doubt, use rem.
  12. Never ever ever ever (I cannot stress this enough!) use pixels for font-sizes! Here is an explainer about why font-size in px immediately makes your solution inaccessible. It's a critical level A (basic) WCAG accessibility failure.
  13. This challenge does not need a media query. However, in future challenges, you should be working mobile first (making the mobile design the default styles) and should be writing media queries using rem or em, never pixels. Here is a post about how to use media queries well. The mobile first approach is important for sites with good performance, and a well established convention across the industry, so get into the habit of following it now.
  14. HTML pages should be called index.html. Lots of hosts will not allow you to host pages with names like this. Usually the index.html file goes inside a directory with the page name, then the index.html does not need to be appended to the url because the browser is expecting it by default.
  15. Not essential for this one but worth being aware of - it is more performant to load local fonts in the HTML head than from 3 rd parties in CSS or via CSS imports

  Marked as helpful

  3
• 

  Georgi Tashev•550

  @xaocccPosted almost 2 years ago

  On my browser it looks exactly as in the design, i used all style guide, but still, the h1 size was 1px bigger which costed me 1 line of 23 pixels. I've learned my lesson.

  0

  ---

  

  T

  Grace•31,230

  @grace-snowPosted 10 months ago

  @xaoccc This is not the goal! The goal of front end development is to build robust accessible solutions that work for all users on all screen sizes with all accessibility settings or assistive technology and honour the designers intention.

  It's worth reading this post by Josh Comeau

  0