Skip to content
OverviewSolutions
3
Comments
3
kepper104
@kepper104

All comments

  • Alex Bergelin•90
    @ABergelinSubmitted 7 months ago
    What are you most proud of, and what would you do differently next time?

    Proud of using flexbox which made it much easier to align a lot of the elements. Also using negative padding to get two elements closer than flexbox would allow.

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

    I completed the flexbox training on freecodecamp before attempting this challenge.

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

    My sizes are all absolute here. I will aim to use responsive sizes (vh, em etc) in future but any tips will be gratefully received.

    Social links page

    1
    kepper104•90
    @kepper104Posted 7 months ago

    Hi! Great job on the card! Some things that I've noticed:

    • You've set a constant height on the card and then used margins and paddings to add spacing between the elements while centering all the elements vertically in a flexbox. I would suggest against using negative paddings and margins (it can be considered a bad practice) and instead letting the card just wrap the elements vertically and using normal top and bottom margins on the element to space them out.

    For example (general layout idea):

    div (flex column with text-align: center)
  img - avatar, horizontal margin auto
  h1 - person's name, top margin of 40px
  h2 - location, top margin of 8px
  h3 - quote, top and bottom margins of 20px
etc

    That's just how I do things, but I'm open to discussion!

    And if you want to really make your solution identical to the design, there are minor differences in font sizes (e.g. "Front-end developer and avid reader." should be a tad smaller than the buttons below it and etc). Though in my solution I also had a bit of trouble nailing all the correct spacings and font sizes, so because there isn't a Figma design available, I believe you did your best ;)

    Marked as helpful
    0
  • kepper104•90
    @kepper104Posted 7 months ago

    Hi, great job! Few things i've noticed:

    1 - The attribution bar at the bottom hangs a bit above the page bottom, you can fix it by changing height: 95vh; of the body selector in the CSS to

    display: flex;
flex-direction: column;
height: 100vh;

    and adding flex: 1; to the main selector in the CSS This way, the body will become a flex container and main page contents will fill maximum available height while leaving just enough space for the attribution bar.

    2 - You left out alt attribute of the image empty, which is a bad practice and fortunately, easy to fix, just change it to something like alt="An abstract programming related illustration"

    3 - Your paddings are a bit off, for example spacing around the image is 16px instead of 24px (like in the reference design)

    4 - Lastly, a minor thing, but in the reference design all the text has a line-height: 150%;, very easy to fix and will make your recreation almost pixel-perfect.

    Good luck in your front-end development journey!

    Marked as helpful
    1
  • Anthony Young•30
    @A-Young-GitSubmitted 7 months ago
    What are you most proud of, and what would you do differently next time?

    I am proud that I was able to match the design well and pick up some beginner experience and knowledge.

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

    CSS knowledge gap

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

    CSS expertise

    QR Code Component Using HTML And Flexbox

    1
    kepper104•90
    @kepper104Posted 7 months ago

    Great job! A few points of feedback:

    • Don't use line breaks (br), try just letting the text wrap itself, add horizontal padding if needed.
    • The card from the reference design has a shadow, and I think yours doesn't.
    • Also you left out a default (?) Document page title in the header section of the HTML, maybe change it to something like QR Code Card.
    • Finally, the card is vertically off-center a bit, but that's a minor thing, I would add something like
    html, body {
    display: flex;
    height: 100%;
    width: 100%;
    align-items: center;
    justify-content: center;
}

    to the CSS, that way the card will be centered (you will also have to remove the margin and margin-top from the div styling)

    First time writing a code review, so sorry if the wording is a bit off or anything else ;)

    Marked as helpful
    1