Account Deleted
Hi
Congratulations on completing the challenge
Landmarks are important for adding structure to your page and landmarks such as <header>, <main>, <nav> and <footer> aid navigation in assistive technologies such as screen readers and for keyboard accessibility.You do not need so many container elements you should collapse .mobile and .container into one and use <main>. You should collapse <article> and <div> into one and use <section> for your card containers, this will get rid of the <h1> validation issues.
Instead of <button> you should have used anchor tag <a> and styled it to look like a button. <button> is intended for actions carried out on the page whereas in this case the learn more button would take you to another page and is therefore a link.
In your media queries you do not need to duplicate the properties you want to keep, you need only write the styles you want to change.
I hope this helps.
Happy coding.
Marked as helpful
@nunes1886
Posted
@madzi-hove - Hey man, thanks a lot for the feedback. I just made some adjustments following your tips. Could you take a look and reevaluate? Thanks!
Account Deleted
@nunes1886
Hi Your markup looks much cleaner now and easier to read. Cleaner and simpler markup makes it easier to style the page. A better idea would be to make <body> the grid container, with this you would then be able to use justify-items and align-items to center .container.
.body {
display: grid;
grid-template-columns: 1fr;
justify-content: center;
align-items: center;
}
justify-items should work just as well as justify-content, play around with them and see what works. With grid doing the centering there will be no need for margin: 0 auto; in .container.
In your card styles you are doing too much duplication. Because your cards have so many similar settings you should add a class .card to each card and put all the similar styles in there eg.
.card {
display: grid;
grid-template-columns: 1fr;
line-height: 20px;
padding: 40px;
}
.card-one {
background-color: hsl(31, 77%, 52%);
}
.card-two {
background-color: hsl(184, 100%, 22%);
}
.card-three {
background-color: hsl(179, 100%, 13%);
}
}
You can cut down on so much of the code you wrote, also border-radius should be on .container only. You can also do the same thing as above to your .btn styles and cut your code further.
You also have a lot of duplication of styles in your media queries. If you set a style you do not have to put it in your media query unless you are changing it. eg
.container {
display: grid;
grid-template-columns: repeat(3, 1fr);
max-width: 700px;
margin: 0 auto;
margin-top: 5.6rem;
}
@media (max-width: 600px) {
.container {
grid-template-columns: auto;
max-width: 300px;
margin-top: 1rem;
}
}
The style are still active in the cascade so there is no need to repeat them.
Put font-family: 'Lexend Deca', sans-serif; in body, the other tags will inherit the font.
Put font-family: 'Gill Sans', 'Gill Sans MT', Calibri, 'Trebuchet MS', sans-serif; in .btn and it will be applied to all three links.
I hope this helps. Feel free to reply if anything is not clear
Happy coding.
Marked as helpful
