Responsive Landing page using CSS grid and HTML

Community feedback

• 

  T

  Grace•31,270

  @grace-snowPosted 2 months ago

  Hi, this is overflowing badly on my screen where I can't access some of the content. But I'll go over all the recommendations as well as that issue. Make sure you refactor this.

  • all content should be contained within landmarks. What you have as a div with class of "container" should be a main landmark and the attribution should be in a footer. Every page you do should always have a main to hold the main content of the page.
  • make sure your code is indented consistently so it's easier to read and spot bugs. Your code editor can even do this formatting automatically for you with prettier. Set the tab spaces to 2 if possible as that is the most common set up.
  • Look up how and when to write alt text on images. Alt text is content not code and needs to be readable like any visible content would be. The image in this is really important content so it needs a proper alt description. The alt in this needs to say what the image is (QR code) and where it goes (to FrontendMentor.io).
  • h2 is an ideal heading level for this heading, you don't need to change that. You would only use a h1 in a component that would act as the main heading of a page and this component would never be used for that. If you wanted to make the accessibility scanner 100% happy you could add a visually-hidden (sr-only) h1 above the component saying something like "QR code component demo" but this isn't necessary.
  • in the css, space it out! Make it easy to read. Line spaces between declarations and consistent spacing.
  • 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.
  • font size shouldn't ever be declared in pixels. Use rem so that user text size preferences can be respected. More info: https://fedmentor.dev/posts/font-size-px/
  • never limit the height of elements that contain text. Use min-height on the body not height. This allows it to extend taller than the screen when it needs to.
  • the component should have a max width in rem. It doesn't need any width or height (although you can optionally give it a percentage width). Use rem for the max width so that the layout scales correctly for all users including those who have a different text size setting. Doing that means people who have a larger text size will see a larger card, not just big text squished into a narrow container like it would be if you'd used pixels.
  • the img must not have a width or height. All it needs is what is already included in the css reset : max-width 100% and display block. You can optionally give it an aspect ratio of 1 to improve browser performance.
  • use pixels or rem for the padding in the card not percentage. That's a property where you want strict control over the amount of padding.
  • the top property on the component isnt doing anything and you wouldn't want to use it even if it was being applied. It's rare you'll need directional properties at all.

  Marked as helpful

  1

  ---

  

  Joshua Olanrewaju Ikulayo•220

  @SaviourjrPosted 2 months ago

  @grace-snow Thank you very much

  0

  

  T

  Grace•31,270

  @grace-snowPosted 2 months ago

  @Saviourjr once you've updated this one try to apply the same learnings to your blog card. Hopefully a lot of the above will help there too. Then once you've tried that I can take a look at that one too for you.

  Marked as helpful

  1
• 

  Daniel 🛸•44,310

  @danielmrz-devPosted 6 months ago

  Hello there!

  Congrats on completing the challenge! ✅

  Your solution is really impressive!

  I've got a couple of ideas (about how to use HTML better) that could make it even stronger:

  📌 First: Think about using <main> to wrap your main content instead of <div>.

  Imagine <div> and <span> in HTML as basic containers. They're good for holding stuff, but they don't tell us much about what's inside or its purpose on the webpage.

  📌 Second: Consider using <h1> for your main title instead of <h2>.

  It's more than just text size — it's about structuring your content effectively:

  • The <h1> to <h6> tags are used to define HTML headings.
  • <h1> is for the most important heading.
  • <h6> is for the least important heading.
  • Stick to just one <h1> per page – it should be the main title for the whole page.
  • And don't skip heading levels – start with <h1>, then use <h2>, and so on.

  These tweaks might not change how your page looks, but they'll make your HTML code clearer and help with SEO and accessibility.

  Hope that's helpful!

  Keep up the great work!

  Marked as helpful

  1

  ---

  

  Joshua Olanrewaju Ikulayo•220

  @SaviourjrPosted 6 months ago

  @danielmrz-dev Thank you very much sir. I'll work on that. Your suggestions are very helpful sir.

  1