Design comparison
Solution retrospective
Hi this is my solution of "social - proof" challenge. I had a problem with making background, if anyone can help me with that, I would appreciate it. I hope it looks okey. Any other feedback is also welcome! ;)
Community feedback
- @grace-snowPosted over 3 years ago
Hi
I think this layout needs more work I'm afraid.
- It's in mobile view even on my MacBook pro screen. min-width 1400 is waaay too late to go desktop
- background images are missing on mobile
- fonts are missing/not applied
- review box background are missing
- content needs max-widths to stop elements / lines of text going really long
- headings aren't in order and some (like 'rated 5 star...' and 'verified...') shouldn't be headings
- star svgs need aria-hidden on them
- box shadow on main is a bit strange on really big screens... makes the content look like it's meant to be a huge card
As a general principle, our goal as frontend developers isn't just to create the designs to look correct at certain sizes as provided by the design files. Our role is to turn them into a dynamic document that will reflect the designers intention no matter what device or method by which they are accessed.
I hope these pointers help
2@sab0taszPosted over 3 years ago@grace-snow Hi! Considering mobile-view. I asked other people to open it on the desktop, and they didn't have the issue you mentioned. Any idea what could cause it at your side? Background is actually missing everywhere, I've looked how to use svg's as bg, I found this solution: background-image: url(../images/bg-pattern-top-desktop.svg); background-size: cover; background-size: 100% 100%; however it doesn't work. Is there some other way to do it? I had no idea there should review box bg. So should the text next to the stars be just <p>?
0@grace-snowPosted over 3 years ago@sab0tasz the mobile view on my desktop will affect lots of people. It is caused because you are not starting the desktop layout until 1400+ pixels wide. It should start much earlier than that
0@grace-snowPosted over 3 years agoI can see your background in the desktop preview above so what you did there has worked. It's just way too big because of the size being set to cover. I would set the size in viewport units maybe 🤔
I expect the method you're using is all fine, but this path may not be right though
url(../images/bg-pattern-top-desktop.svg)
The alternative is adding position relative to the body and adding in pseudo elements. You can then place the images in as backgrounds on that and it can give more control because toy can size and position the pseudo element itself as well as the background- properties
0 - @sab0taszPosted over 3 years ago
Welp, that's a lot to improve, however I expected that since I got stuck a bit, now I now what requires some additional work. Thanks for your tips. I'll do better on my second try! ;)
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