-
Notifications
You must be signed in to change notification settings - Fork 471
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Gabriella Iofe: Project Business Site #378
base: master
Are you sure you want to change the base?
Conversation
…. stiel a little bit with css
… hover effect on the form
…luded the netlify link
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job on completing this project Gabriella! Let's go through some feedback.
HTML
- You've created a clear HTML structure by using semantic elements such as
<header>
,<main>
(and<form>
). - The structure of your hero section is well-organized, using a background image and an overlay for the text. This provides a visually appealing introduction to the website.
- Your form is very well structured! Just make sure to properly indent attributes:
<input
class="fill-out-form"
required type="text"
name="fname"
placeholder="First name">
CSS
- I really like what you did to the form styling-wise, but please be mindful and make all your elements readable by having enough color contrast.
- Nice usage of comments!
Looking forward to seeing your next projects - keep pushing forward! 🎉
<body> | ||
<h1>Business name 🌻</h1> | ||
<header> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is the closing tag? 👀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oups! Fixed it!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, I think you’ve done an excellent job. The code is easy to read and navigate, and the layout is beautiful with the smoke hero header, and the dancer as a background. The naming of the labels is perfect - I easily understand what field I’m looking at in the CSS code, without having to check in the HTML file. I'm also impressed that you used so many different input fields.
The only thing I noticed is that it doesn't appear that the page is specifically adapted for different screen sizes like mobile, tablet, and desktop - I can’t find any media queries (@media) present - still you managed to make the page adapt well to all devices. Well done!
Amazing work @gabster94 !
</div> | ||
|
||
|
||
<!-- Signup form --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed that you've used section comments like in your code, and I just wanted to say how awesome that is! These comments make the code so much easier to read.
placeholder="Last name"> | ||
</label> | ||
|
||
<label class="email"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Work like a charm!
placeholder="Email address"> | ||
</label> | ||
|
||
<label class="phone"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found the following to force the end-user to add digits in a field, and I believe it would work great in your code:
pattern="\d*"
title="Please enter a valid phone number"
<span>Phone</span>
<input
class="fill-out-form"
required
type="tel"
name="phone"
placeholder="Phone">
pattern="\d*" /*Digit Restriction: */
title="Please enter a valid phone number" /*Validation Message*/
</label>
<fieldset class="form-fieldset"> | ||
|
||
<label class="fname"> | ||
<span>First name</span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job using <span>
in instead of <div>
on the inline elements.
The assignment was to create a responsive business one-pager that included a form with a fieldset.
The code was asked to be written clean and with a clear semantic structure.
I started by drawing out an example for my wished end result. Not knnowing exactly what I wanted, I created a simple draft with which I played around with until I found the looks I was happy with.
Like in a previous project, I gave all "boxes" their borders and colors, so I could nore easily understand and see what was happening each time I did changes in my code. My browser was always open to that I could try out different thing, and follow each step.
With more time I would have a video in my hero section, a menu and a footer that includes more information. I would also spend more time on creating a better looking hero section with a well designed logo (which I haven't figured out how to do yet).
View it live
https://flow-movement.netlify.app/