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

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

Conversation

pogorielova
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

@mateacademy-ai-mentor mateacademy-ai-mentor 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 so far! 😊 However, there are a few areas that need some adjustment. Please correct the background-color property in the :focus and :active states of the .form__input class. Also, consider adding a submit button and a more descriptive title for your page. Keep it up! 👍

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨

Comment on lines +20 to +37
<form
action="submit"
class="form"
>
<input
type="text"
data-qa="big"
placeholder="Try “Los Angeles“"
class="form__input form__input_big"
/>

<input
type="text"
data-qa="hover"
placeholder="Try “Los Angeles“"
/>
<input
type="text"
data-qa="small"
placeholder="Try “Los Angeles“"
class="form__input form__input_small"
/>
</form>

Choose a reason for hiding this comment

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

Your form lacks a submit button. Even though it's not explicitly stated in the task, it's a good practice to include one in your form. Without it, users won't be able to submit the form.

Comment on lines +24 to +29
<input
type="text"
data-qa="big"
placeholder="Try “Los Angeles“"
class="form__input form__input_big"
/>

Choose a reason for hiding this comment

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

The data-qa attribute is used for testing purposes. If you're not using it for testing, it's better to remove it.

Comment on lines +31 to +36
<input
type="text"
data-qa="small"
placeholder="Try “Los Angeles“"
class="form__input form__input_small"
/>

Choose a reason for hiding this comment

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

The value of the placeholder attribute is the same for both inputs. It's better to use different placeholders for different inputs to avoid confusion for the users.

src/style.css Outdated

.form__input:focus {
outline: none;
background-color: linear-gradient(180deg, #fff 0%, #f6f6f7 100%);

Choose a reason for hiding this comment

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

The background-color property doesn't accept linear-gradient as a value. You might have wanted to use the background-image property instead.

src/style.css Outdated
}

.form__input:active {
background-color: linear-gradient(180deg, #fff 0%, #f6f6f7 100%);

Choose a reason for hiding this comment

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

Same issue here, the background-color property doesn't accept linear-gradient as a value. You should use the background-image property.

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor 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 on the HTML and CSS structure! The code is clean and well-structured, which makes it easy to understand and maintain. However, there are some areas that need improvement. Particularly, the title of your HTML document should be more descriptive. Also, remember to include the alt attribute in your input tags for better accessibility. Lastly, the action attribute of the form tag should contain a valid URL. Keep these points in mind for your future projects. Keep up the good work and continue learning! 👍

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨

Comment on lines +20 to +23
<form
action="submit"
class="form"
>

Choose a reason for hiding this comment

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

The action attribute of the form tag is supposed to contain the URL where the form data is sent to. In your case, submit is not a valid URL. If you don't have a specific URL to send the form data to, you can just leave this attribute out.

type="text"
data-qa="big"
placeholder="Try “Los Angeles“"
class="form__input form__input_big"

Choose a reason for hiding this comment

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

The alt attribute is missing in your input tags. Although it's not mandatory, it's a good practice to include it as it improves accessibility. The alt attribute provides alternative information for an image if a user for some reason cannot view it (because of slow connection, an error in the src attribute, or if the user uses a screen reader).

type="text"
data-qa="small"
placeholder="Try “Los Angeles“"
class="form__input form__input_small"

Choose a reason for hiding this comment

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

The alt attribute is missing in your input tags. It's a good practice to include it as it improves accessibility. The alt attribute provides alternative information for an image if a user for some reason cannot view 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.

2 participants