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.

Sign up for GitHub

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

Jump to bottom

Develop #5146

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

Develop #5146

wants to merge 5 commits into from

Conversation

JoaoAndreassa
Copy link

mateacademy-ai-mentor
mateacademy-ai-mentor suggested changes Feb 24, 2025
View reviewed changes
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.

Thank you for your hard work on this project! There are a few critical issues that need to be addressed before we can approve the pull request. The repeated content in the product cards suggests a potential misunderstanding of the task requirements, which could impact the functionality if different products are intended to be displayed. Additionally, there are several missing SCSS variables, such as $font-stack, $blue-color, $secondary-color, $dark-gray, and $border-color, which could lead to compilation errors. Please ensure these variables are defined. Once these issues are resolved, your code will be in great shape! Keep up the great work, and don't hesitate to reach out if you need any help. 😊

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

🔄 Fix the issues, commit the changes, and then re-request my review using the Re-request review button in the Reviewers tab.

src/index.html
Comment on lines +102 to +352
<div class="card__price">
<p class="card__price-description">Price:</p>
<p class="card__price-value">$2,199</p>
</div>
<button class="card__link">BUY</button>
</section>
<section
class="card"
data-qa="card"
>
<div class="card__img-container">
<img
class="card__img"
src="images/imac.jpeg"
alt="product_photo"
/>
</div>
<h2 class="card__title">
APPLE A1419 iMac 27" Retina 5K Monoblock (MNED2UA/A)
</h2>
<p class="card__code">Product code: 195434</p>
<div class="card__review">
<div class="stars stars--4">
<div class="stars__star"></div>
<div class="stars__star"></div>
<div class="stars__star"></div>
<div class="stars__star"></div>
<div class="stars__star stars__star--inactive"></div>
</div>
<p class="card__review-amount">Reviews: 5</p>
</div>
<div class="card__price">
<p class="card__price-description">Price:</p>
<p class="card__price-value">$2,199</p>
</div>
<button class="card__link">BUY</button>
</section>
<section
class="card"
data-qa="card"
>
<div class="card__img-container">
<img
class="card__img"
src="images/imac.jpeg"
alt="product_photo"
/>
</div>
<h2 class="card__title">
APPLE A1419 iMac 27" Retina 5K Monoblock (MNED2UA/A)
</h2>
<p class="card__code">Product code: 195434</p>
<div class="card__review">
<div class="stars stars--4">
<div class="stars__star"></div>
<div class="stars__star"></div>
<div class="stars__star"></div>
<div class="stars__star"></div>
<div class="stars__star stars__star--inactive"></div>
</div>
<p class="card__review-amount">Reviews: 5</p>
</div>
<div class="card__price">
<p class="card__price-description">Price:</p>
<p class="card__price-value">$2,199</p>
</div>
<button class="card__link">BUY</button>
</section>
<section
class="card"
data-qa="card"
>
<div class="card__img-container">
<img
class="card__img"
src="images/imac.jpeg"
alt="product_photo"
/>
</div>
<h2 class="card__title">
APPLE A1419 iMac 27" Retina 5K Monoblock (MNED2UA/A)
</h2>
<p class="card__code">Product code: 195434</p>
<div class="card__review">
<div class="stars stars--4">
<div class="stars__star"></div>
<div class="stars__star"></div>
<div class="stars__star"></div>
<div class="stars__star"></div>
<div class="stars__star stars__star--inactive"></div>
</div>
<p class="card__review-amount">Reviews: 5</p>
</div>
<div class="card__price">
<p class="card__price-description">Price:</p>
<p class="card__price-value">$2,199</p>
</div>
<button class="card__link">BUY</button>
</section>
</div>
Copy link

Choose a reason for hiding this comment

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

The product cards are repeated multiple times with the same content. If the intention is to display different products, consider using a templating engine or JavaScript to dynamically generate these cards with varying data.

src/styles/_card.scss Outdated
width: 200px;
padding: 32px 16px 16px;
background-color: white;
border: 1px solid $border-color;
Copy link

Choose a reason for hiding this comment

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

Ensure that the variable $border-color is defined in your SCSS files. If it's not defined, this will cause a compilation error.

src/styles/_card.scss Outdated

