Conversation
raulriato
left a comment
raulriato
left a comment
There was a problem hiding this comment.
You're using fixed width for the body, this is causing side scrolling depending on the size of the viewport. Also, your page does not work as intended with responsivity.
IanLuan
left a comment
IanLuan
left a comment
There was a problem hiding this comment.
Good job, Carlos! Your landing page looks great. However, there are a few things you need to fix. I left some comments on your code, but in general:
- You’re not applying the BEM methodology correctly. Review your styles and structure the blocks and elements properly so your code becomes easier to maintain. Because BEM isn’t being followed, there are several repeated styles.
- You’re repeating styles across different media queries. Make sure to define shared styles once and only override specific properties for other screen sizes.
- Review the checklist items and ensure all of them are followed.
| <div class="nav_baker"> | ||
| <div class="baker_lab"></div> | ||
| </div> |
There was a problem hiding this comment.
Logos in header and footer should be links to home page.
| @@ -6,14 +6,213 @@ | |||
| name="viewport" | |||
| content="width=device-width, initial-scale=1.0" | |||
| /> | |||
| <title>Title</title> | |||
| <title>Bakerlab - Creative Bakery</title> | |||
| <link | |||
| rel="stylesheet" | |||
| href="./styles/main.scss" | |||
| /> | |||
| </head> | |||
src/styles/_desktop_1200.scss
Outdated
| display: none; | ||
| } | ||
|
|
||
| header { |
There was a problem hiding this comment.
Avoid styling HTML tags directly, as this breaks BEM rules. Use class selectors instead.
src/styles/_desktop_1200.scss
Outdated
| } | ||
|
|
||
| .title_product_main { | ||
| font-family: Inter, sans-serif; |
There was a problem hiding this comment.
All texts use this font family, so you can apply it globally.
src/styles/_desktop_1200.scss
Outdated
| .product-1 { | ||
| width: 402px; | ||
| height: 512px; | ||
| background-image: url(../images/product_1_desktop.png); | ||
| background-size: contain; | ||
| background-repeat: no-repeat; | ||
| background-position: top center; | ||
| display: flex; | ||
| flex-direction: column; | ||
| justify-content: flex-start; | ||
| padding-top: 530px; | ||
| box-sizing: border-box; | ||
| gap: 15px; | ||
| } |
There was a problem hiding this comment.
You’re repeating these styles for every product. Apply the BEM methodology to your styles so you can reuse them more effectively.
src/index.html
Outdated
| </div> | ||
|
|
||
| <div class="product_images-1"> | ||
| <div class="product-1"> |
There was a problem hiding this comment.
Use an <img> tag to display the product image instead of using a background image. Also remember to follow the checklist item:
Pictures in the blocks "What We Bake" should be animated on hover
|
I think I managed to do everything I was instructed to do |
IanLuan
left a comment
IanLuan
left a comment
There was a problem hiding this comment.
Congrats, Carlos! It looks really great. I’m going to leave a few tips you can use to improve your project (but it already looks very good):
1- Adjust your hero_container size so the "creative bakery" text doesn’t wrap.
2- Remove the max-width from your body. On large screens, the body is smaller than the viewport width, so some elements like the footer aren’t full width.
3 participants
DEMO LINK
I made a slight change to the modal layout compared to the Figma design