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 2 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
188 changes: 187 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,194 @@
<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
Copy Markdown

@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>

<div class="form__field">
Copy link
Copy Markdown

@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.

I assume you are using this div to wrap all elements in a block elements so it takes the whole available horizontal space, there is no need for that if you already wrap your label and input in label element. Remove this div

Copy link
Copy Markdown

@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 there should be an empty line between surname and input

Copy link
Copy Markdown

@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.

It should look like this:

<label class="form__field">
  Surname:

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

<label class="form__field">
  Name:

  <input
    type="text"
    name="name"
    autocomplete="off"
  >

Adjust rest of you elements and remove unnecessary divs

Copy link
Copy Markdown

@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 consider name a required attribute for the input, after form is submitted values from inputs that don't have names are omitted from the payload, structure of the payload is such that the input's name forms a property with corresponding value from the input on the submit payload

image

You may see this result in network tab after submitting your form, input's that didn't have names are not submitted

Copy link
Copy Markdown
Author

@DominikaKarmel DominikaKarmel Jun 6, 2023

Choose a reason for hiding this comment

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

It should look like this:

<label class="form__field">
  Surname:

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

<label class="form__field">
  Name:

  <input
    type="text"
    name="name"
    autocomplete="off"
  >

Adjust rest of you elements and remove unnecessary divs

Hello Radoslaw, if i remove this divs then i'll have elements inline. How handle it?

Copy link
Copy Markdown

@darokrk darokrk Jun 9, 2023

Choose a reason for hiding this comment

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

Just add to your .form__field css class value display: block it should work. Then you can remove the div elements.

<label>
Surname:
<input
type="text"
Copy link
Copy Markdown

@darokrk darokrk Jun 9, 2023

Choose a reason for hiding this comment

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

Add a 'name' attribute to the input field for 'Surname'. Example: name="surname"

autocomplete="off"
>
</label>
</div>

<div class="form__field">
<label>
Name:
<input
Copy link
Copy Markdown

@darokrk darokrk Jun 9, 2023

Choose a reason for hiding this comment

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

Add a 'name' attribute to the input field for 'Name'. Example: name="name"

type="text"
autocomplete="off"
>
</label>
</div>

<div class="form__field">
<label>
How old are You?
<input
type="number"
Copy link
Copy Markdown

@darokrk darokrk Jun 9, 2023

Choose a reason for hiding this comment

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

Change the 'name' attribute value to a more descriptive name for the age input field. Example: name="age"

name="number"
min="1" max="100"
value="12"
>
</label>
</div>

<div class="form__field">
<label>
Full date of birth:
<input
type="date"
Copy link
Copy Markdown

@darokrk darokrk Jun 9, 2023

Choose a reason for hiding this comment

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

Add a 'name' attribute to the input field for 'Full date of birth'. Example: name="birthdate"

name="date"
>
</label>
</div>

<div>
<label>
I accept the term of the agreement
<input
type="checkbox"
name="agreement"
required
>
</label>
</div>
</fieldset>

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

<div class="form__field">
<label>
E-mail:
<input
type="email"
name="email"
placeholder="email@example.com"
>
</label>
</div>

<div>
Copy link
Copy Markdown

@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>
Password:
<input
type="password"
name="password"
autocomplete="off"
required minlength="5"
Copy link
Copy Markdown

@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.

All attributes here should be on separate lines

maxlength="15"
>
</label>
</div>
</fieldset>

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

@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
Copy Markdown

@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">
Do you love cats?
<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>

<div class="form__field">
<label>
What is your favorite color?
<input
type="color"
name="color"
>
</label>
</div>

<div class="form__field">
<label>
What time do you go to bed?
<input
type="time"
name="time"
>
</label>
</div>

<div class="form__field">
<label for="brandPicker">What are your favorite brand of cars?</label>

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

@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
Copy Markdown

@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
Copy Markdown

@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>

<option value=" " disabled> </option>
Copy link
Copy Markdown

@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.

There is no need for this empty option, remove it

</select>
</div>

<div>
How do you rate our work?
Copy link
Copy Markdown

@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.

This should be a label for the below input

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

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

<div class="form__field">
<label>
Comments:
<textarea id="comments" name="comments"></textarea>
</label>
</div>

<div>
Would you recommend us?
<select name="recommend">
<option value="recommend"
disabled>
</option>

<option value="yes"
Copy link
Copy Markdown

@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.

Wrong formatting, adjust similarly to how attributes are spaced on you inputs, also you don't need empty lines between options as these are same tags

selected>yes
</option>

<option value="no">no</option>
</select>
</div>
</fieldset>

<div class="form-submit">
Copy link
Copy Markdown

@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.

There is no need for this button to be wrapped in div

<button type="submit">Submit</button>
</div>
</form>
</body>
</html>
10 changes: 9 additions & 1 deletion src/style.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,9 @@
/* styles go here */

.form__group {
margin-bottom: 20px;
}

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