Design comparison
Solution retrospective
all feedback's are welcome, please notify my mistakes, that will be more helpful, thanks
Community feedback
- @grace-snowPosted almost 2 years ago
Hello
There are lots of changes recommended in this. Hopefully you will learn from the notes and changes I just made in browser... Main things are to
- NEVER put font size in px. Use rem
- NEVER limit width on the body
- NEVER limit height on elements containing text. Use min-height if you need anything at all (you rarely will need it)
- Work mobile first next time
- Set media queries at the point the layout needs to change. The widths in the styleguide are nothing to do with code, they're just letting you know the sizes of the static design images
- Don't repeat style properties in media queries. You only need to override the specific properties that should change
- It's more performant to link fonts in the head than import them in CSS
/* design.css | https://prithiviraj275.github.io/stars-preview-card.github.io/design.css */ body { /* width: 1440px; */ min-width: ; min-height: 100vh; display: grid; place-content: center; background-color: hsl(233, 47%, 7%); color: #fff; padding: 1rem; } main { /* background-color: hsl(233, 47%, 7%); */ /* color: white; */ /* height: 100vh; */ } .card { /* height: 500px; */ /* width: 1100px; */ max-width: 1100px; border: ; border-radius: 0.7rem; overflow: hidden; } .imgcard { /* height: 500px; */ /* height: 100%; */ /* width: 1000px; */ /* border-radius: 0 10px 10px 0; */ flex-basis: 50%; background-color: purple; background-blend-mode: multiply; note: use the provided purple color; flex: 0 1 50%; background-repeat: no-repeat; opacity: 0.7; note: although this works with background image you are making it harder for yourself and this is less performant. You would be better using a picture element in the html; } .imgs { /* height: 500px; */ /* width: 505px; */ /* border-radius: 0 10px 10px 0; */ /* background-size: cover; */ /* background: rgb(75 0 255 / 70%); */ /* position: absolute; */ } .textcard { flex: 0 1 50%; width: ; } .start { note: these make no semantic sense as heading elements. You should be using a list with 3 list items for these stats; } .sp2 { /* font-size: 16px; */ font-size: 1rem; display: block; note: don't use br elements. Display block to lay out on a new line, margin for vertical space'; margin-top: 0.25em; } .p3 { /* font-size: 24px; */ /* margin-left: 100px; */ font-size: 1.5rem; } .d1 { justify-content: space-between; } @media screen and (max-width: 375px) { .card { /* width: 320px; */ /* height: 700px; */ /* background-color: hsl(244, 38%, 16%); */ /* border-radius: 10px; */ /* display: flex; */ max-width: 320px; note: 375px is way too small for the media query. Change the layout at the point where it needs to change; note: next time do mobile styles first; } .imgcard { /* width: 320px; */ /* height: 210px; */ /* border-radius: 10px 10px 0 0; */ /* background-size: cover; */ width: 100%; min-height: 210px; } .imgs { /* width: 320px; */ /* height: 132px; */ /* border-radius: 10px 10px 0 0; */ } .p3 { /* font-family: 'Lexend Deca', sans-serif; */ /* font-weight: 400; */ /* font-size: 24px; */ /* margin-left: 0; */ } .p2, .sp2 { /* font-family: 'Inter', sans-serif; */ /* font-weight: 400; */ /* color: hsla(0, 0%, 100%, 0.6); */ } .p1 { /* font-family: 'Lexend Deca', sans-serif; */ /* font-size: 900; */ /* font-size: 24px; */ font-size: 1.5rem; } .textcard { /* padding: 0 10px 30px; */ padding: 0 10px; } .sp2 { /* text-transform: uppercase; */ /* font-size: 16px; */ } main { /* background-color: hsl(233, 47%, 7%); */ /* color: white; */ /* height: 900px; */ /* display: flex; */ /* align-items: center; */ /* justify-content: center; */ } body { /* margin: 0; */ /* box-sizing: border-box; */ /* width: 375px; */ } } footer { margin-top: 1rem; margin-block-start: 1rem; }
Marked as helpful0@prithiviraj275Posted almost 2 years ago@grace-snow thanks a lot this will be more helpful
0 - @cyberweb8Posted almost 2 years ago
- To get the image to look like in the example, you can use the mix-blend-mode with the multiply value and include a opacity with the value of 0.8 and the color given in 'style-guide.md' as soft-violet.
img { background-color:hsl(277, 64%, 61%); opacity: 0.8; mix-blend-mode: multiply; }
The statistics info should be built using an unordered list element. More Info about unordered list element:📚The Unordered List element
I have done like this, just for visualization:
<ul class="service__details"> <li class="service__details-companies"> <p>10k+</p> <p>companies</p> </li> <li class="service__details-templates"> <p>314</p> <p>templates</p> </li> <li class="service__details-queries"> <p>12m+</p> <p>queries</p> </li> </ul>
If you have any questions or need further clarification, feel free to reach out to me.
Marked as helpful0@prithiviraj275Posted almost 2 years ago@cyberweb8 thanks a lot for your suggestions
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