Skip to content
OverviewSolutions
4
Comments
2
Adenike Adeyemi
@nikea1

All comments

  • Adenike Adeyemi•40
    @nikea1Posted over 2 years ago

    Hey Dan,

    I must say you did a great job on the layouts for mobile and desktop.

    The only thing I could notice is needing to bold some of your text like the header and the ratings. Make sure when getting the link for your fonts from google fonts, you select all the font weights you need.

    Also, when you get to a mobile size screen the background images start repeating. I think your just missing a background-repeat: no-repeat somewhere.

    And I don't know if you worry about the 4k view, but your background is stuck to the top left corner. Using background-position will help you put each part of the image in the right spot.

    I think that's all I've notice. I wish you luck! :)

    0
  • Adenike Adeyemi•40
    @nikea1Posted over 2 years ago

    Hello topspinppy! I think you did an excellent job with the card so far.

    Just a few things I noticed. First, I think you need to fix the text color, you can't really see it or read it easily. When styling text it's important the text colors and the background colors have enough contrast for readability. I believe one of the files that's downloaded with the challenge called style-guide.md has the colors needed so you can just copy and paste them in your style sheet where needed.

    Second is just some html formatting. So, in the <html> tag, it's best practice to add a lang to declare the language of the document. Since it's in English you put in lang="en". In the image tag you don't need to specify units for the size for height and width. It's already in pixels and <img> will only use pixels. Just the number itself is fine.

    Finally, there accessibility. So, I just learned this myself recently. You just need to use a <main> tag around your content and use <h1> tag somewhere at the beginning of your content.

    I think that's about it. I wish you luck! :)

    2