Skip to content

Develop#5485

Open
liceduardocastillo1386-commits wants to merge 3 commits into
mate-academy:masterfrom
liceduardocastillo1386-commits:develop
Open

Develop#5485
liceduardocastillo1386-commits wants to merge 3 commits into
mate-academy:masterfrom
liceduardocastillo1386-commits:develop

Conversation

@liceduardocastillo1386-commits

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review Summary

I am approving your solution as an educational override so you can keep progressing while you refine your SCSS skills.

You did a strong job implementing the calendar in src/index.html and src/styles/main.scss: the BEM structure is correct, modifiers like calendar--start-day-* and calendar--month-length-* behave as required, and you used SCSS features such as @for, @each, and variables effectively [CHECKLIST ITEM #1]. The main issue is that src/styles/index.css duplicates the calendar styles with repeated hardcoded px values (for example width: 726px;, width: 100px;, margin-left: calc((100px + 1px) * 4);), which conflicts with the requirement to keep SCSS as the single source of truth and to avoid repeated hardcoded pixel values [CHECKLIST ITEM #4]. To improve, remove the calendar styling from index.css (or remove the file entirely if it’s not needed) and rely solely on the compiled output from main.scss so that all layout changes come from your SCSS modifiers.

As a minor refinement, you can centralize your variables by importing _variables.scss into main.scss and reusing those values instead of redefining them, which will make the code more maintainable. Overall, your structure, use of SCSS control directives, and adherence to BEM are very good—cleaning up the extra CSS file will bring the solution fully in line with the technical constraints and help you build even better habits going forward.


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

Footnotes

  1. Rate AI review example

Comment on lines +1 to +6
$day-size: 100px;
$day-color: #eee;
$border-color: black;
$gap: 1px;
$columns: 7;
$padding: 10px;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This file defines SCSS variables as required, but they are not used in index.css. To avoid violating the constraint against repeated hardcoded px values, ensure only SCSS (compiled from main.scss) is used and remove index.css.

Comment thread src/styles/index.css
Comment on lines +293 to +297
.calendar {
display: flex;
flex-wrap: wrap;
gap: 1px;
width: 726px;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The task requires styles to be written in src/styles/main.scss using variables instead of repeated hardcoded px values. This file contains a full duplicate calendar implementation with hardcoded values (e.g. width: 726px;, margin-left: calc((100px + 1px) * 4);, etc.), which conflicts with that requirement. Consider removing these calendar styles from index.css and relying solely on the SCSS implementation in main.scss.

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