Skip to content
OverviewSolutions
9
Comments
5
Lee
@LeeConnelly12

All comments

  • vishwjeet thakur•30
    @ervishwaSubmitted over 1 year ago

    I tried my best. I know it is not perfect but I am trying to improve my skills...I will be glad if you suggest my anything according to you..

    Raw HTML CSS and JS

    3
    Lee•230
    @LeeConnelly12Posted over 1 year ago

    Good job! the colors and positioning look correct.

    However, I've noticed you're adjusting the position of your <body> down with this CSS to vertically center the quote box on the page.

    body {
position: relative;
top: 222px;
}

    You're also setting your <body> as grid and centering items within it.

    body {
display: grid;
place-items: center;
}

    By default, the <body> is the height of the content inside of it, if you inspect the <body> on your page you'll see that it's only about 416px tall.

    A more responsive way to vertically center the quote box would be to make the <body> the full height of the screen and allow the body's grid to vertically center the items inside of it.

    body {
min-height: 100vh;
margin: 0;
}

    Finally, you can remove the position relative and top CSS properties

    body {
position: relative;
top: 222px;
}

    Now, on screens with smaller heights, the quote box will move to be in the center instead of a fixed position.

    Marked as helpful
    1
  • Lee•230
    @LeeConnelly12Posted over 2 years ago

    Your rating component looks super close to the design, good work!

    Only thing I noticed is that your ratings are getting squished.

    This is happening because you're setting the ratings' size using padding: 6%.

    You can see the rating's padding in this screenshot

    As you can see from the screenshot above, padding is getting added around the content in the element.

    So because the numbers are taller than they are wide the padding will make the ratings look tall.

    One way to fix this would be by setting a fixed width and height on your .rating elements

    .rating {
  // padding: 6%; can now remove this as we're setting a width and height
  width: 5rem; 
  height: 5rem;
}

    Then, to center the number inside each of the ratings you could use CSS grid, like so:

    .rating {
  display: grid;
  place-items: center;
}
    0
  • John Soto•130
    @Soto-JSubmitted over 2 years ago

    Fun challenge. Feedback is greatly appreciated!

    Javascript

    1
    Lee•230
    @LeeConnelly12Posted over 2 years ago

    If I don't select a rating and click submit, the thank you step text says "You selected undefined".

    One way you could prevent this is by making the submit button disabled until a rating is selected, then, when a rating is selected you set disabled to be false.

    Doing this will stop the user from going to the next step without choosing a rating first.

    Other than that your rating component looks clean and the hover states are smooth, good job!

    Marked as helpful
    1
  • Eduardo•910
    @KTrick01Submitted over 2 years ago

    Hi! This is my first challenge in this community, im pretty new with css and html so i would like you guys to give me your feedback, even if it seems basic for you, im sure it will help me!

    NFT preview card component hub + Flexbox

    3
    Lee•230
    @LeeConnelly12Posted over 2 years ago

    Good use of BEM to structure your classes.

    While moving in from the right, your blue overlay makes the corners look sharp Example

    To solve this you could set border-radius: 10px on the <picture element.

    This along with having overflow: hidden already set on your <picture> ensures that any content inside of the <picture> tag will not overflow the border radius.

    Marked as helpful
    0
  • Lee•230
    @LeeConnelly12Posted over 2 years ago

    Using calc() to offset the attribution text at the bottom of the page, smart!

    Also love the use of BEM classes, great way to structure CSS.

    I started on mobile and added classes to the <body> to center the page's content so I could see the QR component easier while building it.

    As you know the size of the QR code image, you could set the width and height attributes to be 280, doing this means you don't get any layout shifting while the image is loading.

    0