I did not use any property for this yet.

Community feedback

• 

  Rameesha Padmatilaka•220

  @rameesha0126Posted about 2 months ago

  Replace <div class="container"> with the main tag (<main class="background"). Inside the main tag add a div (<div class="card-component">) that will hold the <img/> and the text sections. Do not use too many <div> tags. For the heading use <h1> tags, and for the paragraph use <p> tags to fix accessibility issues. Fix the issues with <b> tags in place. To center .background on the page, add min-height:100vh; display: flex; align-items: center: justify-content: center; or min-height:100vh; display: grid place-items: center to the body.

  Give .background a background of white a padding value for all the sides, give the <img> a max-width of 100% instead of a width. For responsive content, replace the width of .background with max-width and change the height's value to auto.

  Give .card-component padding values and a fixed width value.

  Use rem or em as unit for the padding, margin, width and preferably rem for the font-size.

  Marked as helpful

  0
• 

  STEPHEN Obayuwana•340

  @xaintobasPosted 9 months ago

  Hi @MaryAMsendoo,

  Nice work on your design! Here are a few opportunities to enhance it:

  -HTML Structure and Semantics:

  Your code uses nested <div> elements which can make the structure harder to follow.

  I will suggest you use <main> and <section> elements. It is a great improvement in terms of semantic HTML. This makes the content more meaningful and accessible.

  <main>
   <section class="card">
      <img src="https://maryamsendoo.github.io/firstchallenge/img1.png" alt="">
      <h1>Improve your front-end skills by building projects</h1>
      <p>Scan the QR code to visit Frontend Mentor and take your coding skills to the next level</p>
     </section>
  </main>
  

  -Flexbox for Centering:

  Your code uses margin for centering elements, which is not as flexible. I will suggest you use Flexbox in the main element to center the content. This makes the layout more responsive and easier to manage.

  main {
    display:flex;
    flex-direction:column;
    justify-content:center;
    align-items:center;
    width:100vw;
    height:100vh;
    background-color: hsl(212, 45%, 89%);
  }
  

  -Styling Improvements:

  Your CSS has some issues, such as width: 50%px for the .container div which I believe is a typo. You should consider fixing this. But here are some improvements you should also consider:

  Using max-width: 320px to ensure the card does not become too wide on larger screens.

  .card {
    background-color:white;
    max-width:320px;
    text-align:center;
    border: 15px solid white;
    border-radius: 14px;
  }
  

  Consider adding an h1 element and also styling your image for responsiveness

  h1{
    font-size:1.6rem;
  }
  
  img{
    width:100%;
  }
  

  I hope you find my feedback valuable and helpful.

  If yes, I would appreciate it greatly if you could mark my comment as helpful!

  Keep coding

  0

  ---

  

  MaryAMsendoo•10

  @MaryAMsendooPosted about 2 months ago

  @xaintobas thanks so much and sorry just getting to receive this mail now. I have improved on my design and seeing this on time would have help more better. https://maryamsendoo.github.io/ Here is my portfolio.... I'll want more reviews

  0