html5, css and flexbox

Community feedback

• 

  T

  Grace•31,270

  @grace-snowPosted about 2 months ago

  Hi, as we said this has quite a few foundational issues you need to fix before moving on to anything else. Don't rush, pay attention to small details, take feedback on board and you will be able to progress onto more complex challenges before you know it.

  • it's better for performance to link fonts in the html head instead of css imports.
  • look up how and when to write alt text on images. Img alt is human readable content, not code like a class name. The image in this is really important content, so it must have a proper alt description. The alt needs to say what the image is (QR code) and where it goes (to FrontendMentor.io).
  • when building single component demos like this it's important to consider the context of where and how that component will be used in a real site. This is a skill you have to build now so that you're ready for building components in js frameworks later. This is a card. It would never be used to serve the main heading on a page, so you know it shouldn't have a h1. Use a lower importance heading level like h2 instead.
  • don't forget to update your footer link to your frontend mentor profile page or github profile page.
  • get into the habit of including a full modern css reset at the start of the styles in every project. Andy Bell or Josh Comeau both have good ones you can look up and use. Read about them and understand why they have included what they have.
  • I think it's really confusing to use a class named "main" for this card. It's not on the main element so it makes the css confusing to read in isolation. I recommend you change that class name to something that makes it clear that is the component style hook.
  • font size should never be in pixels. Use rem instead. See https://fedmentor.dev/posts/font-size-px/.
  • it's fine to change colours on these challenges if you really want to but remember you will never be able to do that in a real job. Designers would get very frustrated.
  • never limit the height of elements that contain text, including body or main. This solution is being cut off (broken!) at some screen sizes and orientations because you've used height: 100vh. Change that to min-height instead so that the browser knows the element is allowed to grow beyond the screen height when it needs to.
  • it's the same principle on the card component. This must not have a height. It doesn't need a min-height either. All you've done there is create a fragile component that will break as soon as content changes, content is translated, or users change text size settings. There is no reason to set that height at all. Let the browser do it's job and decide what height is needed based on the content inside.
  • the card component must not have a width either. Setting explicit widths is almost always a bad idea. The only exceptions are small images, icons, logos and decorative elements. All this card needs is a max-width in rem. This tells the browser it can go narrower if it needs to like on small screens. And using rem means the layout scales correctly for all users including those who have a different text size setting in their browser or device.
  • the image doesn't need a width either. All it needs for it's sizing is what's already included in the css reset. You can optionally give it a width of 100% if you want but that's not really needed.
  • font weights should be defined by number value not keyword as this can differ between browsers.
  • make sure you define explicit font styles including line height rather than relying on browser defaults that can differ between browsers.
  • set text-align center on the card so it can be inherited by the heading and paragraph in one go.
  • your whole solution is smaller than the design because the text is too small. The body font size has been provided in the style guide for a reason. Convert to rem and use it. That's the size the paragraph should be. Once you've set that and applied appropriate font styles to that paragraph you can use that to help you work out the sizes of everything else (unless you have figma access and then you obviously don't need to guess).
  • don't position the footer absolutely. It's overlapping the card in some screen sizes and orientations.
  • don't forget the small details like the box shadow on the card. It will be hard for you to get right when you've changed the background colour but try.

  I hope this helps and I hope it helps you realise how much there actually is to consider in these challenges. Take time to get the foundations right!

  3

  ---

  

  T

  Gwenaël Magnenat•1,290

  @gmagnenatPosted about 2 months ago

  @grace-snow I’ll save this one and screenshot it :) probably the most complete feedback on a qr code challenge I saw here. I salute your dedication

  2

  

  mshah007•20

  @mshah007Posted about 2 months ago

  @grace-snow "This is a card. It would never be used to serve the main heading on a page, so you know it shouldn't have a h1."- i used an h1 cos the website said i needed an h1 after i submitted the solution

  0

  

  T

  Grace•31,270

  @grace-snowPosted about 2 months ago

  @mshah007 that's an automated giving a warning (not an error - note the difference) that you need to check the heading level because every page should have a h1. But as already stated, this isn't a page, it's a single component demo. So should have a h2 not a h1. No h1 is necessary on this.

  0
• 

  leviipope•50

  @leviipopePosted about 2 months ago

  You like this color better?

  0