Gabrielle Parfum HTML and CSS

Community feedback

• 

  Lucas 👾•104,400

  @correlucasPosted about 2 years ago

  👾Hello @Marianellag1, Congratulations on completing this challenge!

  Your solution its almost done and I’ve some tips to help you to improve it:

  Using <picture> you’ve more control over the elements and its better than using the product image as <img> or background-image. Look that for SEO and search engine reasons it isn't a better practice to import this product image with CSS since this will make it harder to the image. You can manage both images inside the <picture> tag and use the html to code to set when the images should change setting the device max-width depending of the device (phone / computer) Here’s a guide about how to use picture: https://www.w3schools.com/tags/tag_picture.asp

  ✌️ I hope this helps you and happy coding!

  Marked as helpful

  1

  ---

  

  Mgamboa•220

  @Marianellag1Posted about 2 years ago

  @correlucas Hello! Thank you so much! I didn't even know there was a picture element. I'll will use it for future reference for pictures!

  0
• 

  Hyron•5,870

  @hyrongennikePosted about 2 years ago

  Hi @Marianellag1,

  Nice job on the challenge

  If you want to center the card on the page, the margin-top: 12% on div inside the container div and replace your body rule with the following one.

  body {
      margin: 0;
      font-family: -apple-system,BlinkMacSystemFont,"Segoe UI",Roboto,"Helvetica Neue",Arial,"Noto Sans",sans-serif,"Apple Color Emoji","Segoe UI Emoji","Segoe UI Symbol","Noto Color Emoji";
      font-size: 1rem;
      font-weight: 400;
      line-height: 1.5;
      color: #212529;
      text-align: left;
      display: flex;
      justify-content: center;
      align-items: center;
      min-height: 100vh;
  }
  

  Hope this is helpful

  Marked as helpful

  1

  ---

  

  Mgamboa•220

  @Marianellag1Posted about 2 years ago

  @hyrongennike Hello! I was having such a hard time centering horizontal and vertical. Especially since I used bootstrap, I kept getting confused. This is very helpful, thank you very much! I think I over complicate myself, when really things are actually more simple.

  0
• 

  Kawsar Ahmed Fahad•2,660

  @faha1999Posted about 2 years ago

  Hello, Mgamboa Congratulations on finishing this project. It's lovely and great on the whole! Just a little tip:

  • You might want to use semantic tags like the <main> to wrap your code, instead of div. like

  <main class="container">
  </main>
  
  <footer class="attribution fixed-bottom">
  </footer>
  

  You could also make use of <section> and/or <article> semantic tags. This would help improve accessibility.

  • add favicon <link rel="icon" type="image/png" sizes="32x32" href="./images/favicon-32x32.png">

  • a small transition & hover to the button will be more attractive.

  • media issues

  @media only screen and (max-width: 600px) {
  .row{
  /*margin-right: -15px; */
      /* margin-left: -15px;*/
  }
  
  .col-md-6:first-child {
  height: 250px;
  }
  
  .col-md-6:first-child  img{
  background-image: URL(./Assets/images/image-product-mobile.jpg);
  }
  
  

  • This is my solution of this challenge live, repo

  I hope it will work. Happy coding.

  Marked as helpful

  1

  ---

  

  Mgamboa•220

  @Marianellag1Posted about 2 years ago

  @faha1999 Hello! Thank you very much! I will definitely keep the semantic tags in mind. It does make sense. I will in the future make the changes with your suggestions. I appreciate the help!

  0