Skip to content
Submitted over 2 years ago

3-column-preview-card

Keagan Dickinson•160
@PhisherFTW
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

I found this a good test, of how quickly I could do a challenge taking around 40 minutes, I know its no record but I found it encouraging I didn't find any major hurdles, learning new concepts constantly like breakpoints to change the design when I get to a mobile width.

If you see any-ways I can optimize and make my code overall better please let me know!

Thanks

Community feedback

  • Fluffy Kas•7,715
    @FluffyKasPosted over 2 years ago

    Hey,

    This looks really good! Only a few tiny suggestions:

    1. You should center the component with either grid or flexbox. For example, you can add this to the <body>:
    display: grid;
place-items: center;
min-height: 100vh;

    When you add this snippet, you'll see a slight vertical overflow that is caused by the margin you added to <main>. You can simply just remove this, the margin is no longer needed.

    1. Add cursor: pointer to the button. Some transitions for the hovers would also look nice ^^

    2. You can also remove flex-direction: row from <main>. This direction is the default when you're using flexbox so there's no need to add it!

    Marked as helpful
    2
    Keagan Dickinson•160
    @PhisherFTWPosted over 2 years ago

    @FluffyKas This is fantastic, and exactly the type of feedback I am looking for thank you so much I have implemented it, right now.

    Thanks!

    1
  • Shashree Samuel•9,260
    @shashreesamuelPosted over 2 years ago

    Hey good job completing this challenge

    Keep up the good work

    Your solution looks great however I think that the font size of each button needs to be a bit bigger.

    In terms of your code since your three divs all use the following properties

    • display: flex;
    • flex-direction: column;
    • padding: 3rem;

    You can create a class for these properties and just reference it in each div element instead of writing these properties above thrice.

    It helps with cleaner, efficient code which means that your three divs will just need to include the background-color and border-radius etc.

    Hope this helps

    Cheers

    Marked as helpful
    1
    Keagan Dickinson•160
    @PhisherFTWPosted over 2 years ago

    @TheCoderGuru Thank you so much for your feedback, this is exactly what I wanted, I am looking for any way to improve my code and learn new things a lot the way, thank you for taking your time to look at my code, and I will implement your suggestions immediately.

    I also noticed I could use the same Logic with the buttons, as I repeated these CSS elements 3 times, I simply put them into "repbtn" and added that class to the buttons.

    • border-style: solid;
    • font-family: 'Lexend Deca', sans-serif;
    • font-weight: 400;
    • cursor: pointer;
    • border-radius: 50px;
    • padding: 1rem 0.25rem;
    • width: 10rem;
    • font-size: medium;

    This did help a lot, thanks

    I also increased the size of the button text

    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