Three columns , made by using basic html and css tags
Design comparison
Community feedback
- @Islandstone89Posted 10 months ago
Hey, here is some feedback - hope it helps :)
HTML:
-
<main>
should never be inside<header>
, as they are different landmarks. Remove the<header>
, it is not needed here. A<header>
is used for content that repeats on all pages, like a logo and a navigation bar. -
The icons are decorative, so the alt text must be empty:
alt=""
. -
There should only be one
<h1>
on a page. Given there are 3 similar headings, I would change all of them into a<h2>
. -
The headings should be written with normal capitalization: "Sedans", "Suvs", and "Luxury". You then use
text-transform: uppercase
to change the capitalization in CSS. -
"Learn More" would navigate to another page, hence it is not a button but a link.
CSS:
-
Performance-wise, it's better to link fonts in the
<head>
of the HTML than using@import
. -
It's good practice to include a CSS Reset at the top.
-
Remove the margins from the card container. To center the card container horizontally and vertically, use Flexbox on the body:
display: flex; flex-direction: column; justify-content: center; align-items: center; min-height: 100vh;
-
The card container needs a
max-width
in rem - this prevents it from getting too wide on larger screens. -
font-size
must never be in px. This is bad for accessibility, as it prevents the font size from scaling with the user's default setting in the browser. Use rem instead. -
line-height
must also never be inpx
. -
Paragraphs have a default value of
font-weight: 400
, so there is no need to declare it. -
Give the "Learn More" links a
display: inline-block
, and usepadding
to size them. Remove thewidth
andheight
. In general, I would recommend not using%
for these types of properties. -
Ideally, you want to do mobile styles as the default and use media queries for larger screens. Not all challenges need media queries, but this one certainly does. Add
flex-direction: column
on the card container, this makes the cards stack on top of each other. Set a media query in rem (use the "Responsive mode" in DevTools to experiment when the layout needs to change, I would assume somewhere around 50rem would be OK), that switches toflex-direction: row
- this makes the card line up side by side. NB: I would use Grid for this challenge, as that's usually the better option when we want equal columns. To create equal columns in Flexbox, addflex-basis: 100%
on the individual cards. If you don't want to write it on all 3 cards, you can write:
main > * { flex-basis: 100%; }
This selects all direct children of
main
, and makes them equally wide.Good luck! :)
Marked as helpful0 -
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