Skip to content
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.

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

Solution-Form #797

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Solution-Form #797

wants to merge 3 commits into from

Conversation

Copy link

@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.

Good start! Improve the code readability according to my comments and check the checklist one more time.

src/index.html Outdated
Comment on lines 16 to 18
<form
action="https://mate-academy-form-lesson.herokuapp.com/create-application"
method="post">

Choose a reason for hiding this comment

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

Suggested change
<form
action="https://mate-academy-form-lesson.herokuapp.com/create-application"
method="post">
<form
action="https://mate-academy-form-lesson.herokuapp.com/create-application"
method="post"
>

src/index.html Outdated
Comment on lines 22 to 23
</legend>
<label class="field">

Choose a reason for hiding this comment

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

Don't forget to add empty lines between multiline sibling blocks of HTML

Suggested change
</legend>
<label class="field">
</legend>
<label class="field">

src/index.html Outdated
name="answer"
value="Yes"
>
<label for="yes">

Choose a reason for hiding this comment

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

Fix the labels here. It doesn't work correctly for now

Screen.Recording.2023-06-01.at.14.00.14.mov

Copy link
Author

Choose a reason for hiding this comment

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

Hi @Anastasiia-Svintsova about this part I do not understand what I have to do? It's giving only one choice yes or no, do I have to change both to be selected? Please explain to me and thanks for your suggestions :)

src/index.html Outdated
Comment on lines 176 to 178
<button class="frame"
type="submit"
name="submit">

Choose a reason for hiding this comment

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

Suggested change
<button class="frame"
type="submit"
name="submit">
<button
class="frame"
type="submit"
name="submit"
>

Copy link

@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.

Hey! Pay attention to my comments and fix the code style, please

src/index.html Outdated
@@ -11,7 +11,215 @@
<link rel="stylesheet" href="./style.css">
</head>
<body>
<h1>HTML Form</h1>
<!-- <h1>HTML Form</h1> -->

Choose a reason for hiding this comment

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

Remove comments

src/index.html Outdated
Comment on lines 19 to 21
>

<fieldset class="frame">

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 components

Suggested change
>
<fieldset class="frame">
>
<fieldset class="frame">

src/index.html Outdated
Comment on lines 33 to 35
>

</label>

Choose a reason for hiding this comment

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

Fix it everywhere

Suggested change
>
</label>
>
</label>

src/index.html Outdated
Comment on lines 114 to 115
<label class="field">
Do you love cats?

Choose a reason for hiding this comment

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

These radio buttons still don't work correctly. Test it more carefully, please. Ask questions in the chat if you need to

Screen.Recording.2023-06-02.at.13.55.45.mov

src/index.html Outdated
Comment on lines 159 to 160
<select name="cars"
id="cars" multiple>

Choose a reason for hiding this comment

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

Check the code style

Suggested change
<select name="cars"
id="cars" multiple>
<select
name="cars"
id="cars"
multiple
>

Copy link

@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.

Great work! I like it!

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.

3 participants