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

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

Conversation

OlhaLishenko
Copy link

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 job on your Pull Request! There are some minor issues such as formatting and best practices that I pointed out, like using gradients correctly and specifying fallback fonts, but they don't significantly impact the overall functionality of your code. Keep up the good work, and consider these tips for future improvements to make your code even better! 😊👍

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

Comment on lines +26 to +31
<input
class="input-bar input-bar--big"
id="input-icon-big"
type="text"
data-qa="keypress"
placeholder="Try “Los Angeles”"

Choose a reason for hiding this comment

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

Keep your attributes correctly formatted. Since the element has more than two attributes, each attribute should start on a new line with 2-space indentation. The closing bracket should align with the opening tag line.

Comment on lines +39 to +44
<input
class="input-bar input-bar--small"
id="input-icon-small"
type="text"
data-qa="hover small"
placeholder="Try “Los Angeles”"

Choose a reason for hiding this comment

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

The attributes of the element should follow the same formatting rules as mentioned above. Each attribute should be on a new line with proper indentation.

src/style.css Outdated
Comment on lines 15 to 17
* {
box-sizing: border-box;
}

Choose a reason for hiding this comment

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

Avoid using the universal selector '*' for setting box-sizing as it impacts performance. Instead, set box-sizing on specific elements that require it.

src/style.css Outdated
Comment on lines 19 to 21
html {
font-family: Avenir, sans-serif;
}

Choose a reason for hiding this comment

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

Remember to use fallback fonts. In case the main font-family doesn't load, it's good to have alternative fonts specified.

src/style.css Outdated
Comment on lines 96 to 97
.input-bar:focus {
background-color: linear-gradient(180deg, #ffff 0%, #f6f6f7 100%);

Choose a reason for hiding this comment

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

The value for the background-color property seems to be incorrect. To use a gradient, you should use the background or background-image property instead.

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