Skip to content

Conversation

@kbekher
Copy link

@kbekher kbekher commented Apr 25, 2023

No description provided.

Copy link

@mykhalenych mykhalenych left a comment

Choose a reason for hiding this comment

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

your links aren't working, also don't forget add links to description

@kbekher
Copy link
Author

kbekher commented Apr 25, 2023

DEMO
TEST REPORT LINK

not sure if my demo link is correct...

@kbekher kbekher requested a review from mykhalenych April 25, 2023 15:37
src/index.html Outdated
Comment on lines 18 to 25
<label for="surname">Surname:</label>
<input
type="text"
name="surname"
id="surname"
autocomplete="off"
class="form-input-gap"
>

Choose a reason for hiding this comment

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

  1. i'd recommend to wrap input with label to avoid using id/for
  2. apply class to your wrapper (label) instead of input to make outer indent
Suggested change
<label for="surname">Surname:</label>
<input
type="text"
name="surname"
id="surname"
autocomplete="off"
class="form-input-gap"
>
<label>
Surname:
<input
type="text"
name="surname"
autocomplete="off"
class="form-input-gap"
>
</label>

src/index.html Outdated
autocomplete="off"
class="form-input-gap"
>
<br>

Choose a reason for hiding this comment

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

it's better avoid using <br />. Use css

src/index.html Outdated
Comment on lines 96 to 99
<label>
Do you love cats?
<span>
<input

Choose a reason for hiding this comment

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

each input/select should be wrapped with a label

Suggested change
<label>
Do you love cats?
<span>
<input
<div>
Do you love cats?
<label>
<input

src/index.html Outdated
>
<br>

<div class="form-input-gap">

Choose a reason for hiding this comment

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

redundant wrapper. apply styles to your labels

Suggested change
<div class="form-input-gap">

src/style.css Outdated
Comment on lines 1 to 7
.form-field {
margin-bottom: 20px;
}

.form-input-gap {
margin-bottom: 10px;
}

Choose a reason for hiding this comment

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

You may only apply classes only to your filedset and form field (label or div)
Example:

.fieldset:not(:last-child) {
  margin-bottom: 20px
}

.field:not(:last-child) {
  margin-bottom: 10px
}

@kbekher kbekher requested a review from Viktor-Kost April 25, 2023 19:19
Copy link

@etojeDenys etojeDenys left a comment

Choose a reason for hiding this comment

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

almost done

Comment on lines 88 to 103
<label>
<input
type="radio"
name="yes"
value="yes"
>
Yes
</label>
<label>
<input
type="radio"
name="no"
value="no"
>
No
</label>

Choose a reason for hiding this comment

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

should be the same name attribute to avoid this
image

@kbekher kbekher requested review from etojeDenys April 26, 2023 07:27
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.

5 participants