Skip to content
Submitted over 1 year ago

Advice generator app main using React and API

Romedix1•820
@Romedix1
Preview site (opens in new tab)View code (opens in new tab)
This is a solution for...Advice generator app
  • HTML
  • CSS
  • JS
  • API
2junior

Design comparison

SolutionDesign

Solution retrospective

Any feedback is welcome.

Community feedback

  • _nehal💎•6,710
    @NehalSahu8055Posted over 1 year ago

    Hello Coder 👋.

    Congratulations on successfully completing the challenge! 🎉

    Few suggestions regarding design.

    ➨ Don't use margin or padding for centering the card . To properly center cards on the page.

    • USING FLEXBOX
    body{
min-height: 100vh;
display: flex;
align-items: center;
justify-content: center;
}
    • USING GRID
    body{
min-height: 100vh;
display: grid;
place-items: center;
}

    ➨ Use Semantics for the proper design of your code.

    <body>
<header>
<nav>...</nav>
</header>
<main>...</main>
<footer>...</footer>
</body>

    or alternatively

    <body>
<div class="container" role="main">
/html code goes here: 📃
</div>
</body>

    I hope you find this helpful.

    Happy coding😄

    Marked as helpful
    0
    Romedix1•820
    @Romedix1Posted over 1 year ago

    @NehalSahu8055 What's wrong with using margin auto to center ?

    0
    _nehal💎•6,710
    @NehalSahu8055Posted over 1 year ago

    @Romedix1

    Try explicitly both ways and you will see the difference.

    That margin:auto is inefficient in centering the card.

    Use flex or grid.

    0
  • Kamlesh Rajesh Yadav•5,060
    @Kamlesh0007Posted over 1 year ago

    Congratulations on completing the challenge! That's a great achievement, and I'm sure you put a lot of effort into it. I really liked the way you approached the challenge and the code you wrote. You demonstrated a good understanding of the concepts and applied them effectively to solve the problem.I have a few suggestions to improve your code further. You need to make the container center using flexbox or grid layout to make the container as per design Here's an example code snippet:

    body {
display: grid;
place-items: center;
min-height: 100vh;
}
now remove below lines of code
main {
background-color: var(--DarkGrayisBlue);
border-radius: 2vw;
margin: 4rem auto auto;
padding-top: 1.2rem;
width: 90%;
}
    Marked as helpful
    0
    Romedix1•820
    @Romedix1Posted over 1 year ago

    @Kamlesh0007 What is a difference between using grid to center and centering by margin auto.

    0
    Kamlesh Rajesh Yadav•5,060
    @Kamlesh0007Posted over 1 year ago

    @Romedix1 If you need more control over your layout and want to create complex grid-based structures, CSS Grid is a better choice. It provides more flexibility and allows you to create both horizontal and vertical centering as well as complex layouts.Its better or optimized for centering a div both horizontally and vertically

    0
    Romedix1•820
    @Romedix1Posted over 1 year ago

    @Kamlesh0007 Thanks

    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