Skip to content

Develop#789

Open
Patryk-Buczkowski wants to merge 8 commits intomate-academy:masterfrom
Patryk-Buczkowski:develop
Open

Develop#789
Patryk-Buczkowski wants to merge 8 commits intomate-academy:masterfrom
Patryk-Buczkowski:develop

Conversation

@Patryk-Buczkowski
Copy link
Copy Markdown

Copy link
Copy Markdown

@Radoslaw-Czerniawski Radoslaw-Czerniawski left a comment

Choose a reason for hiding this comment

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

Adjust according to my comments and the checklist

Comment thread src/index.html Outdated
method="post"
action="https://mate-academy-form-lesson.herokuapp.com/create-application"
>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

There shouldn't be empty line between parent and first child

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You should have empty lines only between siblings on the same level, not between parent and child, here's an example

<section>
  <div>
    <span>First Child of Div</span>

    <span>Second Child of Div</span>
  <div>
</section>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

An exemption to that rule would be an ul

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

In the rest of your markup doesn't adhere to these rules so adjust accordingly, also take a look at the checklist and also adhere to it's rules

Comment thread src/index.html Outdated
</head>
<body>
<h1>HTML Form</h1>
<script type="text/javascript" src="./main.js"></script>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This script should be at the bottom of the markup to work move it below your form

Comment thread src/index.html

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remove those 2 empty lines

Copy link
Copy Markdown

@DorotaLeniecDev DorotaLeniecDev left a comment

Choose a reason for hiding this comment

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

Few minor changes needed :)

Comment thread src/index.html Outdated
Comment on lines +22 to +32
<label for="surname">Surname:</label>

<input
id="surname"
type="text"
name="surname"
autocomplete="off"
class="input"
>

<br>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remove
tags, you can achive this by grouping form-fiels into block element like this:

Suggested change
<label for="surname">Surname:</label>
<input
id="surname"
type="text"
name="surname"
autocomplete="off"
class="input"
>
<br>
<div class="form-field">
<label for="name">Name:</label>
<input
type="text"
name="name"
id="name"
autocomplete="off"
required
>
</div>

When you wrap label and input into div, it will take 100% width and make newline automatically for next elements. Wrapp all fields like this and remove
:)

Comment thread src/style.css Outdated
Comment on lines +1 to +4
.input {
margin-bottom: 10px;
display: inline-block;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

And you can easly style it like this:

Suggested change
.input {
margin-bottom: 10px;
display: inline-block;
}
.form-field {
margin-bottom: 10px;
}

Comment thread src/index.html Outdated
Comment on lines +159 to +160
<label for="car" class="input">
What are your favorite brand of cars?
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Close label tag here:

Suggested change
<label for="car" class="input">
What are your favorite brand of cars?
<label for="car" class="input">
What are your favorite brand of cars?
</label>

Comment thread src/index.html Outdated
Comment on lines +161 to +165
<select
id="car"
name="carBrand"
multiple
>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fix formating:

Suggested change
<select
id="car"
name="carBrand"
multiple
>
<select
id="car"
name="carBrand"
multiple
>

Comment thread src/index.html Outdated
Comment on lines +189 to +190
<label for="comments" class="input">
Comments:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
<label for="comments" class="input">
Comments:
<label for="comments" class="input">
Comments:
</label>

Comment thread src/index.html Outdated
Comment on lines +199 to +203
<label for="recommend" >Would you recommend us?</label>
<select
id="recommend"
name="recommend"
>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Add empty line between siblings

Suggested change
<label for="recommend" >Would you recommend us?</label>
<select
id="recommend"
name="recommend"
>
<label for="recommend" >Would you recommend us?</label>
<select
id="recommend"
name="recommend"
>

Comment thread src/index.html
Comment on lines +207 to +208
</fieldset>
<button type="submit">Submit</button>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
</fieldset>
<button type="submit">Submit</button>
</fieldset>
<button type="submit">Submit</button>

Copy link
Copy Markdown

@darokrk darokrk left a comment

Choose a reason for hiding this comment

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

Looks nice 🥇 ! I give approve but please correct the indentation in the code according to the example given :)

Comment thread src/index.html
<fieldset class="fieldset">
<legend>Personal information</legend>
<div class="form-field">

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Like Radek mentioned before there should not be an empty line.

"You should have empty lines only between siblings on the same level, not between parent and child, here's an example"

<section>
  <div>
    <span>First Child of Div</span>

    <span>Second Child of Div</span>
  <div>
</section>

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It's done ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants