Skip to content
Submitted about 2 years ago

VS Code, Git Hub, Git Hub Pages, HTML, CSS

VaporDusk•230
@VaporDusk
Preview site (opens in new tab)View code (opens in new tab)
This is a solution for...3-column preview card component
  • HTML
  • CSS
1newbie

Design comparison

SolutionDesign

Solution retrospective

How can I improve this project?

Community feedback

  • PhoenixDev22•16,950
    @PhoenixDev22Posted about 2 years ago

    Hi VaporDusk,

    Congratulation on finishing this challenge. Great job on this one! I have few suggestions regarding your solution:

    HTML

    • Page should contain <h1> . The <h1> is most commonly used to mark up a web page title. This challenge is supposed to be one component of a web page. To tackle the accessibility issue in the report , you may use an <h1> visually hidden with class=”sr-only”. You can find it here.
    • Don't capitalize in html, let css text transform take care of that. Remember screen readers won't be able to Read capitalized text as they will often read them letter by letter thinking they are acronyms.
    • In this challenge, the images are much likely to be decorative. For any decorative images, each img tag should have aria-hidden="true" attribute to make all web assistive technologies such as screen reader ignore those images .

    CSS

    • In order to center the card on the middle of the page , you can use the flexbox properties and min-height: 100vh for the <body> add a little padding to the body that way it stops the card from hitting the edges of the browser.
    • You can use flexbox properties to the container that wraps the three card and give it flex-direction : row for the desktop and column for the mobile.
    • If you make each column into a flex column. Then set everything inside the cards to have some margin in one directionmarin-bottom: ; only the link wouldn't need it and use margin-top:auto on the learn more link that will push it to the bottom of the cards.
    • Add border-radius and overflow hidden to the main container that wraps the three cards so you don't have to setborder-radiusto individual corners.
    • It's not recommended to set fixed height to component, you almost never want to set it. let the content of the component define the height.
    • An explicit width is not a good way to have responsive layout . Consider using max-width to the component that wraps the three cards in rem.
    • Remember a modern css reset on every project that make all browsers display elements the same.
    • Don’t Repeat Your CSS(DRY) is a good general principle to follow and eliminating duplication of css code should naturally be part of coding journey.
    • Consider using rem for font size , it' not recommended to use px for font size as absolute units don’t scale for example 15px will always be 15px on the same device. Using pixels is a particularly bad practice for font sizing because it can create some accessibility problems for users with vision impairments.

    Hopefully this feedback helps.

    Marked as helpful
    1
    VaporDusk•230
    @VaporDuskPosted about 2 years ago

    @PhoenixDev22 Hi, I've been trying to implement everything in your feedback but while working through it I've met up with some difficulties:

    • couldn't implement flexbox properties to get the design view: there is gaps between cards or they stick to one of the sides;
    • couldn't walk away from setting border-radius to individual corners. Setting border-radius to the container didn't work. Rest of the feedback work well. Thank you for your feedback.
    0

Please log in to post a comment

Log in with GitHub

Join our Discord community

Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!

Join our Discord