Skip to content
OverviewSolutions
8
Comments
4
David S
@davsanter1

All comments

  • David S•150
    @davsanter1Posted 5 months ago

    Hi! Your code looks good! The only thing that came to mi mind was to adjust the semantic HTLM replacing the <div> for the container, like <main>.

    Also the <ul> and <li> for the social links with <nav> or a <div>, and use <a> tags instead of buttons for links. Example: <nav> <a href="#">GitHub</a> <a href="#">Frontend Mentor</a> <a href="#">LinkedIn</a> <a href="#">Twitter</a> <a href="#">Instagram</a> </nav> Hope it helps! :)

    0
  • David S•150
    @davsanter1Posted 5 months ago

    Hi! Your code looks great!

    The only feedback that came to my mind is that we should ensure alt attributes for images are descriptive. Replace "Logo" and "Profile Picture" with more specific descriptions like "HTML & CSS foundations logo" and "Greg Hooper's profile picture" respectively.

    Greetings!

    Marked as helpful
    0
  • Rebeka•170
    @BekiaDSubmitted 7 months ago
    What are you most proud of, and what would you do differently next time?

    It was minimal HTML, and very little CSS so the skeleton went pretty okay.

    What challenges did you encounter, and how did you overcome them?

    It should have taken a shorter time than it did, but every time I use flexbox I learn a bit more.

    What specific areas of your project would you like help with?

    I am glad I was easily able to center the content, though I feel I have way too much margin.

    Responsive(?) QR code component - practicing flexbox

    2
    David S•150
    @davsanter1Posted 7 months ago

    Hi your code looks great! Tried to find something to provide a feedback but I could find anything, great job!

    1
  • Kyle Deguzman•110
    @CSE-KyleSubmitted 8 months ago
    What are you most proud of, and what would you do differently next time?

    I'm happy that I was able to get the design to look good with the mobile first approach.

    What I would do differently next time is to set mobile first design to be the default design.

    What challenges did you encounter, and how did you overcome them?

    The hardest part for me was figuring out how to switch between image sizes based on either desktop or mobile design.

    I had to reach out to the frontend mentor community for help and also look at others code on how they did it to get an idea how it should be done.

    What specific areas of your project would you like help with?

    In the design, the desktop design still doesn't look accurate.

    I'm having trouble with the right side of the card component to resonate with how the final design should look like, I still need some help with that.

    Please do look over the code in my repository and the live URL and provide me any feedback with how I can improve this project, thank you.

    product preview card component

    1
    David S•150
    @davsanter1Posted 8 months ago

    Hello, your design looks great!

    The reason of the problem with the right side is that you set the width of the "div img-container" to 100%, instead of 50%, but not only that, there is not width for the "div info" so it makes the right side to take as much space as it wants, to fix that set width on both divs to 50% so they "understand" they should share the container.

    Also I would add a padding to the "div info" to make it look closer to the challenge desing.

    Your code corrected should look like this:

    img-container {
    background-image: url(/images/image-product-desktop.jpg);
    background-position: center;
    background-size: cover;
    width: 50%;
    height: 28.125rem;
    z-index: 1;
}

.info {
    display: flex;
    flex-direction: column;
    justify-content: space-around;
    width: 50%;
    padding: 2em;
}

    Hope it helps!

    Marked as helpful
    1