Can someone review my first work please?

Hi everyone,

I am only on my third day of FreeCodeCamp and I am currently working on the first projects. I have managed to pass the first 2, not without difficulty, I had to look at the Objective’s code to understand a few things, but it has taught me loads already.

Could anyone have a look at my 2nd project and let me know if I am doing things right? What kind of things should I do differently?

here is the link to it:
https://codepen.io/ValerioCipolla/pen/poevRye

Also, I think I have used labels correctly, but I am struggling to understand why I need them, why do we use labels?

And finally, I really wanted to center the text and the boxed in the middle of the page, but I can’t seem to be able to do that. Can anyone help me with that?

Thanks a lot!

Kudos to you for embarking on this coding journey! Kick it into low gear as you are still learning the basics. Experiment often with new ways of doing things by exploring how others accomplish similar tasks. With programming, there is no right way to do things, but there are certainly best practices to follow. This particular project can be found done by thousands before you. Dig up some and find out what others are doing differently. You can learn a bunch by looking at the work of others.

Bottom line: your project passes all tests. Very good.

After looking at your html, I will say that you could be doing things better. As a general rule, an HTML document will never ever have more than one <body> element, therefore assigning an id to your <body> is not necessary. It’s just extra html code. You can always select the body with CSSby using body { } or JS using document.body.

The same thing could be said for an <h1> element as typically a page should never have more than one of these on it. You can select this element with h1 { } in CSS.

One thing I noticed is that you are using CSS to text-align: center your h1 and p#description. I recommend replacing their id attributes with a class. A great solution would be to assign a class of 'text-center" to both of them.

<h1 class="text-center">freeCodeCamp Survey Form</h1>
<p class="text-center">Thank you for taking the time to help us improve the plaform</p>

Of course within your CSS you would have to select this class and assign a property value of center to text-align.

.text-center {
   text-align: center;
}

By making this change, you could reuse this same chunk of CSS to center text-align any elements by simply assigning a class to it.

You should take a peek at the display property, specifically how display: block works. Lots of elements are set to display: block by default, which will let you skip the <br> found throughout your code. Line breaks are rarely needed with modern code and have a tendency to ‘muddy’ up HTML.

Within this project you have used labels inconsistently. Sometimes they are used correctly while others they aren’t. When a label is used properly, the for property is assigned with the same value as it’s respective <input name>. What this does is tells the browser which input is intended if the <label> is clicked rather than the input. Each <label> that has a for attribute assigned is following the correct structure. They do not need to have an id property unless you need a way to select it using CSS of JS after the fact. However, I noticed that some or all of your <input> tags are missing a name property. Because of this, your <label>, despite having a for attribute, will not know which input you are referring to. You need to make sure that all of your <input> tags have a name property with the corresponding name that matches the for attribute from the <label>. Users can come across a web page using a variety of tools outside of a browser, such as screen readers, or even a printer. Screen readers do not even consider the CSS on a page and solely rely on your HTML to determine the page. Semantic HTML like <main>, <section>, <label>, etc. make a huge impact on the finished product.

You have a lot of selectors in CSS but not actually doing anything. You could and should clean up any loose ends here.

You an center your entire page by selecting the body using CSS and assigning it a display property of absolute then using position properties and finally a transform to center everything. A bit complicated to see in action, but successful at achieving the results.

body {
    position: absolute;
    top: 50%;
    left: 50%;
    margin-right: -50%;
    transform: translate(-50%, -50%);
}
2 Likes

Thank you very much. I will take on board what you said. I will look at other people’s work on the same project and I will keep working on it.

Thanks a lot, I appreciate your help

COOL survey form
but I think that font size should be little big and u can also use more designs or colors to make the background beautiful

1 Like

@mnichols08 I really liked your review of this. A lot of very useful tips and explanations here that are especially useful to us newbies! I should post my projects and hope for that level of clear, direct feedback lol!

1 Like

This topic was automatically closed 182 days after the last reply. New replies are no longer allowed.