Skip to content
OverviewSolutions
3
Comments
10
Ritika Agrawal
@Ritika-Agrawal811

All comments

  • AlePros•50
    @AleprosDevSubmitted 11 months ago

    I had some difficulty re-understanding container centering issues, as well as choosing between margins and padding. I don't think using padding in the end was the best possible practice.

    CSS Flex

    1
    Ritika Agrawal•170
    @Ritika-Agrawal811Posted 11 months ago

    Hey! great work on the challenge!

    Flex can be confusing to understand so I would like to suggest few points looking at your code:

    • when using flex, if we want to center align a child vertically we need to add a height to the parent. In the "box" class, I think align-items and justify-content are used to center the "container" so just add a min-height/height of 100vh.
    
.box{
    margin-top: 70px;
    display: flex;
    align-items: center;
    justify-content: center;
    height: 100vh: /* just add a height */
}
    • Because you added align-items: center to "container", you don't have to add left and right padding to h1 and p tags. They would already be in center.

    Also I think you have accidently deleted the image from your repo. Keep coding!

    Marked as helpful
    0
  • Ritika Agrawal•170
    @Ritika-Agrawal811Posted 11 months ago

    Great work on the challenge!

    Looking at your code I would suggest to use external stylesheets for CSS and use semantic HTML tags like <article> etc. instead of <div> tags.

    Marked as helpful
    0
  • Ahmed Huzain•380
    @ElHuzainSubmitted 11 months ago

    Hey! This by far my best Front End Mentor challenge Tools & Techniques I used

    • React.js framework
    • Redux state management to update stepper state
    • TailwindCSS for the responsive design
    • React-hook-forms for form validation

    Let me know what you think ^^

    Responsive layout using TailwindCSS

    1
    Ritika Agrawal•170
    @Ritika-Agrawal811Posted 11 months ago

    Amazing work

    I noticed that the heading "Personal info" and the subtitle below it "Please provide your name, email address, and phone number." remain same for all sections. You might want to fix that.

    1
  • Ritika Agrawal•170
    @Ritika-Agrawal811Posted 11 months ago

    Hey nice work on the challenge!

    Looking at your code, I would like to suggest to use rem/em values instead of pixels. Helps to easily make responsive designs.

    1
  • Aphelion Web Development•120
    @Aphelion-imSubmitted 11 months ago

    The result is quite good on my desktop pc.

    However, on my mobile and in Chrome dev tools/Firefox dev tools (On mobile view), a part of the top of the site is seemingly cut off.

    I watched this Youtube video with Kevin Powell explaining the vh unit, and some other units to solve the problem, but that didn't help.

    https://www.youtube.com/watch?v=veEqYQlfNx8 (Kevin Powell, The problems with viewport units)

    On ipad, the product card, seems to vertically stretched, so the text on the right gets too much white space.

    Product Preview Card Component

    2
    Ritika Agrawal•170
    @Ritika-Agrawal811Posted 11 months ago

    Hey, I inspected your code in Chrome Dev tools and noticed that height is set to 100% for body and html tags. It seems its coming from the CSS reset. I would suggest to add this height only for desktop screens.

    To solve this problem I always add the height inside media query for desktop screens. For mobile screens I just give a top and bottom margin for the card.

    I have completed this challenge, you may look into my code to see how I did it.

    Marked as helpful
    1
  • Ritika Agrawal•170
    @Ritika-Agrawal811Posted 11 months ago

    Hey, good job on the challenge!

    I saw your preview and noticed that you haven't added the active state( hover state ) to add to cart button. You can use :hover pseudo class on the button tag for it!

    Something like:

    
button{
 transition: background-color 150ms ease-in-out;
}

button:hover{
 background-color: /* the color code */
}

    Hope this comment was helpful! Happy coding!

    0
  • Ritika Agrawal•170
    @Ritika-Agrawal811Posted 11 months ago

    Hey! Great work on the challenge!

    I saw your code and its looks good! I just think you have missed adding a box-shadow to the card. Also I would suggest to use relative units (em and rem ) instead of pixels.

    Marked as helpful
    0
  • Ritika Agrawal•170
    @Ritika-Agrawal811Posted 11 months ago

    Hey, good job on the challenge

    I saw your code and would like to point out that you don't have to repeat all the properties for an element inside a media query.

    For example, I noticed that for the body tag you have below properties outside the query:

    body {
    background: hsl(30, 38%, 92%);
    padding: 30px 0;
    margin: 1rem;
}

    and then inside the media query you have these :

      body {
        background: hsl(30, 38%, 92%);
        padding: 60px 0;
        margin: 1rem;
        display: flex;
        justify-content: center;
        align-items: center;
    }

    because you are not changing margin and background color, you can remove those properties from inside the media query. Those rules will be applied automatically for all screens. This is will help greatly reduce your CSS code!

    Hope this comment was helpful!, Happy coding! 🙂

    Marked as helpful
    0
  • Ritika Agrawal•170
    @Ritika-Agrawal811Posted 11 months ago

    Hey! Good work on the challenge

    I saw your code and would like to share few points to help with responsivity.

    1. Use relative units, don't use pixels everywhere

    It's best to use rem for font-sizes and em for other properties. em values are calculated relative to an element's font size and rem are calculated against a default browser size which is 16px in most cases. You may read more about it here.

    Once you start using relative units your CSS code will reduce greatly. You won't have to use so many media queries too 🙂

    1. Don't set a max-width on body tag

    I noticed that you have assigned a maximum width of body tag. Instead add that to the main tag.

    Hope these points are helpful to you!

    Marked as helpful
    0
  • Ritika Agrawal•170
    @Ritika-Agrawal811Posted 11 months ago

    Looks great! You can also add cursor: pointer property to button element to change the mouse cursor to pointer (a hand pointing to button)

    Marked as helpful
    1