London Class 9 - Karleen Richards - HTML/CSS Coursework - Week 1 - #249
London Class 9 - Karleen Richards - HTML/CSS Coursework - Week 1#249karleenmsrichards wants to merge 14 commits into
Conversation
Added all the requirements for HTML/CSS Coursework Week 1 - 3 articles with article:firstchild
Also changed the title
Fixed HTML sematic elements and CSS names
PeteLindsell
left a comment
There was a problem hiding this comment.
You have created a really good looking design which is surprisingly difficult to accomplish. I have made quite a few comments which I am happy to discuss any that dont make sense.
This is really good for your first week of html css!
|
|
||
| main { | ||
| margin-bottom: 50px; | ||
|
|
There was a problem hiding this comment.
Being constant about line gaps will make the code easier to read. There is no need for this one here.
| ul li a:hover { | ||
| color: black; | ||
| } | ||
|
|
||
| .border-bottom { | ||
| border-bottom: 1px solid black; | ||
| } |
There was a problem hiding this comment.
Try to keep indention consistent throughout your code
| margin-bottom: 50px; | ||
| } | ||
|
|
||
| ul li a { |
There was a problem hiding this comment.
This will target all a tags within uls on your site using a class to just target the menu could be better
| } | ||
|
|
||
| .h1 { | ||
| font-size: 30px; |
There was a problem hiding this comment.
Setting font sizes as pixels will break one of the requirements of accessibility using rem here would be better
| .logo { | ||
| display: flex; | ||
| flex-direction: row; | ||
| font-size: 80px; | ||
| font-weight: bold; | ||
| justify-content: center; | ||
| padding-right: 68px; | ||
| text-decoration: underline; | ||
| } |
There was a problem hiding this comment.
It looks like you have done some really good experiments of flexbox throughout this file! After you get the effect you want it is worth disabling the attributes in the browsers inspector to check what they are actually doing. In this instance display: flex, flex-direction: row; and justify-content: center; will have no effect because they are properties to set on a parent element to affect its children and this element doesnt have any children
| left: 0; | ||
| bottom: 0; |
There was a problem hiding this comment.
These are not doing anything here. The footer is looking offset to the right because you are setting width: 100%; which the browser takes and then adds the padding to it. if you remove width the footer will naturaly fill the space
| <section class="top-right"> | ||
| <p class="top-right-p1">BRITAIN</p> | ||
| <p class="top-right-p2">SUBSCRIBE</p> | ||
| <img class="hburger-img" alt="Picture of a hamburger menu" src="https://encrypted-tbn0.gstatic.com/images?q=tbn:ANd9GcQhhwlrve66B8GHVhBRth6kDr6ml2SQR73R5g&usqp=CAU"> |
There was a problem hiding this comment.
Alt text is difficult you generally dont want to describe the image instead state its function. Here something like "Menu" or "Display main menu" would be more suitable
| <details> | ||
| <summary class="aside-border">Top outerwear this winter</summary> | ||
| </details> |
There was a problem hiding this comment.
the detail and summary tag are used as follows
<details>
<summary>Details</summary>
Something small enough to escape casual notice.
</details>
The summary is always displayed and the rest of the content of details is hidden until the summary is clicked
| <h3 class="stories">FASHION NEWS</h3> | ||
|
|
||
| <section class="article-container"> | ||
| <article> | ||
| <h1>Styling tips for the puffer</h1> |
There was a problem hiding this comment.
You should only have 1 h1 on a page which uniquely titles the content of the page. After a h1 you can have one or more h2 but you should not skip to h3 or h4
| <h1>Styling tips for the puffer</h1> | ||
| <p>There are various ways to style the puffer jacket but there are different rules which depends on numerous | ||
| factors. What's the length, what's the fit and what about the color. <strong>Author: Mishy Mish</strong></p> | ||
| <button><a class="link" |
There was a problem hiding this comment.
A link inside a button is not valid. A link should always have a href linking to part of a page or another url. A button will fire some other event such as submitting a form or opening a modal. Links are often designed to look like button but this shoul be done with css rather than a button element
Made changes to <section> tags to <div> tags.
1. Adjusted h tags by followings numerical increments 2. Fixed alt attributes for images to state function etc. 3. Removed button with link inside and added styles in CSS for links. Changed some semantic tags. 4. Used fixed font sizes, used fixed colour attributes.
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?
What did you find hard?
What do you still not understand?
Any other notes?