@NicolasPirezGit
Aldo Adabunu
@KwakuAldoAll comments
- @KwakuAldo
Your site isn't responsive, it doesn't look good on mobile. It really looks good on desktop based on the screenshot comparison with the design. You should make the mobile design as the jpg in the design folder.
- @samlan24@KwakuAldo
Hello Sam, you need to put a hover state on the image. You can have a look at my solution here to help you with that https://www.frontendmentor.io/challenges/nft-preview-card-component-SbdUL_w0U/hub/nftpreviewcardcomponentmain-made-with-css-flexbox-QYjZ70xgz
Marked as helpful - @xeuxdev@KwakuAldo
-
Always try and use semantic html, instead of <div class="container"> use the html main element, then section the cards. Avoid using too many divs.
-
you should set a max-width rule for your body element to avoid it over stretching on wider screens, my suggestions is to use max width 1080-1280(anything between these numbers shd be okay for this challenge)
-
Also try and use the font-family that was specified.
Everything else looks good. Happy Coding! 💪🏿
Marked as helpful -
- @samlan24@KwakuAldo
You need to put some margin between the elements, particularly between your <p> tags to make the layout cleaner, also in mobile view the cards are to narrow, try adding some width to it fill up the screen a bit.
Marked as helpful - @Wantaiq@KwakuAldo
you need to set a max-width for the body to about 1440px, the element stretches too much that it sqews the design, plus you are leaving a lot of white space at the bottom of the section containers.
Other than that everything seems fine and dandy, happy coding 💪🏿
Marked as helpful - @dannygomes@KwakuAldo
Hello Danny, this looks great. The mobile design is almost pixel perfect. Well done.
- @arnoldrubi@KwakuAldo
You need to adjust the "3 days left", push it a bit to the right.
- @UsamaBinKashif@KwakuAldo
This looks good, try and use semantic html more, and make sure all the elements are in one landmark container.
- @Beats-Ayush@KwakuAldo
Don't declare an absolute width value for the body. DO something like this: body { width: 100%; }
main { width: 350px; max-width: 414; }
That way the page is already set up for mobile view also since the main component does not need to be large for desktop.
Marked as helpful - @Jennifer1919@KwakuAldo
How did you get the element to center so that in landscape view on mobile it is scrollable?
