PR Estefanny Project week 3 business website#372
PR Estefanny Project week 3 business website#372FannyEste wants to merge 11 commits intoTechnigo:masterfrom
Conversation
There was a problem hiding this comment.
Hi Estefanny, excellent work on your business site! 🍷
HTML
- Great use of a video in the header (
<video autoplay muted loop>), which creates an engaging first impression. You also provide an overlay with a clear call-to-action (CTA) button, making it easy for users to navigate to the sign-up form 🥳 - Nice usage of the
requiredattribute for some basic validation! - When the label is not wrapping the input, we need to add the
forattribute to the label, and theidattribute to the input:
<label for="name">First Name</label>
<input
required
id="name"
type="text"
placeholder="First Name"
name="name
/>
CSS
- The color scheme is well thought out, with yellow and black creating a strong contrast, especially in the footer. Love the yellow text on the images 🤩
- Try to always keep the styling to the CSS file, i.e. instead of using the ´b´ tag, add some rules for font-weight in the CSS.
- The hover effect on the button reduces the opacity significantly (opacity: .6). Consider lowering it slightly less (e.g., opacity: .8) to maintain better readability.
- I strongly suggest to give the form more space in mobile, to make it less cramped.

Clean Code
- Make sure you have proper indentation throughout your HTML file. Children should be indented one level (and one level only), such as:
<div class="image-gallery">
<img src="./images/our-wine.png" alt="our-wine" class="gray-image">
<img src="./images/month-wine.png" alt="month-wine">
<img src="./images/next-events.png" alt="events">
</div>
and siblings should have the same level of indentation, such as:
<label></label>
<input/>
We'll allow it this time, but please think about this going forward as it will make your code much easier to read and maintain.
- Remove unused code (e.g. the end of your CSS file)
You've done an amazing job! A few tweaks related to clean code and mobile optimization will make this project even more polished. Keep up the good work 🚀
There was a problem hiding this comment.
Well done Estefanny on creating such a nice wine business site!
I really like the title/name "wine me". Overall I think your project and your code is well structured with a great use of names and comments. Your deployed site looks great such as all the images and code. Your media queries also makes the page look good in different sizes and devices. I tried your sign up form and filled in some details, which worked perfectly.
Once again, well done Estefanny 👯
code/index.html
Outdated
| <!---hero section--> | ||
| <div class="hero-container"> | ||
| <!---Hero video--> | ||
| <video autoplay muted loop id="hero-video"> |
There was a problem hiding this comment.
Very nice video that works well on the deployed link and fits the theme perfect!
code/index.html
Outdated
| <input type="email" placeholder="Email" name="email" required> | ||
|
|
||
| <label> | ||
| <input type="checkbox" name="newsletter" style="margin-bottom: 15px;"> Sign me up for the newsletter |
There was a problem hiding this comment.
Here it would be nice to have the same indentation as the other rows, it looks like "me up for the newsletter" is a bit outside the rest of the code. You probably have the space 2 indentation on and I'm not sure how to do it but something we can research for the future :)
code/index.html
Outdated
| <input type="checkbox" name="newsletter" style="margin-bottom: 15px;"> Sign me up for the newsletter | ||
| </label> | ||
|
|
||
| <p>By creating an account you agree to our <a href="#" style="color: dodgerblue">term and conditions</a>.</p> |
There was a problem hiding this comment.
The same thing I mentioned on row 74 I think could be applicable here as well.
| border-radius: 8px; | ||
| } | ||
|
|
||
| button:hover { |
There was a problem hiding this comment.
Really nice with the hover effect here! I haven't used the this effect in my CSS yet but will definitely
take som inspiration from you here!
code/style.css
Outdated
| } | ||
|
|
||
| .logo-container img { | ||
| height: 50px; /* Ajusta la altura de la imagen según sea necesario */ |
There was a problem hiding this comment.
I can see some of your comments are in spanish here. Maybe it's good to do them all in english :)
| } | ||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
I tried your media queries you've written here on your deployed site and it worked really well! Good job :)
| opacity: .6; | ||
| transition: .3s; | ||
| } | ||
|
|
There was a problem hiding this comment.
There's some styling elements here I haven't tried before which sounds really interesting! Will try it out on my next project.
No description provided.