Design comparison
SolutionDesign
Community feedback
- @grace-snowPosted over 1 year ago
Hi
I hope this feedback is helpful.
- ALWAYS include a modern CSS reset at the start of your styles. Look up Andy Bell's reset and read the post that accompanies it
- To center a component on the viewport like in this design, use flex/grid properties on the body and
min-height: 100vh; min-height 100dvh
(in that order - the fallback is for older browsers who don't support dvh units yet) - Main must not have a width. Instead, set max-width only, and use rem units for it.
- I recommend placing the picture element in a wrapping div
- The image should be width 100%, needs the object-fit property so it does not distort at some screen sizes
- Remove the filter you are trying to use to get the effect on the image. You will need to give the image containing div a purple background color then use the mix-blend property and some opacity on the image itself
- The text area of the card is not really a section element, just use a div! This is allllll one component, an image and some text. So you could place a section element around the whole card if you wanted, but you shouldn't be making just one inner part of it into a section. But overall, sections are unnecessary in this challenge. They act just like divs unless labelled so it makes little difference, it's just a bad habit to get into if you use them inappropriately.
- Don't use padding on text-only elements like headings and paragraphs. Padding is for internal spacing, creating some space on the inner edges of boxes. They should only have vertical margins
- If you think this is a valuable image, you must describe it properly in the alt attribute. You never write words like 'image' in alt as it is already an image element. The alt has to either describe what the image looks like, or it should be left empty if the image is decorative.
- The 3 statistics here should be an unordered list with 3 list items inside, one for each stat (number + word). Those numbers can be wrapped in a strong or span with a class set to display block so they sit on their own line.
- The numbers in those stats (what I'm saying should be inside list items) make absolutely no sense as heading elements. It's an important principle to learn now that you must use heading elements appropriately. They are what communicates contents of a webpage, much like the contents page of a long document or book.
- I think you've forgotten to change text alignment on larger screens
- Font size must never be in px. Supper super important! NEVER. Use rem so that user font size preferences can be respected
- Remove the margin bottom from the text part of the card. As said previously, that is a box that should only have padding to create the internal space and keep it's child elements from touching its edges.
- This component (main) should have a little margin on all sides, or its wrapping element (the body) have a little padding, so that the component cannot hit the screen edges on smaller screens
Marked as helpful1
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