Skip to content
OverviewSolutions
10
Comments
4
polukarp
@polukarp

All comments

  • polukarp•190
    @polukarpPosted about 2 years ago

    Love your solution, but I'd suggest a couple of minor fixes here:

    • Why is your <div className="nav-bottom"> not a part of your <nav> element but still a part of your Header component? Try placing it in your main content, especially as it contains the <h1> tag. You can name it a hero section if you wish.
    • Also your <ul> elements in the footer should not contain directly any element rather than <li>, or <template> elements.
    Marked as helpful
    0
  • polukarp•190
    @polukarpPosted about 2 years ago

    Awesome solution, but I found that on mobile view your .info-grid-item-img aren't responsive. So what I suggest you do about it is use object-fit: contain on it and that's gonna solve this issue.

    Also I'm not sure about the padding here, I'd just use max-width:1440px and margin: 0 auto to make sure your website looks good on ultrawide monitors.

    Let me know if you have any questions.

    Marked as helpful
    1
  • polukarp•190
    @polukarpPosted about 2 years ago

    Hey, Ken. Great solution overall, but I could suggest several things to upgrade:

    • Add fonts that are listed in style-guide.md (it would look even better if you apply them)
    • Use aspect-ratio: 1/1; for your .qr-image container so that it is the same ratio and size as your img element. That would be important if you would want to make a scale hover effect or something like that, because your image would overflow a liitle on the bottom, so consider using aspect-ratio property for that. Here's a link if you want to learn more about it.
    Marked as helpful
    1
  • polukarp•190
    @polukarpPosted about 2 years ago

    Overall great solution, but I'd recommend you use <h1></h1> element to underline the main heading of your component so that it's easier for people with visual impairment to navigate your website/component. And also try to erase your comment because HTML validation says "The document is not mappable to XML 1.0 due to two consecutive hyphens in a comment."

    0