partially responsive(lol) landing page with css grid and flexbox

Community feedback

• 

  Hassia Issah•50,650

  @HassiaiPosted almost 2 years ago

  Replace <h2> with <h1> and <h6> with <p> to fix the accessibility issues. There is no need for <span> in the html and its styling in the css.

  To center the section on the page, add min-height: 100vh to the body.

  There is no need to give the section a height value. increase the width and padding value of the section for it to be equivalent to the design. width: 320px , padding: 15px.

  Give the img a max-width of 100% instead of a width and height values.

  Give h1 and p the same font-size of 15px, text-align: center, the same margin-left, margin-right and margin-top values. Give p a margin bottom value.

  Use relative units like rem or em as unit for the padding, margin, width values and preferably rem for the font-size values, instead of using px which is an absolute unit. For more on CSS units Click here

  Hope am helpful.

  Well done for completing this challenge. HAPPY CODING

  Marked as helpful

  0
• 

  Melvin Aguilar 🧑🏻‍💻•61,140

  @MelvinAguilarPosted almost 2 years ago

  Hello there 👋. Good job on completing the challenge !

  I have some suggestions about your code that might interest you.

  • Why did you add "css/" to the link of your styles in the most recent changes to the repository? There is currently no folder with that name, and no style is visible in your solution. :(. Update to <link rel="stylesheet" href="style.css">

  • Using visibility: hidden in your solution only hides the title, it does not remove the space that the element would occupy; all the space above the component is for the entire height of the title.

    To completely hide the title and prevent it from taking up space in your solution, you should copy these styles for your Sr-only class:

    .Sr-only {
      clip: rect(0 0 0 0); 
      clip-path: inset(50%);
      height: 1px;
      overflow: hidden;
      position: absolute;
      white-space: nowrap; 
      width: 1px;
    }
    

  Alt text 📷:

  • The alt attribute should explain the purpose of the image. Uppon scanning the QR code, the user will be redirected to the frontendmentor.io website, so a better alt attribute would be QR code to frontendmentor.io

    If you want to learn more about the alt attribute, you can read this article. 📘.

  CSS 🎨:

  • To center an element vertically, you should use a height to its container. In this case it is recommended to use "min-height: 100vh" so that it occupies 100% of the viewport height. e.g.: body { min-height: 100vh; }

  I hope you find it useful! 😄

  Happy coding!

  Marked as helpful

  0

  ---

  

  krisskross69•70

  @krisskross69Posted almost 2 years ago

  @MelvinAguilar i added the ccs/ in the stylesheet because i had problems running the code through github it only worked when i added the css/.

  0