Design comparison
Community feedback
- @devmor-jPosted over 2 years ago
Hey great job π
I've been crawling through your code (cloned your repo π) and here are some tips if you're interested in other's opinions:
- 1οΈβ£ Mobile view is very small because you missed the most important css3
viewport
meta tag. Just add this tag to yourindex.html
and everything will fix:
<meta name="viewport" content="width=device-width, initial-scale=1.0">
This will bring in some issues with sizing and I'm sure you can easily deal with it but feel free to ask any questions if stuck.
- 2οΈβ£ I suggest remove any extra
.container
from your style that just makes it complicated and hard to debug and this simple app does not need that high specificity caused by so many class names. - 3οΈβ£ Try not to set width sizes using un-responsive units like 'px' and instead you can use 'rem' as a good starting point. I usually set my cards width anywhere from '16rem' to '24rem'.
And that's it for now.
This is important to know that you did the job done and this is good.
So don't make it too hard on yourself and just keep going; One day you will find yourself highly confident in your code πͺππ
Marked as helpful1@elahemirzaeePosted over 2 years ago@devmor-j First of all thanks for your valuable message Devmor, it really means a lot to me that you checked my project very precisely it was really helpful. i am a junior :) and these small tips makes me more concentrate on my code. You made my day really :)
1 - 1οΈβ£ Mobile view is very small because you missed the most important css3
- @correlucasPosted over 2 years ago
πΎHello Elahe, congratulations for your solution!
I've inspected your solution code and I liked a lots the level of detail you've reached here. I found only two things that you could consider change in order to improve it a bit (just a bit). You can wrap all content using the main tag you've already add with no need for the the div inside of that.
Note that you pricing section items are too much close when the screen scales down and the container contract, so I've created a media query to change the section flow and give it a vertical padding between the elements in order to have them in different rows. See the code below:
@media (max-width: 300px) { .nft-card .nft-card-description { display: flex; align-items: center; justify-content: space-between; padding: rem 0; border-bottom: 2px solid var(--Very-dark-blue-line); flex-direction: column; } } .nft-card .nft-card-description li:first-child { color: var(--Cyan); padding: 10px 0; font-weight: 600; }}
Hope it helps and happy coding!
Marked as helpful0@elahemirzaeePosted over 2 years ago@correlucas Hello Lucas :) Thanks for you feedback it really helped meπ I made the changes you mentioned in comment in my codeππ½now it looks really better
1@correlucasPosted over 2 years ago@elahemirzaee Happy to hear that Elahe, keep it up!
Marked as helpful0 - @Pulkit-s21Posted over 2 years ago
You should add some inline margin for mobile view because in that the card sticks to the edges and that doesn't look nice visually
0@Pulkit-s21Posted over 2 years ago@elahemirzaee nice.. It doesnt stick the edges anymore but it isn't vertically centered and I checked your code so maybe you can set your body as
display: grid; min-height: 100vh; place-items: center
to fix it0
Please log in to post a comment
Log in with GitHubJoin our Discord community
Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!
Join our Discord