-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Catalog #4906
base: master
Are you sure you want to change the base?
Catalog #4906
Conversation
TaniaP26
commented
Dec 7, 2024
- DEMO LINK
- TEST REPORT LINK
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.
Sorry, the pull request review was unsuccessful for the following reasons:
Warning
There are no files to review. Please make sure that you have added files to the pull request. Some files and directories may be ignored from the review or failed to load.
[CODE: 4]
Additional info:
Ignored files list:
src/index.html
src/styles/blocks/cards.scss
src/styles/blocks/header.scss
src/styles/blocks/stars.scss
src/styles/index.scss
src/utils/mixins.scss
src/utils/variables.scss
If the issue is within your control, please address it.
The next review attempt will automatically launch in 1 hour.
If you believe this is an error or need assistance, please contact the support team.
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.
Sorry, the pull request review was unsuccessful for the following reasons:
Warning
There are no files to review. Please make sure that you have added files to the pull request. Some files and directories may be ignored from the review or failed to load.
[CODE: 4]
Additional info:
Ignored files list:
src/index.html
src/styles/blocks/cards.scss
src/styles/blocks/header.scss
src/styles/blocks/stars.scss
src/styles/index.scss
src/utils/mixins.scss
src/utils/variables.scss
If the issue is within your control, please address it.
The next review attempt will automatically launch in 1 hour.
If you believe this is an error or need assistance, please contact the support team.
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.
Sorry, the pull request review was unsuccessful for the following reasons:
Warning
There are no files to review. Please make sure that you have added files to the pull request. Some files and directories may be ignored from the review or failed to load.
[CODE: 4]
Additional info:
Ignored files list:
src/index.html
src/styles/blocks/cards.scss
src/styles/blocks/header.scss
src/styles/blocks/stars.scss
src/styles/index.scss
src/utils/mixins.scss
src/utils/variables.scss
If the issue is within your control, please address it.
The next review attempt will automatically launch in 1 hour.
If you believe this is an error or need assistance, please contact the support team.
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.
Sorry, the pull request review was unsuccessful for the following reasons:
Warning
There are no files to review. Please make sure that you have added files to the pull request. Some files and directories may be ignored from the review or failed to load.
[CODE: 4]
Additional info:
Ignored files list:
src/index.html
src/styles/blocks/cards.scss
src/styles/blocks/header.scss
src/styles/blocks/stars.scss
src/styles/index.scss
src/utils/mixins.scss
src/utils/variables.scss
If the issue is within your control, please address it.
The next review attempt will automatically launch in 1 hour.
If you believe this is an error or need assistance, please contact the support team.
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.
src/index.html
Outdated
<div class="card__title"> | ||
APPLE A1419 iMac 27" Retina 5K Monoblock (MNED2UA/A) | ||
</div> |
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.
<div class="card__title"> | |
APPLE A1419 iMac 27" Retina 5K Monoblock (MNED2UA/A) | |
</div> | |
<h3 class="card__title"> | |
APPLE A1419 iMac 27" Retina 5K Monoblock (MNED2UA/A) | |
</h3> |
Consider leveraging semantic tags
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.
Not fixed
src/index.html
Outdated
APPLE A1419 iMac 27" Retina 5K Monoblock (MNED2UA/A) | ||
</div> | ||
|
||
<div class="card__product-code">Product code: 195434</div> |
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.
<div class="card__product-code">Product code: 195434</div> | |
<p class="card__product-code">Product code: 195434</p> |
src/index.html
Outdated
<div class="card__product-code">Product code: 195434</div> | ||
|
||
<div class="card__reviews"> | ||
<div class="card__review-stars stars stars--1"> |
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.
<div class="card__review-stars stars stars--1"> | |
<div class="card__review-stars stars stars--4"> |
src/index.html
Outdated
<div class="stars__star"></div> | ||
</div> | ||
|
||
<div class="card__reviews-reviews">Reviews: 5</div> |
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.
<div class="card__reviews-reviews">Reviews: 5</div> | |
<p class="card__reviews-reviews">Reviews: 5</p> |
src/index.html
Outdated
<div class="card__price"> | ||
<div class="card__price-how">Price</div> | ||
<div class="card__price-what">$2.199</div> | ||
</div> |
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.
<div class="card__price"> | |
<div class="card__price-how">Price</div> | |
<div class="card__price-what">$2.199</div> | |
</div> | |
<p class="card__price"> | |
<span class="card__price-how">Price</span> | |
<span class="card__price-what">$2.199</span> | |
</p> |
src/styles/blocks/cards.scss
Outdated
} | ||
|
||
&--button:hover { | ||
background-color: #fff; |
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.
Consider using a variable here
src/styles/blocks/header.scss
Outdated
} | ||
} | ||
|
||
li:first-child a { |
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.
li:first-child a { | |
&__item:first-child &__link { |
src/styles/blocks/stars.scss
Outdated
margin-right: 0; | ||
} | ||
|
||
.stars--1 .stars__star:nth-child(-n + 1) { |
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.
.stars--1 .stars__star:nth-child(-n + 1) { | |
.stars--1 .stars__star:nth-child(-n + 1), | |
.stars--2 .stars__star:nth-child(-n + 2), | |
.stars--3 .stars__star:nth-child(-n + 3), | |
.stars--4 .stars__star:nth-child(-n + 4), | |
.stars--5 .stars__star:nth-child(-n + 5) { |
src/styles/blocks/stars.scss
Outdated
.stars--2 .stars__star:nth-child(-n + 2) { | ||
background-image: url(../images/star-active.svg); | ||
} | ||
|
||
.stars--3 .stars__star:nth-child(-n + 3) { | ||
background-image: url(../images/star-active.svg); | ||
} | ||
|
||
.stars--4 .stars__star:nth-child(-n + 4) { | ||
background-image: url(../images/star-active.svg); | ||
} | ||
|
||
.stars--5 .stars__star:nth-child(-n + 5) { | ||
background-image: url(../images/star-active.svg); | ||
} |
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.
.stars--2 .stars__star:nth-child(-n + 2) { | |
background-image: url(../images/star-active.svg); | |
} | |
.stars--3 .stars__star:nth-child(-n + 3) { | |
background-image: url(../images/star-active.svg); | |
} | |
.stars--4 .stars__star:nth-child(-n + 4) { | |
background-image: url(../images/star-active.svg); | |
} | |
.stars--5 .stars__star:nth-child(-n + 5) { | |
background-image: url(../images/star-active.svg); | |
} |
src/index.html
Outdated
@@ -23,5 +23,380 @@ | |||
|
|||
<body> | |||
<h1>Catalog</h1> |
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.
<h1>Catalog</h1> |
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.
Good job!
Let's improve your code
Cards should shrink as smoothly as they grow.
Also, you need to fix all comments from the previous review
src/index.html
Outdated
@@ -23,5 +23,380 @@ | |||
|
|||
<body> | |||
<h1>Catalog</h1> |
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.
src/index.html
Outdated
<div class="card__title"> | ||
APPLE A1419 iMac 27" Retina 5K Monoblock (MNED2UA/A) | ||
</div> |
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.
Not fixed
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.
Consider fixing previous comments
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.
Hi, the comments from the previous review still not fixed, you need to fix all comments and make deploy again after your changes
.stars { | ||
display: flex; | ||
} | ||
|
||
.stars__star { |
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.
.stars { | |
display: flex; | |
} | |
.stars__star { | |
.stars { | |
display: flex; | |
&__star { | |
........ |
you should use nesting
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