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 1 commit
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
143 changes: 142 additions & 1 deletion src/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,148 @@
<link rel="stylesheet" href="./style.css">
</head>
<body>
<h1>HTML Form</h1>
<script type="text/javascript" src="./main.js"></script>
<form action="http://localhost:8080/api" method="post"
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.

action according to requirements should be https://mate-academy-form-lesson.herokuapp.com/create-application

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.

Also formatting is wrong, adjust according to the checklist p. 4

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>
<div class="form-field">
Surname:
<input
type="text"
autocomplete="off"
>
</div>
<div 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"
autocomplete="off"
>
</div>
<div class="form-field">
How old are You?
<input
type="number"
name="number"
min="1" max="100"
value="12"
>
</div>
<div class="form-field">
Full date of birth:
<input
type="date"
name="date"
>
</div>
<div>
I accept the term of the agreement
<input
type="checkbox"
name="agreement"
required
>
</div>
</fieldset>
<fieldset class="form__group">
<legend>Registration</legend>
<div class="form-field">
E-mail:
<input
type="email"
name="email"
placeholder="email@example.com"
>
</div>
<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

Password:
<input
type="password"
name="password"
autocomplete="off"
required minlength="5"
maxlength="15"
>
</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">
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">
What is your favorite color?
<input
type="color"
name="color"
>
</div>
<div class="form-field">
What time do you go to bed?
<input
type="time"
name="time"
>
</div>
<div class="form-field">
<label for="brand-picker">What are your favorite brand of cars?</label>
<select id="brand-picker" name="cars"
required multiple>
<option value="BMW">BMW</option>
<option value="Audi">Audi</option>
<option value="Lada">Lada</option>
<option value=" "
disabled>
</option>
</select>
</div>
<div>
How do you rate our work?
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.

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">
Comments:
<textarea id="comments" name="comments"></textarea>
</div>
<div>
Would you recommend us?
<select name="recommend">
<option value="recommend"
disabled>
</option>
<option value="yes"
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.

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

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