Skip to content
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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

gabster94
Copy link

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/

Copy link
Contributor

@HIPPIEKICK HIPPIEKICK left a 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>
Copy link
Contributor

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? 👀

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oups! Fixed it!

Copy link

@joheri1 joheri1 left a 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 -->
Copy link

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">
Copy link

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">
Copy link

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>
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants