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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

WhiteP8wder8
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 submission! 🎉 Your code meets most of the task requirements and constraints, and you've done well in structuring your HTML and SCSS files. Here are some key points and areas for improvement:

  • BEM Blocks: You've correctly implemented the card and stars blocks in their respective SCSS files.
  • SCSS Nesting and Variables: Your use of SCSS nesting is effective, and you've placed variables in a separate variables.scss file. However, ensure that $main-color, $second-color, and $blue are defined and correctly imported to avoid any issues.
  • Attributes and Styles: You've added the required data-qa attributes and implemented hover styles for links. The card width is set correctly, and the font is imported from Google Fonts.
  • Browser Default Margins: Remember to reset the browser's default margins to meet the task requirements.

While there are minor issues, such as ensuring all SCSS variables are defined and resetting browser margins, these do not prevent your code from being functional. Please review the comments to improve your code further. Keep up the good work and continue learning from these experiences! 🚀

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

line-height: 18px;
margin: 0;
padding-bottom: 4px;
color: $main-color;

Choose a reason for hiding this comment

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

The variable $main-color is used here, but it's not defined in this file. Ensure that it is defined in a file that is imported into this one, or define it here.

line-height: 14px;
margin: 0;
padding-bottom: 16px;
color: $second-color;

Choose a reason for hiding this comment

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

The variable $second-color is used here, but it's not defined in this file. Ensure that it is defined in a file that is imported into this one, or define it here.

font-weight: 400;
line-height: 14px;
text-align: right;
color: $main-color;

Choose a reason for hiding this comment

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

The variable $main-color is used here, but it's not defined in this file. Ensure that it is defined in a file that is imported into this one, or define it here.

text-align: left;
font-size: 12px;
line-height: 18px;
color: $second-color;

Choose a reason for hiding this comment

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

The variable $second-color is used here, but it's not defined in this file. Ensure that it is defined in a file that is imported into this one, or define it here.

line-height: 18px;
text-align: right;
font-weight: 700;
color: $main-color;

Choose a reason for hiding this comment

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

The variable $main-color is used here, but it's not defined in this file. Ensure that it is defined in a file that is imported into this one, or define it here.

&__button {
box-sizing: border-box;
border-radius: 5px;
background-color: $blue;

Choose a reason for hiding this comment

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

The variable $blue is used here, but it's not defined in this file. Ensure that it is defined in a file that is imported into this one, or define it here.


&:hover {
background-color: #fff;
color: $blue;

Choose a reason for hiding this comment

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

The variable $blue is used here, but it's not defined in this file. Ensure that it is defined in a file that is imported into this one, or define it here.

&:hover {
background-color: #fff;
color: $blue;
border: 1px solid $blue;

Choose a reason for hiding this comment

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

The variable $blue is used here, but it's not defined in this file. Ensure that it is defined in a file that is imported into this one, or define it here.

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