Skip to content

add task solution #801

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
aRabiej wants to merge 6 commits into
base: master
Choose a base branch
from
Open

add task solution #801

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
6e63de7
add task solution
aRabiej Jun 15, 2023
444f258
links should be visible
aRabiej Jun 18, 2023
e3ccaf8
'added corrections'
aRabiej Jun 28, 2023
8899675
made more adjustments, trying to update readme, so it contains demo link
aRabiej Jun 29, 2023
caf1045
formatting fixes, added some forgotten IDs
aRabiej Jun 30, 2023
9b7bd56
added the minor changes!
aRabiej Jun 30, 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
1 change: 0 additions & 1 deletion readme.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,4 +61,3 @@ Create HTML page with form. On form submit send form data to `https://mate-acade
placeholder="..."
></textarea>
```

225 changes: 161 additions & 64 deletions src/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,80 +13,177 @@
method="get"
Copy link
Copy Markdown

@Radoslaw-Czerniawski Radoslaw-Czerniawski Jun 30, 2023

Choose a reason for hiding this comment

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

Method here should be POST rather than GET -> https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods

name="form"
>
<fieldset>
<fieldset class="box_style">
<legend>Personal information</legend>
Surname:
<input type="text" name="surname"
value=" " minlength="3"
maxlength="100" autocomplete="off"
><br>
Name:
<input type="text" name="name"
value=" " minlength="3"
maxlength="100" autocomplete="off"
><br>
How old are You?
<input type="number" name="age"
value="12"
min="1" max="100"
><br>
Full date of birth:
<input type="date" name="date"
value=" "
><br>
I accept the term of the agreement
<input type="checkbox"
value="check" name="check"

<div class="field_wrapper">
<label for="surname">Surname: </label>

<input
type="text"
name="surname"
id="surname"
value=" "
Copy link
Copy Markdown

@darokrk darokrk Jun 29, 2023

Choose a reason for hiding this comment

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

Empty space as a default value in input is not a good practice. Please remove this attribute value=" " it's redundant here

minlength="3"
maxlength="100"
autocomplete="off"
>
</div>

<div class="field_wrapper">
<label for="name">Name: </label>

<input
type="text"
name="name"
id="name"
value=" "
Copy link
Copy Markdown

@darokrk darokrk Jun 29, 2023

Choose a reason for hiding this comment

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

Empty space as a default value in input is not a good practice. Please remove this attribute value=" " it's redundant here

minlength="3"
maxlength="100"
autocomplete="off"
>
</div>

<div class="field_wrapper">
<label for="age">How old are You? </label>

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

<div class="field_wrapper">
<label for="date">Full date of birth: </label>

<input
type="date"
name="date"
id="date"
value=" "
Copy link
Copy Markdown

@darokrk darokrk Jun 29, 2023

Choose a reason for hiding this comment

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

Empty space as a default value in input is not a good practice. Please remove this attribute value=" " it's redundant here

>
</div>

<label for="check">I accept the term of the agreement </label>

<input
type="checkbox"
value="check"
name="check"
id="check"
>
</fieldset>
<fieldset>

<fieldset class="box_style">
<legend>Registration</legend>
E-mail:
<input type="email" name="email"
value=" " placeholder="email@example.com"
multiple
><br>
Password:
<input name="user-password" type="password"
required autocomplete="off"

<div class="field_wrapper">
<label for="email">E-mail: </label>

<input
type="email"
name="email"
id="email"
value=" "
Copy link
Copy Markdown

@darokrk darokrk Jun 29, 2023

Choose a reason for hiding this comment

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

Empty space as a default value in input is not a good practice. Please remove this attribute value=" " it's redundant here

placeholder="email@example.com"
multiple
Copy link
Copy Markdown

@DorotaLeniecDev DorotaLeniecDev Jun 30, 2023

Choose a reason for hiding this comment

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

You can use multiple attribute with input type email, but I think that in this case more suitable would be to add multiple attribute to select tag, as this is registration form so we are allowing one email value :)

>
</div>

<label for="user-password">Password: </label>

<input
name="user-password"
Copy link
Copy Markdown

@DorotaLeniecDev DorotaLeniecDev Jun 30, 2023

Choose a reason for hiding this comment

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

Value of name attribute should be in camelCase like userPassword

id="user-password"
type="password"
required autocomplete="off"
Copy link
Copy Markdown

@DorotaLeniecDev DorotaLeniecDev Jun 30, 2023

Choose a reason for hiding this comment

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

Fix formatting:

Suggested change
required autocomplete="off"
required
autocomplete="off"

>
</fieldset>
<fieldset>

<fieldset class="box_style">
<legend>An interesting fact about you!</legend>
Do you love cats?
<input type="radio" id="yes"
name="cat" value="Y"

