Design comparison
Solution retrospective
Hello everybody,
this is my first submission on Frontend Mentor and I'd be grateful for some feedback, especially on whether my formatting is efficient or if I should have followed a different approach somewhere.
Thank you!
Community feedback
- @adimidaniaPosted over 3 years ago
Hey Michael! Welcome to Fronted Mentor's community! Congratulations on completing your first project. Just a few notes :
- I guess the background image for the mobile version needs to be added for the screens with a width of 375px and less (according to the challenge).
- Your card is looking great on large screens, but on small screens (mobile version), It needs to be adjusted, in other words, It's not fully responsive.
- The card needs to be centered. One way to do this is :
body { display:flex; align-items:center; justify-content:center; height:100vh; margin:0; }
Marked as helpful0@MichaelGeehPosted over 3 years ago@adimidania Hi Dania, thanks for your points! I hope it looks better now on small screens? I'm not sure I understand the point about card centering. At least in Chrome the card seems horizontally centered and vertically only slightly more to the top than to the bottom. Does it look different for you?
I suppose with the code you posted I need a further adjustment to the main card because it has like 20px of white space above of it. I'll try to figure it out.
Thanks again :)
1@adimidaniaPosted over 3 years ago@MichaelGeeh You are always welcome Michael! Yes! It's looking much better on small screens now. Actually, the card is well centered horizontally, but vertically It's not (slightly more to the top than to the bottom), and in order to fix this, you can use the code I posted, or another approach. You are welcome again! keep coding!
Marked as helpful0@MichaelGeehPosted over 3 years ago@adimidania Thanks again Dania for taking the time! I will give that a go.
1 - @dusanlukic404Posted over 3 years ago
Hey Michael! There are a few things you could change in your project.
- You forgot to make this card responsive so I recommend you to add at least one media query
- Try to add transition property on links and buttons when are you hovering over them. That's not important but it will certainly improve the impression on the observers
- Change the font-weight of button text to bold and change the color of (change) link
- For next project try to avoid vertical and horizontal scrollbar
Marked as helpful0@MichaelGeehPosted over 3 years ago@dusanlukic404 Hi Dusan, thanks for your suggestions! I have tried to correct the points you mentioned. Does it look better now?
1@dusanlukic404Posted over 3 years ago@MichaelGeeh Yes, it looks awesome. I forgot also to recommend you using cursor:pointer on buttons. They are very helpful also on links but links have by default value pointer on cursor property.
Marked as helpful2@MichaelGeehPosted over 3 years ago@dusanlukic404 Thanks again Dusan! I appreciate that you took the time to review my solution and I have now added the cursor: pointer (though not yet committed).
1
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