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

add task solution #3866

Closed
wants to merge 4 commits into from
Closed

add task solution #3866

wants to merge 4 commits into from

Conversation

danil-maksimenko
Copy link

  • DEMO LINK

  • TEST REPORT LINK

  • Icon implemented using background-image CSS property

  • Inputs are written inside of 'form' tag with correctly passed attributes

  • All Typical Mistakes from BEM lesson theory are checked.

  • Code follows all the Code Style Rules ❗️

Copy link

@Esceype Esceype left a comment

Choose a reason for hiding this comment

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

GJ!
Read the following comments

src/style.css Outdated
color: var(--main-text-colot);
}

input {
Copy link

Choose a reason for hiding this comment

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

Set it in search-form__input

Copy link
Author

Choose a reason for hiding this comment

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

I need to set color: var(--main-text-colot) in search-form__input ?

Choose a reason for hiding this comment

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

do not style by tags. move all styles for input to .search-form__input selector.
box-sizing better to set in * selector

src/style.css Outdated
}

html {
font-family: Avenir, sans-serif;
Copy link

Choose a reason for hiding this comment

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

Add fallback font

src/index.html Outdated
data-qa="big"
>
<label class="search-form__label">
<span class="search-form__image search-form__image--big"></span>
Copy link

Choose a reason for hiding this comment

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

Just set all styles in this selectors to search-form__input, you don`t need this additional span here and in the small form

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.

Please check comments.
And demo_links - they are not works.
Try to deploy again.
If will have some problems with deploy - feel free to ask in chat.

src/index.html Outdated
<span class="search-form__image search-form__image--big"></span>
<input
type="text"
class="search-form__input search-form__input--big"

Choose a reason for hiding this comment

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

Suggested change
class="search-form__input search-form__input--big"
class="search-form__input"

you don't need --big modifier.
set all styles in .search-form__input selector and change necessary for --small modifier.
same for other elements

src/style.css Outdated
color: var(--main-text-colot);
}

input {

Choose a reason for hiding this comment

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

do not style by tags. move all styles for input to .search-form__input selector.
box-sizing better to set in * selector

src/style.css Outdated
Comment on lines 102 to 103
padding-left: 62px;
font-size: 16px;

Choose a reason for hiding this comment

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

move this styles in .search-form__input selector

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.

Well done.


.search-form__input {
outline: none;
box-sizing: border-box;

Choose a reason for hiding this comment

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

box-sizing better to set for * selector.

* {
  box-sizing: border-box
}

@danil-maksimenko danil-maksimenko closed this by deleting the head repository Aug 21, 2024
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