Skip to content
Submitted almost 3 years ago

order summary component with html and flexbox properties of CSS3

Kalim•70
@Kalim22
Preview site (opens in new tab)View code (opens in new tab)
This is a solution for...Order summary component
  • HTML
  • CSS
1newbie

Design comparison

SolutionDesign

Solution retrospective

Can some one tell me what I missing. I don't know, but I feel something wrong with our designing and coding.

Community feedback

  • Vanza Setia•27,775
    @vanzasetiaPosted almost 3 years ago

    👋 Hi Kalim!

    🎉 Congratulations on finishing this challenge!

    Like Buhaianu has said, you are using different font families (not following the style-guide.md). I would recommend fixing all the issues that he has mentioned.

    Here is some feedback from me:

    • Accessibility
      • Create a custom :focus-visible styling to any interactive elements (button, links, input, textarea). This will make the users can navigate this website using keyboard (Tab) easily.
      • 👍 You have done a great job on taking those images (the alternative text).
      • 👍 It's great that you specify the type of the button elements.
    • Best Practice (Recommended)
      • Make sure the indentation on your HTML is consistent. I notice that the <body> tag is indented while the <head> tag is not.
      • If the HTML element does not use any classes, it is recommended to not use the class property at all instead of leaving it empty.
    <img src="./images/illustration-hero.svg" alt="" class="" >
    • Are you using tailwind? If not, then consider removing the tag.
    • About the commented code, you should either remove it or use it, because the other developer (it might be you in the future) who wants to work with this project might get confused about whether or not the code should be deleted or used. It's okay to have commented in development process, but when you push your code in production, it's best to not have any commented code.
    .card{
    width: var(--width);
    /* height: 80vh; */
    • Styling
      • I would recommend letting the element inside it control the height of the parent element. Like in this case, I would recommend using min-height instead of height. Right now, on mobile landscape mode, the user can't see the rest of the content (can't scroll down).
      • Also, I would recommend making the body element as the flexbox container to make the card in the middle of the page. body element should be the main element that control the size of the web page.
      • If you have targeted all the elements using a universal selector (*), you don't need to add body element since it's already included.
    *, body{
    margin: 0;
    padding: 0;
    box-sizing: border-box;
}

    That's it! Hopefully, this is helpful!

    0
  • Remus D. Buhaianu•3,125
    @MiculinoPosted almost 3 years ago

    Hey @Kalim22, your design looks nice!

    I had a look through your final solution and I have a few suggestions to offer that I hope will be useful:

    • The top right border of your card looks weird. The edge is not completely round, it looks like it was cropped a bit.

    • The fonts you used for your card's content are different

    • Your card is a bit too small

    • The button is missing a box shadow similar to the one in the original design

    • The box shadow for your card is different

    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