london-class-8/shadi-safaee/karma-week-2#312
Conversation
beckyf0keefe
left a comment
beckyf0keefe
left a comment
There was a problem hiding this comment.
Over all it looks like you're getting there with semantic tags, just make sure to use it for all the relevant content. The site visually was looking good so overall well done with the css, just make sure that you are using flexbox where appropriate and avoiding using ids as css selectors.
As a goal if you have time please change the hero image to a css background image and change the alt tags to be more descriptive.
| <main> | ||
| <div class="container"> | ||
| <img | ||
| class="big-image" | ||
| src="img/first-background.jpg" | ||
| alt="working with your tablet" | ||
| /> | ||
| <div class="text-container"> | ||
| <h1 class="first-text"> | ||
| Introducing Karma | ||
| </h1> | ||
| <h5 class="second-text"> | ||
| Bring WiFi with you, everywhere you go. | ||
| </h5> | ||
| <button class="button1"> | ||
| Learn More | ||
| </button> | ||
| </div> | ||
| </div> | ||
| </main> |
There was a problem hiding this comment.
When we use the main tag, is should contain all the content on the page that isn't in the header or footer, where do you think the closing tag should be moved to instead?
| </head> | ||
| <col> | ||
| <!-- Nav bar --> | ||
| <header> |
There was a problem hiding this comment.
Good use of the header semantic tag
| <nav class="topnav"> | ||
| <img class="logo" src="img/karma-logo.svg" alt="logo" /> |
There was a problem hiding this comment.
Should the logo image be inside the nav tag? Nav tends to only be used for the set of links used for navigation
| <div class="footer"> | ||
| <hr> | ||
| <p>Join us on</p> | ||
| <footer> | ||
| <div class="social"> | ||
| <img class="social_icon" | ||
| src="img/twitter-icon.svg" | ||
| alt="twitter" | ||
| /> | ||
| </div> | ||
| <div class="social"> | ||
| <img class="social_icon" | ||
| src="img/facebook-icon.svg" | ||
| alt="facebook" | ||
| /> | ||
| </div> | ||
| <div class="social"> | ||
| <img class="social_icon" | ||
| src="img/instagram-icon.svg" | ||
| alt="instagram" | ||
| /> | ||
| </div> | ||
| </footer> | ||
| <p class="footer-text"> | ||
| <span>©</span> | ||
| Karma Mobility, Inc | ||
| </p> | ||
|
|
||
| </div> |
There was a problem hiding this comment.
Having a div with class name called footer and then a separate footer could get a little confusing, remember the footer should be the whole section at the bottom of the page
| <span class="sub-title">Internet for all devices</span> | ||
| </div> | ||
| <div id="item"> | ||
| <img class="icon" id="thumbnail" src="img/icon-coffee.svg" alt="coffee"/> |
There was a problem hiding this comment.
make sure with your alt tag describes what you are seeing, imagine you couldn't see the page, would you want someone to describe this image to you as "coffee" or as "orange line art of a steaming coffee cup"?
| width: 15vw; | ||
| height: 15vw; | ||
| } | ||
| #thumbnail:hover { |
There was a problem hiding this comment.
good to see you using the :hover selector, it will be really important in the future
| text-align: center; | ||
| } | ||
|
|
||
| #title-main { |
There was a problem hiding this comment.
try not to use ids in your css, if we make this a class then we can reuse elsewhere if we need to
| .big-image { | ||
| width: 97%; | ||
| margin: 0; | ||
| } |
There was a problem hiding this comment.
On the image that we are recreating the hero image goes all the way across the screen, a keen eye for detail is really important to make sure that we are recreating it exactly to what the image shows
| font-size: 1.5vw; | ||
| } | ||
|
|
||
| .article-summary { |
There was a problem hiding this comment.
this is one way to do this but after what we learned in class last week, try to have a go at using flex-box instead to achieve the same effect
| footer { | ||
| display: flex; | ||
| justify-content: center; | ||
| } |
Based on TA`s comments, some codes are changed
There was a problem hiding this comment.
Thank you @beckyf0keefe for all your comments, I really appreciate it. All updated based on your comments.
4 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? All easy
What did you find hard? nothing
What do you still not understand? nothing
Any other notes? -