<div class="field_wrapper">
<label for="cat">Do you love cats? </label>

<input
type="radio"
id="yes"
name="cat"
value="Y"
Copy link
Copy Markdown

@Radoslaw-Czerniawski Radoslaw-Czerniawski Jun 30, 2023

Choose a reason for hiding this comment

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

This value is very vague, it would be better here to reflect what is checked at the input -> Yes and below No

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

<input
type="radio"
id="no"
name="cat"
value="N"
>
<label for="no">No</label>
</div>

<div class="field_wrapper">
<label for="color">What is your favorite color? </label>

<input
type="color"
name="color"
value=" "
Copy link
Copy Markdown

@darokrk darokrk Jun 29, 2023

Choose a reason for hiding this comment

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

Empty space as a default value in input is not a good practice. Please remove this attribute value=" " it's redundant here

>
Comment on lines +129 to +133
Copy link
Copy Markdown

@DorotaLeniecDev DorotaLeniecDev Jun 30, 2023

Choose a reason for hiding this comment

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

Label for this field is not clickable(when clicking label it should focuse on the input).
Add id which coresponds with for attribute in label

Suggested change
<input
type="color"
name="color"
>
<input
type="color"
name="color"
id="color"
>

</div>

<div class="field_wrapper">
<label for="time">What time do you go to bed? </label>

<input
type="time"
name="time"
id="time"
step="2"
value=" "
Copy link
Copy Markdown

@darokrk darokrk Jun 29, 2023

Choose a reason for hiding this comment

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

Empty space as a default value in input is not a good practice. Please remove this attribute value=" " it's redundant here

>
</div>

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

<textarea
name="cars"
id="cars"
rows="4"
cols="2"
>BMW Audi Lada
</textarea>
Copy link
Copy Markdown

@DorotaLeniecDev DorotaLeniecDev Jun 30, 2023

Choose a reason for hiding this comment

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

Here more suitable would be to use select tag instead of textarea
Here is an example:

<select name="cars" id="cars" multiple>
  <option value="volvo">Volvo</option>
  <option value="saab">Saab</option>
  <option value="opel">Opel</option>
  <option value="audi">Audi</option>
</select>

This will also fix your test results :)

</div>

<label for="range">How do you rate our work? </label>

<input
type="range"
name="range"
value="0"
>
Comment on lines +160 to +168
Copy link
Copy Markdown

@DorotaLeniecDev DorotaLeniecDev Jun 30, 2023

Choose a reason for hiding this comment

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

Add proper id so the label is clickable

<label for="yes">Yes</label>
<input type="radio" id="no"
name="cat" value="N"
>
<label for="no">No</label><br>
What is your favourite color?
<input type="color" name="color"
value=" "
><br>
What time do you go to bed?
<input type="time" name="time"
value=" "
><br>
What are your favorite brand of cars?
<textarea name="cars" rows="4"
cols="3"
>BMW Audi Lada
</textarea><br>
How do you rate our work?
<input type="range" name="range"
value=" "
><br>
</fieldset>
<fieldset>

<fieldset class="box_style">
<legend>Additional info:</legend>
Comments:
<textarea name="comment" rows="2"
cols="20"
>
</textarea><br>

<div class="field_wrapper">
<label for="comment">Comments: </label>

<textarea
Copy link
Copy Markdown

@DorotaLeniecDev DorotaLeniecDev Jun 30, 2023

Choose a reason for hiding this comment

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

Add id

name="comment"
rows="2"
cols="20"
>
</textarea>
Copy link
Copy Markdown

@DorotaLeniecDev DorotaLeniecDev Jun 30, 2023

Choose a reason for hiding this comment

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

<textarea> is a tricky tag to format:
It requires you to put a closing tag on the same line as the > of the opening tag, not to have default content, and be able to see the placeholder.
At the same time, if it has several attributes, you need to put each of them on its own line.
So, use the following formatting where > is moved 1 position left from the normal alignment:

  <textarea
    class="..."
    placeholder="..."
 ></textarea>

</div>

<label for="reco">Would you recommend us?</label>
<select id="reco" name="reco">
<option value="yes" selected>yes</option>
Expand Down
15 changes: 4 additions & 11 deletions src/style.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,8 @@
fieldset {
margin-bottom: 20px;
}

input {
.field_wrapper {
Copy link
Copy Markdown

@darokrk darokrk Jun 29, 2023

Choose a reason for hiding this comment

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

following the BEM naming convention this name should have double underscore mark field__wrapper

margin-bottom: 10px;
}

textarea {
margin-bottom: 10px;
}

label {
margin-top: 10px;
.box_style {
Copy link
Copy Markdown

@darokrk darokrk Jun 29, 2023

Choose a reason for hiding this comment

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

following the BEM naming convention this name should have double underscore mark box__style

display: block;
margin-bottom: 20px;
}