Design comparison
Community feedback
- @0xabdulkhaliqPosted over 1 year ago
Hello there π. Congratulations on successfully completing the challenge! π
- I have other recommendations regarding your code that I believe will be of great interest to you.
BODY MEASUREMENTS π:
- The
width: 1440px
property forbody
element is not necessary. because it's a block level element which will take the full width of the page by default.
- Use
min-height: 100vh
forbody
instead ofheight: 100vh
. Setting theheight: 100vh
may result in the component being cut off on smaller screens.
- For example; if we set
height: 100vh
then thebody
will have100vh
height no matter what. Even if the content spans more than100vh
.
- But if we set
min-height: 100vh
then thebody
will start at100vh
, if the content pushes thebody
beyond100vh
it will continue growing. However if you have content that takes less than100vh
it will still take100vh
in space.
.
I hope you find this helpful π Above all, the solution you submitted is great !
Happy coding!
Marked as helpful0@FerZvaPosted over 1 year ago@0xAbdulKhalid Thank you! so much for your suggestion I appreciate. I will take them in mind for future solutions!
0 - @shakhboz-shukhratPosted over 1 year ago
Hello thereπ! Congratulations on completing this challenge!
There are a few problems with the code:
The variable import statements are missing file extensions (e.g. "_variables.scss").
In the "section-img-container" selector, the "background-image" property is using a Sass variable incorrectly. It should be "background-image: url($Img-desktop);"
In the "section-desc-container" selector, the "&_first-title" nested selector is missing the parent "&" symbol. It should be "&._first-title".
In the "&_prices-container" nested selector, "&_green-price" and "&_old-price" are missing the parent "&" symbol. They should be "&._green-price" and "&._old-price".
Here's the corrected code:
@use '_default.scss'; @use '_variables.scss'; body { background-color: $Cream; } main { background-color: $White; border-radius: 5px; display: flex; flex-wrap: wrap; @media screen and (max-width: 375px) { display: flex; flex-wrap: wrap; flex-direction: column; height: 680px; width: 300px; border-top-right-radius: 5px; } } .section-img-container { width: 300px; height: 450px; background-image: url($Img-desktop); background-size: cover; border-top-left-radius: 5px; border-bottom-left-radius: 5px; @media screen and (max-width: 375px) { background-image: url($Img-mobile); height: 240px; border-top-right-radius: 5px; border-bottom-left-radius: 0px; } } .section-desc-container { width: 300px; padding: 32px; &._first-title { margin-bottom: 25px; font-family: 'Montserrat', serif; color: $Dark-grayish-blue; letter-spacing: 0.2rem; font-size: 10px; } &._product-name { margin-bottom: 32px; font-family: 'Fraunces', serif; } &._paraph { color: $Dark-grayish-blue; margin-bottom: 35px; font-weight: 400; line-height: 1.5rem; } &._prices-container { display: flex; align-items: center; margin-bottom: 30px; &._green-price { color: $Dark-cyan; font-family: 'Fraunces', serif; } &._old-price { margin-left: 20px; font-family: 'Fraunces', serif; color: $Dark-grayish-blue; } } &._btn { margin: 0 auto; width: 100%; padding:10px; border-radius: 5px; border: none; background-color: $Dark-cyan; color: $White; font-family: 'Montserrat', sans-serif; display: flex; justify-content: center; align-items: center; &._icon { margin-right: 10px; } &:hover { transition: 0.2s ease; background-color: $Active-btn-color; cursor: pointer; } } @media screen and (max-width: 375px) { height: 240px; } } .attribution { font-size: 11px; text-align: center; @media screen and (max-width: 375px) { margin-top: 20px; } } .attribution a { color: hsl(228, 45%, 44%); }
Anyway, your solution is great. Hope you will find this helpful. Happy coding!
Marked as helpful0@FerZvaPosted over 1 year ago@Trueboss Thank you so much for let it me know, I appreciate your help and thank you so much for show me the code corrected
0 - @as90695Posted over 1 year ago
Hello There! FerZva Congractulation for completing this challange. You've done a great job and created a wonderful landing page to look at.
Here are some suggestions I hope this will help you in your upcoming projects.
-
I came across your site and noticed that your content is not vertically centered. I dont know if you left that purposely or not but you can add one more line in body to make it vertically centered. align-item: center;
-
use min-height attribute in body tag instead of height attribute. min-height: 100vh;
other than this you've done a great job. keep learning and keep growing. feel free to contact me for any suggestion you would like to give to me.
Marked as helpful0@FerZvaPosted over 1 year ago@as90695 Thank you so much for let it me know I appreciate your feedback
0 -
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