Scot-Class-5/Karma-Webpage - #232
Conversation
| body { | ||
| font-family: 'Roboto', sans-serif; | ||
| -webkit-font-smoothing: antialiased; | ||
| font-size: 16px; |
There was a problem hiding this comment.
| font-size: 16px; |
We should never override the browser font size setting, as this means if the user changes their browser setting font size to 24px then this webpage doesn't increase in size. Also, it's good practice to use relative units like % or rem for font-size instead of px so elements change proportionally when the user changes font size.
| header{ | ||
| display: flex; | ||
| justify-content: space-evenly; | ||
| align-content: center; |
There was a problem hiding this comment.
| align-content: center; | |
| align-items: center; |
It can be confusing because of the similar words, but align-items is the one we want here to centre the elements vertically
| nav ul{ | ||
| display: flex; | ||
| list-style: none; | ||
| justify-content: space-evenly; | ||
| width: 50rem; | ||
| padding: 1rem 0rem; | ||
|
|
||
| } |
There was a problem hiding this comment.
| nav ul{ | |
| display: flex; | |
| list-style: none; | |
| justify-content: space-evenly; | |
| width: 50rem; | |
| padding: 1rem 0rem; | |
| } | |
| nav ul li { | |
| display: inline-block; | |
| list-style: none; | |
| justify-content: space-evenly; | |
| padding: 1rem; | |
| } |
Instead of setting a specific width for the menu links and using flexbox, we can turn the links into inline boxes so they are horizontal and they will have their own natural width
| color:rgb(197, 189, 189); | ||
| } | ||
|
|
||
| nav ul a:hover{ |
There was a problem hiding this comment.
| nav ul a:hover{ | |
| nav ul a:focus, | |
| nav ul a:hover{ |
When adding hover, we should also add focus so keyboard users can use the Tab key to jump to this and also get the same style
| color: rgb(105, 102, 102); | ||
| font-weight: 500; | ||
| } | ||
| x-special/nautilus-clipboard |
There was a problem hiding this comment.
| x-special/nautilus-clipboard |
Looks like this was accidentally left in!
| width: auto; | ||
| margin: 0 auto; | ||
| padding-top: 4rem; | ||
| box-shadow: 0px 1px 0px rgb(189, 187, 187); |
There was a problem hiding this comment.
| box-shadow: 0px 1px 0px rgb(189, 187, 187); | |
| border-bottom: 1px solid rgb(189, 187, 187); |
For a single line, we usually use border instead of box-shadow, so we could change it here
| footer section{ | ||
| display: flex; | ||
| } |
There was a problem hiding this comment.
| footer section{ | |
| display: flex; | |
| } |
The social media icons are already horizontal so we don't need to use flex here
| footer img{ | ||
| margin: 0.5rem; | ||
| height: 1.5rem; | ||
| padding: 1rem 1rem; |
There was a problem hiding this comment.
| padding: 1rem 1rem; | |
| padding: 1rem; |
We usually use the shorthand so we can change it to just 1rem
| <img src="img/first-background.jpg"> | ||
| <div class="hero-text"> |
There was a problem hiding this comment.
| <img src="img/first-background.jpg"> | |
| <div class="hero-text"> |
We can remove these, like I mentioned in my CSS suggestions
| <img src="img/first-background.jpg"> | ||
| <div class="hero-text"> | ||
| <h1>Introducing Karma</h1> | ||
| <h2>Bring WiFi with you, everywhere you go.<h2> |
There was a problem hiding this comment.
| <h2>Bring WiFi with you, everywhere you go.<h2> | |
| <p>Bring WiFi with you, everywhere you go.<p> |
This doesn't seem like a heading to me — there is already a heading above it — so we could change this to a p
2 participants
Volunteers: Are you marking this coursework? You can find a guide on how to mark this coursework in
HOW_TO_MARK.mdin the root of this repositoryYour Details
Homework Details
Notes
What did you find easy?
html codes
What did you find hard?
css and aligning the items
What do you still not understand?
flexbox
Any other notes?