London-9-Karleen-Richards-HTML/CSS-Coursework-Week3 #406
Conversation
1. Added HTML/CSS to to accommodate mobile phone screens with a max-width 540px
karleenmsrichards
changed the title
1.Added media query for max-width 900px 2.Adjusted header and grid images to fit this width screen. 3.Removed Hamburger menu for this width. 4.Added nav bar with list items.
Used CSS grid to align images on web page Also fixed padding an margins for max-width 540px screen
1. Used CSS grid for images 2.Fixed margins and padding 3.changed color and layout
1.Changed menu for various breakpoints 2.Adjusted hero styles for breakpoints 3.Changed positions of log for different breakpoints
Added different layout Updated HTML/CSS to accommodate changes Added 2 grids to the layout
1.Adjusted color scheme of buttons and logo 2.Added more text to Hero
1.Added footer icons etc 2.Added padding and margin 3.Aligned footer to change positioning at breakpoint
1.Displayed site menu 2.Removed hamburger menu 3.Fixed padding and margin in the nav menu to fit on min-width 540px screen
Adjusted the width and height of the hero image so that it is responsive
| <span class="bar"></span> | ||
| </div> | ||
|
|
||
| <p class="nav-bar-p">Lee-Lee Bakes</p> |
There was a problem hiding this comment.
Its good practice to wrap the logo/site name in an anchor that links to the homepage of the site
| </div> | ||
| </header> | ||
|
|
||
| <div class="nav-menu"> |
There was a problem hiding this comment.
The div with a class of nav-menu could be a nav element
| </div> | ||
| </section> | ||
|
|
||
| <section class="imgs-section"> |
There was a problem hiding this comment.
Sections should almost always have headings and always have some text content so this should be a div or a ul of lis but as there is no text content then div is ok
There was a problem hiding this comment.
Sure I would amend it.
| .nav-menu { | ||
| display: none; | ||
| padding: 0; | ||
| } | ||
|
|
||
| .nav-ul { | ||
| list-style: none; | ||
| display: none; | ||
| flex-direction: row; | ||
| gap: 10px; | ||
| } | ||
|
|
||
| .nav-list { | ||
| display: none; | ||
| } | ||
|
|
||
| .nav-item { | ||
| padding: 0; | ||
| font-size: 1em; | ||
| text-transform: uppercase; | ||
| color: #2c2828; | ||
| font-weight: bold; | ||
| text-decoration: none; | ||
| display: none; | ||
| } |
There was a problem hiding this comment.
There is no need to have display:none; on all of these elements if you have it on .nav-menu it will hide all its children then if you add
.nav-menu.active {
display: block;
}
it should all work
| <!-- Add a link to your css file here --> | ||
| </head> | ||
|
|
||
| <body> | ||
| <!-- Add your markup here --> | ||
| <header> | ||
| <p class="call-to-action-p">Sign up and get <strong>£5 OFF*</strong> your first order <button class="call-to-action-btn">SIGN UP</button></p> |
|
|
||
| .imgs-container { | ||
| display: grid; | ||
| grid-template-columns: repeat(auto-fill, minmax(200px, 1fr)); |
There was a problem hiding this comment.
Good use of auto-fill to create a responsive grid without the need of media queries
| .imgs-container { | ||
| display: grid; | ||
| grid-template-columns: repeat(auto-fill, minmax(200px, 1fr)); | ||
| grid-template-rows: repeat(2, 150px); |
There was a problem hiding this comment.
150px is causing the images to be squashed, you can either remove it or set object-fit: cover; to the image so that it is cropped instead of squashed
| } | ||
|
|
||
| .nav-menu { | ||
| display: contents; |
There was a problem hiding this comment.
display: contents; is probably not the thing you want here this is used to ignore the element but wont show the hidden children. It doesnt have the best support and I have never used it. You should go for display: block; or display: absolute; if you dont want to push the content down when it opens
| </section> | ||
| <footer> | ||
| <div class="footer-icons"> | ||
| <a href="#"><img alt="twitter" class="footer-imgs" src="./images/facebook-icon.svg"></a> |
There was a problem hiding this comment.
This should be an image inside the anchor and to make it accessible it would need either alt="LeeLee bakes twitter" on the image or aria-label="LeeLee bakes twitter" on the anchor tag
|
I will make the necessary changes that you suggested. |
#Live Demo
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?
It was easy to understand what was expected
What did you find hard?
It was difficult to size images
What do you still not understand?
I still do not understand how to size images properly
Any other notes?