Skip to content

html-form #798

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.

Sign up for GitHub

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

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

html-form #798

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
22d3147
html-form
DominikaKarmel Jun 1, 2023
507605a
layout_html-form
DominikaKarmel Jun 5, 2023
74e2d31
layout_html-form
DominikaKarmel Jun 7, 2023
72c24ef
layout_html-form
DominikaKarmel Jun 12, 2023
8c89f09
layout_html-form
DominikaKarmel Jun 13, 2023
32231db
layout_html-form
DominikaKarmel Jun 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions readme.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# HTML form
Replace `<your_account>` with your Github username and copy the links to Pull Request description:
- [DEMO LINK](https://<your_account>.github.io/layout_html-form/)
- [TEST REPORT LINK](https://<your_account>.github.io/layout_html-form/report/html_report/)
- [DEMO LINK](https://DominikaKarmel.github.io/layout_html-form/)
- [TEST REPORT LINK](https://DominikaKarmel.github.io/layout_html-form/report/html_report/)

> Follow [this instructions](https://mate-academy.github.io/layout_task-guideline/#how-to-solve-the-layout-tasks-on-github)
___
Expand Down Expand Up @@ -40,7 +40,7 @@ Create HTML page with form. On form submit send form data to `https://mate-acade
- Age should be at least `1` and at max `100` with a default value of `12`
- The email field should have placeholder value: `email@example.com`.
- Text fields should have `autocomplete="off"`.
- `Submit` button should have a `type="submit"`
- `Submit` button should have a `type="submit"`
- Vertical distance between inputs should be `10px`
- Vertical distance between groups should be `20px`
- Any other styles should be browser default
Expand Down
182 changes: 181 additions & 1 deletion src/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,188 @@
<title>HTML Form</title>
<link rel="stylesheet" href="./style.css">
</head>

<body>
<h1>HTML Form</h1>
<script type="text/javascript" src="./main.js"></script>

<form action="https://mate-academy-form-lesson.herokuapp.com/create-application" method="post"
class="form">
<fieldset class="form__group">
Copy link

@Radoslaw-Czerniawski Radoslaw-Czerniawski Jun 1, 2023

Choose a reason for hiding this comment

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

Generally you don't have any empty lines between elements making this code very hard to read, for example you should always have empty line between siblings:

<div>
  <span>first child</span>
  
  <p>second child</p>
</div>

there is an exception to this rule, for example when you have a bunch of similar elements like in ul

<ul>
  <li>text</li>
  <li>text</li>
  <li>text</li>
  <li>text</li>
</ul>

<legend>Personal information</legend>

<label class="form__field">
Surname:

<input
type="text"
name="surname"
autocomplete="off"
>
</label>

<label class="form__field">
Name:
Copy link

@Radoslaw-Czerniawski Radoslaw-Czerniawski Jun 1, 2023

Choose a reason for hiding this comment

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

Such text should be a label for appropriate input so upon clicking on it correct field is focused


<input
type="text"
name="name"
autocomplete="off"
required
>
</label>

<label class="form__field">
How old are You?

<input
type="number"
name="age"
min="1"
max="100"
value="12"
>
</label>

<label class="form__field">
Full date of birth:

<input
type="date"
name="birthdate"
>
</label>

<div>
<label>
I accept the term of the agreement

<input
type="checkbox"
name="agreement"
required
>
</label>
</div>
Copy link

@DorotaLeniecDev DorotaLeniecDev Jun 12, 2023

Choose a reason for hiding this comment

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

Why you wrapped this label with div element instead of adding form_field class as for other labels? You should try to be consistant with the way you write your code :)

</fieldset>

<fieldset class="form__group">
<legend>Registration</legend>

<label class="form__field">
<span>E-mail:</span>

<input
type="email"
name="email"
placeholder="email@example.com"
>
</label>

<div>
Copy link

@DorotaLeniecDev DorotaLeniecDev Jun 12, 2023

Choose a reason for hiding this comment

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

Same question here

<label>
<span>Password:</span>

<input
type="password"
name="password"
autocomplete="off"
required
minlength="5"
maxlength="15"
>
</label>
</div>
</fieldset>

<fieldset class="form__group">
<legend>An interesting fact about you!</legend>
Copy link

@Radoslaw-Czerniawski Radoslaw-Czerniawski Jun 5, 2023

Choose a reason for hiding this comment

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

As stated in the previous review all siblings should have an empty line between them apart from some specials situations with repeated tags:

<fieldset class="form__group">
  <legend>An interesting fact about you!</legend>
  
  <div class="form__field">
    <span>Do you love cats?</span>
    
    <label for="love">Yes</label>

    <input
      type="radio"
      id="love"
      name="cats"
      value="yes"
    >

    <label for="hate">No</label>

    <input
      type="radio"
      id="hate"
      name="cats"
      value="no"
    >
  </div>

Copy link

@Radoslaw-Czerniawski Radoslaw-Czerniawski Jun 5, 2023

Choose a reason for hiding this comment

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

Also see that I have wrapped you Do you love cats? text in a span,div element is a container to group up your elements for the purpose their styling/positioning, text alone is not an element and should be nested inside a tag (label wrapping a text and input is one of the exceptions from that rule)


<div class="form__field">
<span>Do you love cats?</span>

<input
type="radio"
id="love"
name="cats"
value="yes"
>

<label for="love">Yes</label>

<input
type="radio"
id="hate"
name="cats"
value="no"
>

<label for="hate">No</label>
</div>

<label class="form__field">
What is your favorite color?

<input
type="color"
name="color"
>
</label>

<label class="form__field">
What time do you go to bed?

<input
type="time"
name="time"
>
</label>

<label class="form__field">
What are your favorite brand of cars?

<select id="brandPicker" name="cars"
Copy link

@Radoslaw-Czerniawski Radoslaw-Czerniawski Jun 5, 2023

Choose a reason for hiding this comment

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

Formatting here is wrong, see how it's done for example on you inputs

Copy link

@DorotaLeniecDev DorotaLeniecDev Jun 12, 2023

Choose a reason for hiding this comment

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

You should fix formatting here as Radek mentioned

Suggested change
<select id="brandPicker" name="cars"
<select
id="brandPicker"
name="cars"
required
multiple
>

required multiple>
<option value="bmw">BMW</option>
Copy link

@Radoslaw-Czerniawski Radoslaw-Czerniawski Jun 5, 2023

Choose a reason for hiding this comment

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

These options shouln't have empty lines between them as they are repetitions of the same tag

<option value="audi">Audi</option>
<option value="lada">Lada</option>
</select>
</label>

<div>
<label>
How do you rate our work?

<input
type="range"
name="range"
min="1"
max="100"
>
</label>
</div>
</fieldset>

<fieldset class="form__group">
<legend>Additional info:</legend>

<label class="form__field">
Comments:

<textarea id="comments" name="comments"></textarea>
</label>

<div>
<span>Would you recommend us?</span>
Copy link

@Radoslaw-Czerniawski Radoslaw-Czerniawski Jun 13, 2023

Choose a reason for hiding this comment

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

Change this to label for the select and get rid of the div


<select name="recommend">
<option value="yes" selected>yes</option>
<option value="no">no</option>
</select>
</div>
</fieldset>

<button type="submit">Submit</button>

Copy link

@DorotaLeniecDev DorotaLeniecDev Jun 12, 2023

Choose a reason for hiding this comment

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

Remove empty line, it is not needed between parent and child elements

</form>
</body>
</html>
11 changes: 10 additions & 1 deletion src/style.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,10 @@
/* styles go here */

.form__group {
margin-bottom: 20px;
}

.form__field {
display: block;
box-sizing: border-box;
margin-bottom: 10px;
}