Skip to content
OverviewSolutions
7
Comments
31

Pipow

@PipouwPieuwToulouse / France500 points

I’m a mysterious individual who has yet to fill out my bio. One thing’s for certain: I love writing front-end code!

Latest solutions

View more solutions

Latest comments

  • Pipow•500
    @PipouwPieuwPosted almost 2 years ago

    You did well, I think it looks good on desktop and mobile!

    That being said, there's an horizontal scroll on desktop. The section linha-5-imgs seems to be at fault.

    In the footer, I guess social media icons need to be linked to their respective social media page.

    Your interactive elements (links and buttons mostly) would look even better if you put hover effects on them, like you did on the Contact button. You may also set a transition so the effect would be more pleasing to the eye :)

    Well done overall!

    Marked as helpful
    1
  • Pipow•500
    @PipouwPieuwPosted about 2 years ago

    Your page looks really neat!

    I was wondering about the loading="lazy" error in the HTML validation report. I got the same errors when I submitted my solution so I just removed the loading attributes from all my images. Does this mean the loading attribute should not be used or is the report tool simply not up to date ? Seems a bit odd.

    0
  • Pipow•500
    @PipouwPieuwPosted about 2 years ago

    Hello and well done completing this challenge!

    To answer your question, in this case you can use either flexbox or grid. I don't think there is any good practice that makes one of these better than the other.

    Flexbox didn't work in your example because you also need to make sure that your element's height is at least equal to the window's height, as you did in your grid example. Adding min-height: 100vh should work. You would also need to set flex-direction: column in this case because of the .attribution element being a sibling of .card-container:

    body {
    display: flex;
    align-items: center;
    justify-content: center;
    flex-direction: column;
    min-height: 100vh;
}

    Hope this helps. Keep coding :)

    Marked as helpful
    1
  • Pipow•500
    @PipouwPieuwPosted about 2 years ago

    Hello and well done completing this challenge, your component look good!

    I have a few suggestions to further improve it:

    • To handle the responsive behaviour of the image, instead of using two <img> you could use a picture element.
    • A good alt attribute shouldn't just repeat the name of the product. Here you should really describe the image as you would describe it to someone that can't see it.
    • You could use a greater breakpoint to display the mobile version. Try viewing your page with a window width set to ~ 400-500px, you'll see it's all cramped.
    • To make the button hover effect a little better you could use a transition (0.2s / 0.3s is usually enough).

    Apart from these minor matters, you did a really good job! Keep going :)

    0
  • Pipow•500
    @PipouwPieuwPosted about 2 years ago

    Hello and well done completing this challenge!

    This image is not purely decorative and is important to the page content, therefore it shouldn't be displayed as a background. I would use an <img> here, or even better a <picture>.

    This may even fix your sizing issues. :)

    Also you put a <h2> right before the <h1> which is semantically incorrect. You could wrap the word Perfume inside a <p> or maybe a <span> instead.

    Keep going, happy coding!

    Marked as helpful
    0
  • Pipow•500
    @PipouwPieuwPosted about 2 years ago

    Hello! Well done completing this challenge, your page looks really neat! 🎉

    To answer your question, I may be wrong but I don't think hover effects have any negative impact on performance. They are good for the user experience though, so ideally every interactive element should have one :)

    Keep coding! 🧑‍💻

    0
View more comments