Skip to content
Submitted over 1 year ago

3-column preview card component

Gledson Nunes Santos•70
@nunes1886
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

Community feedback

  • Account deleted

    Hi

    Congratulations on completing the challenge

    Landmarks are important for adding structure to your page and landmarks such as <header>, <main>, <nav> and <footer> aid navigation in assistive technologies such as screen readers and for keyboard accessibility.You do not need so many container elements you should collapse .mobile and .container into one and use <main>. You should collapse <article> and <div> into one and use <section> for your card containers, this will get rid of the <h1> validation issues.

    Instead of <button> you should have used anchor tag <a> and styled it to look like a button. <button> is intended for actions carried out on the page whereas in this case the learn more button would take you to another page and is therefore a link.

    In your media queries you do not need to duplicate the properties you want to keep, you need only write the styles you want to change.

    I hope this helps.

    Happy coding.

    Marked as helpful
    1
    Gledson Nunes Santos•70
    @nunes1886Posted over 1 year ago

    @madzi-hove - Hey man, thanks a lot for the feedback. I just made some adjustments following your tips. Could you take a look and reevaluate? Thanks!

    0
    Account deleted

    @nunes1886

    Hi Your markup looks much cleaner now and easier to read. Cleaner and simpler markup makes it easier to style the page. A better idea would be to make <body> the grid container, with this you would then be able to use justify-items and align-items to center .container.

        .body {
        display: grid;
        grid-template-columns: 1fr;
        justify-content: center;
        align-items: center;
}

    justify-items should work just as well as justify-content, play around with them and see what works. With grid doing the centering there will be no need for margin: 0 auto; in .container.

    In your card styles you are doing too much duplication. Because your cards have so many similar settings you should add a class .card to each card and put all the similar styles in there eg.

    .card {
    display: grid;
    grid-template-columns: 1fr;
    line-height: 20px;
    padding: 40px;
}

.card-one {
    background-color: hsl(31, 77%, 52%);
}

.card-two {
    background-color: hsl(184, 100%, 22%);
}

.card-three {
    background-color: hsl(179, 100%, 13%);
}
}

    You can cut down on so much of the code you wrote, also border-radius should be on .container only. You can also do the same thing as above to your .btn styles and cut your code further.

    You also have a lot of duplication of styles in your media queries. If you set a style you do not have to put it in your media query unless you are changing it. eg

    
.container {
    display: grid;
    grid-template-columns: repeat(3, 1fr);
    max-width: 700px;
    margin: 0 auto;
    margin-top: 5.6rem;
}

@media (max-width: 600px) { 
    .container {
        grid-template-columns: auto;
        max-width: 300px;
        margin-top: 1rem;

    }
}

    The style are still active in the cascade so there is no need to repeat them.

    Put font-family: 'Lexend Deca', sans-serif; in body, the other tags will inherit the font.

    Put font-family: 'Gill Sans', 'Gill Sans MT', Calibri, 'Trebuchet MS', sans-serif; in .btn and it will be applied to all three links.

    I hope this helps. Feel free to reply if anything is not clear

    Happy coding.

    Marked as helpful
    1

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