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 #665

Closed
wants to merge 9 commits into from
Closed

solution #665

wants to merge 9 commits into from

Conversation

Taras0506
Copy link

@Taras0506 Taras0506 commented Apr 10, 2023

Copy link

@anstsot anstsot left a comment

Choose a reason for hiding this comment

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

Please check the links, add the report and fix your code according to checklist. Please add all needed fields as shown in the task.

src/index.html Outdated Show resolved Hide resolved
@Taras0506 Taras0506 requested a review from anstsot April 18, 2023 18:30
Copy link

@VitaliyBondarenko1982 VitaliyBondarenko1982 left a comment

Choose a reason for hiding this comment

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

check demo links.
they does not work

Comment on lines +22 to +24
<label class="form-field">
Surname:
<input

Choose a reason for hiding this comment

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

Suggested change
<label class="form-field">
Surname:
<input
<label class="form-field">
Surname:
<input

Comment on lines +38 to +40
<label class="form-field">
How old are You?
<input

Choose a reason for hiding this comment

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

Suggested change
<label class="form-field">
How old are You?
<input
<label class="form-field">
How old are You?
<input

check and fix other places of wrong indentation

Choose a reason for hiding this comment

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

still not fixed

src/index.html Outdated
Comment on lines 98 to 103
<label>
<input
type="radio"
name="sex"
value="m"
>

Choose a reason for hiding this comment

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

Suggested change
<label>
<input
type="radio"
name="sex"
value="m"
>
<label>
<input
type="radio"
name="sex"
value="m"
>

src/style.css Outdated
Comment on lines 7 to 9
.field::placeholder {
color: green;
}

Choose a reason for hiding this comment

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

Suggested change
.field::placeholder {
color: green;
}

redundant

@Taras0506
Copy link
Author

check demo links. they does not work

Hi, please write how its correct?

@Taras0506
Copy link
Author

Taras0506 commented Apr 20, 2023

check demo links. they does not work

Hi, please write how its correct.
I don't know why ,but my line with that links is empty.
Could you copy it for me here please if its possible of course.

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.

if you have some problems with deploy, feel free to ask about it in the slack chat!

src/index.html Outdated
<option selected>BMW</option>
<option selected>Audi</option>
<option>Lada</option>
<option></option>

Choose a reason for hiding this comment

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

There is an empty <option> element in the select field for favorite brand of cars. You can remove it or add a value to it, such as 'Other'. 🚗

src/index.html Outdated
Comment on lines 51 to 56
<input
name="feedback"
placeholder="mm/dd/yyyy"
rows="1"
cols="20"
>

Choose a reason for hiding this comment

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

It should be an input with a date type. And why do you need rows and cols attributes here?

Copy link
Author

Choose a reason for hiding this comment

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

Its doesnt logic, you right!
Add type="date"

src/index.html Outdated
<script type="text/javascript" src="./main.js"></script>
<form
action="http://localhost:3000/api"

Choose a reason for hiding this comment

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

Fix the action here according to the requirements
image

Copy link
Author

Choose a reason for hiding this comment

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

I can`t open this link. I must insert instead this link in line action ?

Comment on lines +25 to +26
<label class="form-field">
Surname:

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
<label class="form-field">
Surname:
<label class="form-field">
Surname:

Choose a reason for hiding this comment

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

still not fixed

src/index.html Outdated
Comment on lines 29 to 30
rows="1"
cols="20"

Choose a reason for hiding this comment

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

Remove these rows and sols everywhere, please

src/index.html Outdated
Comment on lines 32 to 33
</label>
<label class="form-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
</label>
<label class="form-field">
</label>
<label class="form-field">

readme.md Show resolved Hide resolved
Copy link

@OleziO OleziO left a comment

Choose a reason for hiding this comment

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

Fix the demo link.

@Taras0506 Taras0506 requested a review from OleziO April 23, 2023 14:57
@Taras0506
Copy link
Author

Explain please how to fix it? I understand that its simply task , but it doesnt work... I check every line before, what`s the reasons

Copy link

@VitaliyBondarenko1982 VitaliyBondarenko1982 left a comment

