Skip to content

Added task solution#782

Open
kyrylomanko wants to merge 3 commits intomate-academy:masterfrom
kyrylomanko:develop
Open

Added task solution#782
kyrylomanko wants to merge 3 commits intomate-academy:masterfrom
kyrylomanko:develop

Conversation

@kyrylomanko
Copy link
Copy Markdown

@kyrylomanko kyrylomanko commented Apr 26, 2023

Copy link
Copy Markdown

@Anastasiia-Svintsova Anastasiia-Svintsova left a comment

Choose a reason for hiding this comment

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

  • The user should be able to choose only one option (yes or no). Fix these radio buttons, pelase
Screen.Recording.2023-04-27.at.12.36.45.mov

Comment thread readme.md
Comment on lines +3 to +4
- [DEMO LINK](https://kyrylomanko.github.io/layout_html-form/)
- [TEST REPORT LINK](https://kyrylomanko.github.io/layout_html-form/report/html_report/)
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 add these links to the PR description

Comment thread src/index.html Outdated
id="surname"
name="surname"
autocomplete="off"
><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.

Why do you need these br tags everywhere? Remove them

Suggested change
><br>
>

Comment thread src/index.html Outdated
<label for="surname">Surname:</label>
<input
cletype="text"
class="distance"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Use more meaningful class names. For example:

Suggested change
class="distance"
class="form-field"

Comment thread src/index.html Outdated
Comment on lines +59 to +61
>

</fieldset>
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>
>
</fieldset>

Comment thread src/index.html Outdated
Comment on lines +79 to +81
>

</fieldset>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Don't add spaces between parent and child elements

Suggested change
>
</fieldset>
>
</fieldset>

Comment thread src/style.css Outdated
margin-bottom: 10px;
}

fieldset {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Don't style tags, use class selector instead

Copy link
Copy Markdown

@witflash witflash 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 but just one thing:
image
Could you check if you use these attributes at least one time?

@kyrylomanko kyrylomanko requested a review from witflash April 27, 2023 15:07
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