C22-Phoenix-Marjana-Khatun #16
Conversation
| #mainbar{ | ||
|
|
||
| grid-column: 2/5; | ||
| grid-row: 2/4; | ||
|
|
||
| } | ||
|
|
||
| #footerbar{ | ||
|
|
||
| grid-column: 2/span 3; | ||
| grid-row: 4/5; | ||
| align-self: end ; | ||
|
|
||
| } | ||
|
|
||
| a:hover { | ||
| background-color: darkgreen; | ||
|
|
||
| } | ||
|
|
||
| footer { | ||
| padding-top: 120px; | ||
| } |
There was a problem hiding this comment.
Spacing inside of rule sets across the CSS files is inconsistent, I suggest removing any blank lines inside of rule sets then keeping a single blank line between each rule set.
| margin-bottom: 0; | ||
| padding-top: 50px; | ||
| padding-bottom: 0; | ||
| font-family:Papyrus ; |
There was a problem hiding this comment.
We should be able to remove this line. Since we set font-family to Papyrus in the shared styles.css file, the value should be inherited if we import both styles.css and facts.css into the facts.html page.
| width: 1400px; | ||
| height: 600px; | ||
| justify-content:center; | ||
| } |
| <h1>Fun Facts</h1> | ||
| </header> | ||
| <section class="container"> | ||
| <div> | ||
| <h3>Indoor plants improve air quality</h3> |
There was a problem hiding this comment.
In the MDN docs it's highly suggested to not skip headings:
A common navigation technique for users of screen reading software is jumping from heading to quickly determine the content of the page. Because of this, it is important to not skip one or more heading levels. Doing so may create confusion, as the person navigating this way may be left wondering where the missing heading is.
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/Heading_Elements#navigation
Here the suggestion would be to use h2 instead of the h3s and style the h3s as needed to match the smaller font size desired.
| <div> | ||
| <h3>Indoor plants improve air quality</h3> | ||
| </div> | ||
| <div> | ||
| <h3>Plants can filter sound and aborb</h3> | ||
| </div> | ||
| <div> | ||
| <h3> Indoor plants can sense light</h3> | ||
| </div> | ||
| <div> | ||
| <h3>Plants can communicate through root system and emit chemical signals</h3> | ||
| </div> | ||
| <div> | ||
| <h3>Plants can touch sense</h3> | ||
| </div> | ||
| <div> | ||
| <h3>Indoor plants remove toxins from air</h3> | ||
| </div> | ||
| <div> | ||
| <h3>Indoor plants helps boost mood and reduce stress</h3> | ||
| </div> |
There was a problem hiding this comment.
I recommend replacing these nested divs with semantic tags like article. Where else across the files do you see divs that could be replaced with a semantic tag?
No description provided.