Skip to content
OverviewSolutions
8
Comments
6
Ebere Ndukwu
@eby-coder

All comments

  • Ramita•10
    @ramitaaroraSubmitted about 2 years ago

    Definitely still struggling with the sizing issue. Feedback welcome!

    QR Code Challenge

    2
    Ebere Ndukwu•180
    @eby-coderPosted about 2 years ago

    Hi Ramita. Congrats on completing this challenge. A simple way of dealing with your sizing issue is to set the width of the card.

    .qr-code {
    width: 400px;
}

    As you have already set the image width to 100%, reducing the width of the card would also make the image width to reduce.

    You should also remove the height: 400px; that you set for the image in your HTML file as this would increase the image size.

    In addition, to further reduce the image size, you can increase the padding of the card:

    .qr-code {
    width: 400px
    padding: 20px;
}

    These should solve the problems you're having with the image size. I hope this is helpful.

    Marked as helpful
    1
  • Oishik Biswas•70
    @pippal5536Submitted about 2 years ago

    I tried to make the project without many lines of code but I am having a feeling that the project could be done with way lesser code. Please suggest me lesser code if there are any.

    Mobile First Approach with css flex box.

    3
    Ebere Ndukwu•180
    @eby-coderPosted about 2 years ago

    Congrats on completing this challenge. As for your question, I think one way of reducing the code and making it effective is to use the universal selector(*) instead of listing out all your html elements.

    html,
body,
div,
span,
applet,
object,
iframe,
h1,
h2,
h3,
h4,
h5,
h6,
...
mark,
audio,
video {
    margin: 0;
    padding: 0;
    border: 0;
    font-size: 100%;
    font: inherit;
    vertical-align: baseline;
}

    The universal selector selects any and all types of elements in an HTML page. It is useful when we want to select all the elements on the page. So, instead of this⬆️, you can use this⬇️

    *  {
    margin: 0;
    padding: 0;
    border: 0;
    font-size: 100%;
    font: inherit;
    vertical-align: baseline;
}

    I hope this is helpful. Congrats once more on your solution.

    Marked as helpful
    1
  • Unzila Maqsood•60
    @Unzila02Submitted about 2 years ago

    this is my second challenge solution and I improve a lot thanks for my mentor and also how guide me in comment. I face some difficultly but I solve it.

    Pure Html5 & Css3

    2
    Ebere Ndukwu•180
    @eby-coderPosted about 2 years ago

    Hi👋. Congrats on completing this challenge. I'll like to point out a few things that I think may help.

    Firstly, it is the best practice to make your code as effective as possible. Therefore, any div or style that is not needed should be removed rather than put in comments.

      <div class="attribution">
    Challenge by <a href="https://www.frontendmentor.io?ref=challenge" target="_blank">Frontend Mentor</a>. 
    Coded by <a href="unzila.dorik.io">Unzila02</a>.
  </div>

    It will be best to remove any unnecessary classes so that your code would look clean.

    In addition, while your desktop view is beautiful, the mobile view doesn't look nice. This is because you set the width of ur container in percentage.

    .container{
    margin: 10% auto;
    width:22%;
    background-color:hsl(0, 0%, 100%) ;
    border-radius: 15px;
}

    This width: 22% sets the width of the container to 22% of the screen's width, hence, while it may look okay when viewed with a laptop, when you use a phone, it looks bad.

    The best way to set your width is to use pixels, so that the width is fixed regardless of what the screen size may be.

    I hope this helps.😁

    1
  • Ebere Ndukwu•180
    @eby-coderPosted about 2 years ago

    Try this. HTML src attribute is case-sensitive. The folder for your images is saved as Images and you set the url to images.

    <img src="images/icon-music.svg" alt="icon-music">

    Simply change the src="images/icon-music.svg" to src="Images/icon-music.svg"

    I hope this helps. Also, congrats on completing the challenge. Your solution is beautiful.

    Marked as helpful
    0
  • Ebere Ndukwu•180
    @eby-coderPosted about 2 years ago
    .product {
    background-image: url(../images/image-product-mobile.jpg);
    background-size: 20rem;
    background-repeat: no-repeat;
    height: 13.9rem;
    border-top-right-radius: 10px;
    border-bottom-left-radius: 0;
}

    I think I can help you out here. Firstly, I noticed that you set the images as background images. I won't recommend doing this as the images were not set to be background images. So, instead of doing this, I'll recommend this method.

    <img src="image-product-desktop.jpg" class="desktop-img">
 <img src="image-product-mobile.jpg" class="mobile-img">

    .mobile-img {
        display: none;
}

.desktop-img {
      display: block;
      width: 50%;
}

@media screen and (max-width: 900px){
     .mobile-img {
         width: 100%;
         display: block;
         height: 50%;
     }

     .desktop-img {
         display: none;
     }
}

    Putting it this way would make it appear better and not cause possible problems in your page.

    In addition, if you set a border-radius for the card and the images you put overflow and cover the border-radius you set, you can simply use the overflow property in CSS as so:

    .main {
     overflow: hidden;
}

    This would make the overflowing to be hidden. Use this instead of re-adding the border-bottom-left-radius or border-top-right-radius to your image.

    I hope this helps.

    Marked as helpful
    0
  • Ebere Ndukwu•180
    @eby-coderPosted about 2 years ago

    Your page is very beautiful and I love the hover styles you added. Wonderful job. I just wish that your CSS code was more readable and structured and not compressed. I had a hard time reading your styles and learning them.

    Marked as helpful
    0