Skip to content
OverviewSolutions
27
Comments
7
Bill McDavitt
@BMcdavitt

All comments

  • Bill McDavitt•460
    @BMcdavittPosted almost 2 years ago

    Nice Job! I like the animation on the image change.

    I noticed that the navigation buttons tend to get misaligned in the mobile view, you may want to look into using a grid layout to keep these in sync instead of trying to use position absolute, it worked well for my solution

    Marked as helpful
    0
  • Bill McDavitt•460
    @BMcdavittPosted about 2 years ago

    Hello, great job on this solution!

    You could look into using @media queries as a way to control what edges of the image to apply the radius to.

    Good luck and happy coding.

    1
  • Bill McDavitt•460
    @BMcdavittPosted about 2 years ago

    Congratulations on completing your first challenge with Front End Mentor!

    Your solution looks great. One quick tip if you don't mind, a lot of challenges here ask for the content to be centered vertically. The easiest way to do this would be to add the following CSS to your code:

    body{
    height: 100vh;
    display: flex;
}

div.box{
    margin: auto;
}

    Good luck, and happy coding.

    Marked as helpful
    1
  • GRAVIT•80
    @gravit09Submitted about 2 years ago

    I have done many mistakes in this project like I am unable to put the annual plan next to the image, can't add more space between my items, if you find some more mistakes please provide me feedback your feedback helps me a lot.

    thank you

    Order summary Component

    2
    Bill McDavitt•460
    @BMcdavittPosted about 2 years ago

    Hi GRAVIT,

    Another suggestion to help you center the card would be to set the body height to 100vh. This will allow you to set the display to flex and align-items to center:

      height: 100vh;
  display: flex;
  justify-content: center;
  align-items: center;

    Now you can remove the margins from .card and everything will center perfectly in all displays.

    Happy coding!

    Marked as helpful
    1
  • Bill McDavitt•460
    @BMcdavittPosted about 2 years ago

    Nice job on your first solution, and kudos for using React.

    A couple of comments if you wouldn't mind:

    • Defining both height and width on the image can make it appear distorted. You would be better defining only one of these and letting the image scale in size proportionally.
    • I am seeing a vertical scroll bar. Setting your body margin and padding to 0 should eliminate this.

    Happy coding!

    Marked as helpful
    0
  • Bill McDavitt•460
    @BMcdavittPosted about 2 years ago

    Nice job completing your first solution!

    One quick suggestion if you don't mind, an easy way to center your content vertically is to set the height to 100vh within the body definition. This would make the height of the body equal to the height of the viewport. This taken with the current "align-items: center" definition will nicely put your content in the middle of the browser's view port.

    body{ display: flex; height: 100vh; align-items: center; justify-content: center; background-color:hsl(212, 45%, 89%); }

    Marked as helpful
    1
  • Bill McDavitt•460
    @BMcdavittPosted about 2 years ago

    This looks good, nice work.

    I did notice that you are able to click submit without selecting a rating first, and this displayed 'you selected 3 out of 5 stars'. Perhaps finding a way to prevent submitting until a score is given would be worth looking into.

    Here is my solution if your interested: https://www.frontendmentor.io/solutions/interactive-rating-component-lA5R_rYZNp

    1