Skip to content
OverviewSolutions
9
Comments
8
Numan Iftikhar
@numan-iftikhar

All comments

  • Numan Iftikhar•160
    @numan-iftikharPosted about 2 years ago

    Hey, here are some suggestions:

    • Avoid using different selectors for selecting same thing i.e., button etc.
    • You are using 3 selectors to select the button. Just give it a class name and boom!
    • You can make grid column responsive using auto-fit and minmax etc on grid-template-columns
    • Set font-size in rem
    • Use <a></a> tag for learn more button, bcz its a navigation.
    0
  • Numan Iftikhar•160
    @numan-iftikharPosted about 2 years ago

    Hi, you have provided the wrong link to your repo. It links to QR code challenge. Some tips:

    • Use semantic HTML
    • Use radio buttons for ratings
    • Use form tag for wrapping ratings and submit button
    0
  • Numan Iftikhar•160
    @numan-iftikharPosted about 2 years ago

    Hi, you did good work. Here are some tips:

    • Use main tag to wrap card content
    • Use flex-direction: column; to have footer at bottom
    • Brush up your flexbox if you need
    • Don't use media queries, these are just irrelevant
    • Use classes instead of ids
    • Decrease the height and width of your card
    • Use rem for font-size property
    Marked as helpful
    0
  • Numan Iftikhar•160
    @numan-iftikharPosted about 2 years ago

    Hi, you did fantastic work with your design. It's almost pixel perfect. Here are some suggestions:

    • Use main tag to wrap all the content
    • Get your HTML right
    • Don't use unnecessary div tags
    • Always use rem instead of px for setting font-size property
    • Avoid text-align: center in every element
    Marked as helpful
    0
  • Numan Iftikhar•160
    @numan-iftikharPosted about 2 years ago

    Hey, You did great work with design. Here are some suggestions:

    • Your HTML is not semantic
    • article tag is not necessary since it's not an article at all
    • Try centering main card by applying styling on the body tag
    • Use anchor tag instead of a button in proceed button
    • Wrap nav links in nav tag
    Marked as helpful
    0
  • A.Tech Services•100
    @A-Tech-ServicesSubmitted about 2 years ago

    This is my second attempt of making this project, and i really appreaciate the feed back from those people that corrected me in my first attempt of this project. i will appreciate if this one can also be reviewed and how i can improve on my coding.

    Thank you very much.

    Responsive Rating component. Using Css Flex-Box

    1
    Numan Iftikhar•160
    @numan-iftikharPosted about 2 years ago

    Hey, I've checked out your code. You did a great work. I've a few recommendations:

    • use radio buttons instead of divs
    • add validation, if rating not selected
    • improve semantics and class names
    • use camelCase naming convention
    1
  • Numan Iftikhar•160
    @numan-iftikharPosted about 2 years ago

    Hey dear, I've made a pull request to your code on github. I've fixed a lot of things. Do check it out and remember to read the pull request message :)

    0
  • Valentino•10
    @VvalenpSubmitted over 2 years ago

    I'm new doing this and it's my first challenge... I did this project with HTML and CSS only. At first it didn't cost me but finally I had problems adapting what I did in desktop (1440px) to device (375px). I highly recommend it for practice.

    First challenge (HTML, CSS)

    1
    Numan Iftikhar•160
    @numan-iftikharPosted over 2 years ago

    It's not responsive because relative units are being used and some additional CSS properties are also there that don't work with position: absolute; HTML needs to be well-structured and CSS needs to be precise. Attrubution div must be centered at the bottom.

    0