Skip to content
OverviewSolutions
3
Comments
3
jan28x
@jan28x

All comments

  • jan28x•50
    @jan28xPosted 3 months ago

    A few improvements that could be made:

    • it seems you're not linking to the actual picture provided in the assets folder. Try taking the first '.' out of "./assets/images/avatar-jessica.jpeg"
    • the html could be cleaned up a bit. Try moving the
    .attribution { font-size: 11px; text-align: center; }
.attribution a { color: hsl(228, 45%, 44%); }

    in the index.html file to the styles.css file.

    Using an unordered list of links instead of buttons would also be a good idea as the actual site would be used to link the user to the creator's various social media profiles.

    Setting the background colour as the one specified in the style guide would also make the solution closer to the design.

    0
  • jan28x•50
    @jan28xPosted 3 months ago

    Your solution is incredibly similar to the design!

    Minor changes in the appearance of the site could be:

    • The preview card itself is not really centred in the design, it seems to be more towards the top than the bottom. Using relative units was a good choice, however maybe using % instead of vh units would allow it to be centred more. Using vh units adds a lot of extra space under the credits.
    • Using any of the methods listed here woudl allow you to centre the div, and would reposition the card in the center of the screen even if you made the screen smaller height-wise. Currently your card requires scrolling the page if the height of the site is made smaller.
    • The box-shadow disappears off-screen on a phone, so perhaps adding a bit of padding to the "card" div would ensure the entire thing would show up.
    0
  • jan28x•50
    @jan28xPosted 3 months ago

    Your solution looks really similar to the design and overall looks really well done!

    The HTML is easy to read, and includes semantic HTML, like including Alt text for the image.

    The layout looks generally good on both wide screens and phone screens.

    It could be improved in small areas by:

    • rounding the corners a little bit more using the border-radius property in css
    • centering the .container div using the methods detailed here instead of relying on padding, as it seems to be closer to the bottom than the top. This is more visible on a phone screen.
    Marked as helpful
    0