Design comparison
Community feedback
- @grace-snowPosted over 3 years ago
Hi @didhee
This solution looks good overall but has some issues and isn't quite matching the design.
I have a list of improvements for you that I hope will be helpful. It looks like a long list but most stuff is small/easy 🙂
- it's essential to have font sizes in responsive units, including body tag. You can just remove font size from the body completely on this one, as this challenge uses the browser's default size of 16px anyway. If styleguide said the base size should be different to 16px, you would add the font-size declaration to body, but in rem instead.
- Change h2 to h1. Change other headings to h2. It's very important that headings go in order, and you should always have one h1 on a page.
- Change button element for an anchor tag element. It's not a form and very likely this 'Sign up' would trigger navigation, not an action. That's why it must be an anchor tag.
- Use padding inside each box to stop content hitting the sides, not padding on individual elements. This is much easier to maintain. For vertical spacing like between paragraphs, that's where you should be using margin.
- Similar, don't use margin at explicit values to center
.boxes
. This is unnecessary and means you have to change it at each breakpoint. Instead, let the component be 100% width and give it a max-width. That would be the same for all screen sizes. Use flexbox or css grid to center the component on the page instead of margin. - Speaking of CSS grid, I recommend you have another go at this challenge, using css grid to position each box instead of flexbox. That is what this challenge is designed for really - it's a 2 dimensional layout that is perfect for CSS Grid. It would also mean your HTML box div would not be needed.
- Only add widths when you really need to. As said before, width 100% (if even needed! remember block elements are full width anyway) and min- or max-width are better techniques. e.g. The button is breaking out of its box at several screen sizes at the moment because it has a width on it.
- Check the design closely and use the styleguide to get closer...
- e.g. Your font sizes are larger than those defined in the stylesguide, and smaller on mobile. Font sizes shouldn't need to change.
- e.g. The colors are different on the background, button and price wording.
- e.g. The shadow is missing from button and doesn't match the design on the component
That's all from me. Keep going and have fun learning!
1@didheePosted over 3 years ago@grace-snow
Wow! This surely is a lot to grasp all at once. Thank you for your feedback. I really appreciate it. I'll have another go at this challenge with your candid feedback in mind. Thank you for taking out time to go through my solution. Being a self-taught developer, your feedback means a lot to me.
Once again, thank you. ❤
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