Choose a reason for hiding this comment

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

I fixed demo link in PR description (was missed / in url)
Test report link still not works.
Try to run npm test locally and deploy again.
If test report link stiil will be wrong, make screenshot with tests and show it in PR

src/style.css Outdated
Comment on lines 7 to 17
.field:invalid {
border: 1px solid red;
}

.field:focus {
background-color: azure;
}

.field:checked ~ .form__label {
color: grey;
}

Choose a reason for hiding this comment

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

Suggested change
.field:invalid {
border: 1px solid red;
}
.field:focus {
background-color: azure;
}
.field:checked ~ .form__label {
color: grey;
}

looks like redundant styles for this task

@Taras0506
Copy link
Author

Снимок экрана (199)
how to rigth correct this error ?

@Taras0506
Copy link
Author

Add second screenshot
Снимок экрана (200)

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.

if you have any problems with the deploy, ask it in the chat, we will be able to help you as soon as possible

Comment on lines +38 to +40
<label class="form-field">
How old are You?
<input

Choose a reason for hiding this comment

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

still not fixed

@Taras0506 Taras0506 mentioned this pull request Apr 24, 2023
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.

report link still does not work

<label class="form-field">
Surname:
<input
name="feedback"

Choose a reason for hiding this comment

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

The 'name' attribute should be unique for each input field. Please change the 'name' attribute for the Surname input field to something like 'surname'.

Choose a reason for hiding this comment

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

image

fix it everywhere

@Taras0506
Copy link
Author

report link still does not work

Hi! What does look like this link ? Why its doesnt work ?

<label class="form-field">
Name:
<input
surname="feedback"

Choose a reason for hiding this comment

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

The attribute for the Name input should be 'name' instead of 'surname'

type="range"
id="rate"
min="0"
>

Choose a reason for hiding this comment

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

The 'name' attribute is missing for the How do you rate your work range input

</label>
<div class="form_field">
What are your favorite brand of cars?
<select name="text">

Choose a reason for hiding this comment

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

The 'name' attribute for the favorite brand of cars select input should be 'brand' or a more descriptive name instead of 'text'

Comment on lines +25 to +26
<label class="form-field">
Surname:

Choose a reason for hiding this comment

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

still not fixed

@etojeDenys
Copy link

report link still does not work

Hi! What does look like this link ? Why its doesnt work ?

if you have any problems with the deploy, ask it in the chat, we will be able to help you as soon as possible

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.

reports link still does not work, and all prev comments are not resolved

@Taras0506 Taras0506 requested a review from etojeDenys May 1, 2023 10:32
Copy link

@VitaliyBondarenko1982 VitaliyBondarenko1982 left a comment

Choose a reason for hiding this comment

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

About wrong test report link.
Ran your project locally.
A lot of tests failed.
Screenshot from 2023-05-01 13-54-16

First of all run tests locally (npm test), compare reference and test and fix them.
For success deploy tests link should be backstop folder in project.
For fix tests check class names form-field (in some places it is form_field)
add class for fieldset and such styles

   margin-bottom: 20px;
   padding-bottome: 0

and for form-field

  .form-field {
    display: block;
    margin-bottom: 10px;
  }

if test demo link will be wrong at least fix tests locally and add screenshot as above but with correct tests.
and if you have any questions about how to fix tests - feel free to ask in fe_chat

Comment on lines +123 to +125
<label class="form-field">
What time do you go to bed?
<input

Choose a reason for hiding this comment

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

Suggested change
<label class="form-field">
What time do you go to bed?
<input
<label class="form-field">
What time do you go to bed?
<input

still not fixed in all similar places

<option>Other</option>
</select>
</div>
<label for="start">

Choose a reason for hiding this comment

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

Suggested change
<label for="start">
<label for="rate">

for and id in input should be the same

value="--:--:--"
>
</label>
<div class="form_field">

Choose a reason for hiding this comment

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

Suggested change
<div class="form_field">
<div class="form-field">

@Taras0506 Taras0506 closed this by deleting the head repository Jun 25, 2023
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.

6 participants