West Midlands Class 2 - Kawthar - HTMLCSS - Week 3 - #136
Conversation
pddys
left a comment
There was a problem hiding this comment.
Good job. This is a complex layout to create and you've managed to develop it using some advanced CSS and some smart use of flexbox.
The things you could improve upon is trying to be dry with your HTML and CSS and let features inherent to CSS such as specificity and the cascade do the work for you. This means greater use of classes instead of IDs, and being a bit more selective about which HTML you add.
Generally though this is good work, and it's good to see that you've challenged yourself by looking at CSS beyond your curriculum such as CSS grid.
| </head> | ||
|
|
||
| <body> | ||
| <div style="display: flex; flex-direction: column; align-items: stretch; margin: 0 auto; width: 90%;"> |
There was a problem hiding this comment.
You don't really want to mix inline css with external css
| <body> | ||
| <div style="display: flex; flex-direction: column; align-items: stretch; margin: 0 auto; width: 90%;"> | ||
| <div id="firstrow"> | ||
| <img src="https://www.clipartkey.com/mpngs/m/162-1624572_cake-logo-design-png.png" alt="cake logo" /> |
|
|
||
| </div> | ||
|
|
||
| <div class="carousel"> |
There was a problem hiding this comment.
Was this class name from another website? It's described as a carousel but in this case it's a fixed image.
A carousel would be closer to something like this: https://ganlanyuan.github.io/tiny-slider/demo/
| <img src="https://www.crazyforcrust.com/wp-content/uploads/2020/05/chocolate-cake-recipe-9.jpg" /> | ||
| </div> | ||
| <div class="slices"> | ||
| <div> |
There was a problem hiding this comment.
Do you need this empty div? You might have been able to lose it to keep your HTML drier
| </div> | ||
| </div> | ||
| </div> | ||
| <div style="align-self: center; width: 100%; "> |
There was a problem hiding this comment.
again, it's better to stick with external css rather than inline css and this can be hard to override as inline css has a lot of specificity.
| color: darkblue; | ||
| } | ||
| .content{ | ||
| display: grid; |
There was a problem hiding this comment.
Now this is pretty advanced CSS. Well done.
| .slices img{ | ||
| width: 90%; | ||
| height: 90%; | ||
| /* object-fit: cover; */ |
There was a problem hiding this comment.
This property should maybe have stayed in as it's stops the images from stretching to fill the space.
| width: 100%; | ||
| } | ||
|
|
||
| @media screen and (min-width:333px) |
There was a problem hiding this comment.
Good to see a media query, although the smaller device that you'd probably design for is the iPhone 5 which is 320px wide. So something like 400px might encompass a greater range of devices.
| { | ||
|
|
||
| .content{ | ||
| grid-template-columns: 40% 60%; |
There was a problem hiding this comment.
This is a good way to create layouts. Using media queries & grid.
| } | ||
|
|
||
| /* 660 max */ | ||
| @media screen and (min-width: 448px) |
There was a problem hiding this comment.
What layout effects are these media queries having?
Since the cakes container is already wrapping with flex-wrap, it's generally working responsively already.
|
Your coursework submission has been closed because nobody has interacted with it in six weeks. You are welcome to re-open it to get more feedback. |
Your Details
Homework Details