&__title {
text-align: left;
color: $secondary-color;
Copy link

Choose a reason for hiding this comment

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

Ensure that the variable $secondary-color is defined in your SCSS files. If it's not defined, this will cause a compilation error.

src/styles/_card.scss
font-size: 10px;
font-weight: 400;
line-height: 14px;
color: $dark-gray;
Copy link

Choose a reason for hiding this comment

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

Ensure that the variable $dark-gray is defined in your SCSS files. If it's not defined, this will cause a compilation error.

src/styles/_card.scss Outdated

&__review-amount {
text-align: right;
color: $secondary-color;
Copy link

Choose a reason for hiding this comment

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

Ensure that the variable $secondary-color is defined in your SCSS files. If it's not defined, this will cause a compilation error.

src/styles/_card.scss
&:hover {
background-color: white;
color: $blue-color;
border: 1px solid $blue-color;
Copy link

Choose a reason for hiding this comment

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

Ensure that the variable $blue-color is defined in your SCSS files. If it's not defined, this will cause a compilation error.

src/styles/_card.scss
Comment on lines +115 to +117
main {
grid-template-columns: repeat(2, 1fr);
}
Copy link

Choose a reason for hiding this comment

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

The main selector here assumes a grid layout, but there is no grid-related CSS in the main styles. Ensure that the main element is set up as a grid container elsewhere in your styles.

src/styles/_card.scss
Comment on lines +121 to +123
main {
grid-template-columns: repeat(1, 1fr);
}
Copy link

Choose a reason for hiding this comment

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

The main selector here assumes a grid layout, but there is no grid-related CSS in the main styles. Ensure that the main element is set up as a grid container elsewhere in your styles.

src/styles/_header.scss
.header {
.nav {
.nav__list {
.nav-item {
Copy link

Choose a reason for hiding this comment

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

There is a typo in the class name .nav-item. It should be .nav__item to match the class name used elsewhere in the file. This will ensure consistent styling across different screen sizes.

src/styles/_reset.scss
margin: 0;
padding: 0;
box-sizing: border-box;
font-family: $font-stack;
Copy link

Choose a reason for hiding this comment

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

Ensure that the variable $font-stack is defined in your SCSS files. If it's not defined, this will cause a compilation error.

mateacademy-ai-mentor
mateacademy-ai-mentor approved these changes Feb 24, 2025
View reviewed changes
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.

Thank you for your submission! 🎉 Your SCSS files are well-structured, and you've addressed the previously missing variables, which is great progress. However, there are a few areas to improve:

  • Product Cards: All product cards currently have identical content. To better reflect a realistic catalog page, each card should display unique product details, including different images, titles, product codes, reviews, and prices.

  • SCSS Variables: Ensure all variables are defined in your SCSS variables file to avoid compilation errors. You've made good progress, but double-check that all necessary variables are included.

  • Typo in Header SCSS: There's a typo in the media query section where .nav-item should be .nav__item to match the class used elsewhere.

These improvements will enhance the functionality and presentation of your project. Keep up the great work, and continue learning from these experiences! 🚀

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

src/index.html
Comment on lines +102 to +351
</div>
<div class="card__price">
<p class="card__price-description">Price:</p>
<p class="card__price-value">$2,199</p>
</div>
<button class="card__link">BUY</button>
</section>
<section
class="card"
data-qa="card"
>
<div class="card__img-container">
<img
class="card__img"
src="images/imac.jpeg"
alt="product_photo"
/>
</div>
<h2 class="card__title">
APPLE A1419 iMac 27" Retina 5K Monoblock (MNED2UA/A)
</h2>
<p class="card__code">Product code: 195434</p>
<div class="card__review">
<div class="stars stars--4">
<div class="stars__star"></div>
<div class="stars__star"></div>
<div class="stars__star"></div>
<div class="stars__star"></div>
<div class="stars__star stars__star--inactive"></div>
</div>
<p class="card__review-amount">Reviews: 5</p>
</div>
<div class="card__price">
<p class="card__price-description">Price:</p>
<p class="card__price-value">$2,199</p>
</div>
<button class="card__link">BUY</button>
</section>
<section
class="card"
data-qa="card"
>
<div class="card__img-container">
<img
class="card__img"
src="images/imac.jpeg"
alt="product_photo"
/>
</div>
<h2 class="card__title">
APPLE A1419 iMac 27" Retina 5K Monoblock (MNED2UA/A)
</h2>
<p class="card__code">Product code: 195434</p>
<div class="card__review">
<div class="stars stars--4">
<div class="stars__star"></div>
<div class="stars__star"></div>
<div class="stars__star"></div>
<div class="stars__star"></div>
<div class="stars__star stars__star--inactive"></div>
</div>
<p class="card__review-amount">Reviews: 5</p>
</div>
<div class="card__price">
<p class="card__price-description">Price:</p>
<p class="card__price-value">$2,199</p>
</div>
<button class="card__link">BUY</button>
</section>
<section
class="card"
data-qa="card"
>
<div class="card__img-container">
<img
class="card__img"
src="images/imac.jpeg"
alt="product_photo"
/>
</div>
<h2 class="card__title">
APPLE A1419 iMac 27" Retina 5K Monoblock (MNED2UA/A)
</h2>
<p class="card__code">Product code: 195434</p>
<div class="card__review">
<div class="stars stars--4">
<div class="stars__star"></div>
<div class="stars__star"></div>
<div class="stars__star"></div>
<div class="stars__star"></div>
<div class="stars__star stars__star--inactive"></div>
</div>
<p class="card__review-amount">Reviews: 5</p>
</div>
<div class="card__price">
<p class="card__price-description">Price:</p>
<p class="card__price-value">$2,199</p>
</div>
<button class="card__link">BUY</button>
</section>
Copy link

Choose a reason for hiding this comment

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

All product cards have identical content. If the task requires displaying different products, each card should have unique content, such as different images, titles, product codes, reviews, and prices. Please update the product cards to reflect the intended variety of products.

src/styles/_card.scss
.card {
width: 200px;
padding: 32px 16px 16px;
background-color: $white-color;
Copy link

Choose a reason for hiding this comment

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

The variable $white-color is used here but is not defined. Please ensure it is defined in your SCSS variables file.

src/styles/_card.scss
width: 200px;
padding: 32px 16px 16px;
background-color: $white-color;
border: 1px solid $light-gray;
Copy link

Choose a reason for hiding this comment

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

The variable $light-gray is used here but is not defined. Please ensure it is defined in your SCSS variables file.

src/styles/_card.scss

&__title {
text-align: left;
color: $blue-nav;
Copy link

Choose a reason for hiding this comment

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

The variable $blue-nav is used here but is not defined. Please ensure it is defined in your SCSS variables file.

src/styles/_card.scss
font-size: 10px;
font-weight: 400;
line-height: 14px;
color: $dark-gray;
Copy link

Choose a reason for hiding this comment

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

The variable $dark-gray is used here but is not defined. Please ensure it is defined in your SCSS variables file.

src/styles/_header.scss
align-items: center;
justify-content: space-between;
padding-inline: 50px;
box-shadow: 0 2px 4px $border-color;
Copy link

Choose a reason for hiding this comment

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

The variable $border-color is used here but is not defined. Please ensure it is defined in your SCSS variables file.

src/styles/_header.scss
.header {
.nav {
.nav__list {
.nav-item {
Copy link

Choose a reason for hiding this comment

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

There is a typo here: .nav-item should be .nav__item to match the class used elsewhere in the file.

src/styles/_reset.scss
margin: 0;
padding: 0;
box-sizing: border-box;
font-family: $font-stack;
Copy link

Choose a reason for hiding this comment

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

The variable $font-stack is used here but is not defined. Please ensure it is defined in your SCSS variables file.

src/styles/_reset.scss
.nav {
display: flex;
justify-content: center;
background: $white-color;
Copy link

Choose a reason for hiding this comment

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

The variable $white-color is used here but is not defined. Please ensure it is defined in your SCSS variables file.

src/styles/_reset.scss
}

.card {
background: $white-color;
Copy link

Choose a reason for hiding this comment

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

The variable $white-color is used here but is not defined. Please ensure it is defined in your SCSS variables file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mateacademy-ai-mentor mateacademy-ai-mentor mateacademy-ai-mentor approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@JoaoAndreassa @mateacademy-ai-mentor