Hello everyone. I tried to do this challenge as best as possible. If anyone can check the code quality, what can be improved? Can you check the correctness of the CSS file? There is any recommended structure or order to write from top to bottom the CSS ? Any other feedback is highly appreciated. Thank you
Larrie
@AnirogAll comments
- @soitirakisSubmitted over 2 years ago@AnirogPosted over 2 years ago
I read that media queries should always go at the bottom of CSS.
0 - @soitirakisSubmitted over 2 years ago
Hello everyone. I tried to do this challenge as best as possible. If anyone can check the code quality, what can be improved? Can you check the correctness of the CSS file? There is any recommended structure or order to write from top to bottom the CSS ? Any other feedback is highly appreciated. Thank you
@AnirogPosted over 2 years agoHi, your CSS looks really clean, I used a different approach to get the card exactly in the center of the screen using viewport height, viewport width and flexbox.
.container { width: 100vw; height: 100vh; display: flex; align-items: center; justify-content: center; }
There is any recommended structure or order to write from top to bottom the CSS ?
In CSS the code at the bottom overrides any styles above that style, so if you had:
h1 { color: red; } h1 { color: blue; }
The h1 tag would be blue.
0 - @bobbe86Submitted over 2 years ago
This challenge took me a couple hours to finish and I'm wondering..
What minor things would you change about the HTML and CSS layout and flow to follow best practice standards?
Did I use one too many divs? Not enough divs?
Should the container have a max-width of 375px?
Thanks in advance for your help!
@AnirogPosted over 2 years agoI only used one div
.container
and then styled the elements in the.container
like this:.container { . . . } .container img { . . .} .container h1 { . . . } .container p { . . . }
Not sure if this is best practice but it was easier for me to keep the index.html file small.
I think what you did using
max-width: 375px;
was better than what I did.I just used
width: 322px;
Your resets could be made simpler by using a wildcard selector
*
which styles everything so instead of:/* Remove default margin */ body, h1, h2, h3, h4, p, figure, blockquote, dl, dd { margin: 0; }
You could do this:
* { margin: 0; padding: 0; box-sizing: border-box; }
0 - @helenborges201Submitted over 2 years ago@AnirogPosted over 2 years ago
- In index.html the h1 tag is missing the
1
from the closing tag.
<h1>Improve your front-end skills en by building projects</h>
Should be
<h1>Improve your front-end skills by building projects</h1>
- Add colour and padding to h1 in style.css
- Add padding to paragraph text in style.css
h1
h1 { padding: 0px; }
Should be
h1 { padding: 0px 20px 15px; color: hsl(218, 44%, 22%); }
p
p { padding: 0px 20px 15px; }
I'm not exactly sure if this the the best way to add padding but it looks more like the original design :)
Marked as helpful0 - In index.html the h1 tag is missing the