-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
done #4812
base: master
Are you sure you want to change the base?
done #4812
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix the test report link and then re-request the review
feel free to ask for some help with that in the chat
.stars { | ||
display: flex; | ||
align-items: center; | ||
} | ||
|
||
.stars__star { | ||
width: 16px; | ||
height: 16px; | ||
background-position: center; | ||
background-repeat: no-repeat; | ||
background-image: url(../images/star.svg); | ||
} | ||
|
||
.stars__star:not(:last-child) { | ||
margin-right: 4px; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rewrite it using the sass nesting (&__)
|
||
@media (min-width: 488px) { | ||
grid-template-columns: repeat(2, 200px); | ||
grid-template-rows: repeat(2, 1fr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you don't need to repeat this line in all media queries
|
||
@media (min-width: 768px) { | ||
grid-template-columns: repeat(3, 200px); | ||
grid-template-rows: repeat(2, 1fr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.
|
||
@media (min-width: 1024px) { | ||
grid-template-columns: repeat(4, 200px); | ||
grid-template-rows: repeat(2, 1fr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks so much!
Checklist
❗️ Replace <your_account> with your Github username and copy the links to Pull Request description:
DEMO LINK
TEST REPORT LINK
[x]All component follow BEM and use SCSS
[x]repaeted sizes and special colors are put to variables
[x]Grid is used for the columns
[x]cards are shown in 1, 2, 3 or 4 columns based on screen resolution
[x]All changes on :hover are smooth
[x]Code follows all the Code Style Rules ❗️