Skip to content

task soluiton#5482

Closed
liceduardocastillo1386-commits wants to merge 1 commit into
mate-academy:masterfrom
liceduardocastillo1386-commits:develop
Closed

task soluiton#5482
liceduardocastillo1386-commits wants to merge 1 commit 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 requesting changes because one explicit core requirement about the stylesheet entry file is not met, which makes the solution non‑compliant with the task description. The task clearly says styles must be written in src/styles/main.scss, but your main SCSS file is src/styles/index.scss, and index.html links to styles/index.css instead of using main.scss as the entry point; this is a critical mismatch with the specified file path/name, not just a cosmetic choice. To fix this, rename or move src/styles/index.scss to src/styles/main.scss, ensure it is the SCSS entry that Parcel compiles, and update the <link> tag in src/index.html to reference styles/main.scss (or the CSS generated from main.scss, depending on the expected setup) so it literally follows the requirement. In addition, you currently duplicate the same SCSS variable definitions in src/styles/_variables.scss and in your main SCSS file without importing _variables.scss; instead, import _variables.scss into main.scss and keep the variables defined in one place to follow the intent of using shared, clearly named variables. Functionally, your calendar behavior, BEM structure, hover effects, and modifier logic are all implemented very well, and changing calendar--start-day-* / calendar--month-length-* correctly affects the layout [CHECKLIST ITEM #1], with each day using only calendar__day [CHECKLIST ITEM #2]. Once you align the stylesheet entry file with main.scss and centralize the variables via the partial import, your solution will fully satisfy both the functional and structural requirements.


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

Important

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

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

Footnotes

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

Comment thread src/index.html
<link
rel="stylesheet"
href="styles/index.scss"
href="styles/index.css"

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 description requires: Write styles in src/styles/main.scss instead of src/style.css, but your main stylesheet is index.scss and you link to styles/index.css. You need to move/rename your SCSS to main.scss (and adjust the link) to comply with the specified file path and name.

@@ -0,0 +1,6 @@
$day-size: 100px;

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 explicitly says: Write styles in src/styles/main.scss instead of src/style.css. You are using index.scss/index.css instead of main.scss, so you need to move/rename your SCSS entry file to main.scss (and its compiled CSS accordingly) to comply with the requirement.

Comment thread src/styles/index.css
margin: 0;
font-family: Arial, sans-serif;
font-size: 30px;
}

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, but the HTML links to styles/index.css and there is no main.scss file. This violates the description requirement to use main.scss for styles; consider either renaming your SCSS entry file to main.scss and updating the link, or creating main.scss that imports your current SCSS.

Comment thread src/styles/index.css
Comment on lines +1 to +6
/* stylelint-disable no-descending-specificity */
/* stylelint-disable rule-empty-line-before */
body {
display: flex;
flex-direction: column;
justify-content: center;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You defined base calendar variables in _variables.scss, but did not import that file here and instead duplicated the same variable declarations. This contradicts the requirement to avoid repeated hardcoded values and use shared variables for clarity; consider importing _variables.scss into this file and removing the duplicate definitions.

Comment thread src/styles/index.scss
tue: 1,
wed: 2,
thu: 3,
fri: 4,

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 description requires: Write styles in src/styles/main.scss instead of src/style.css. Currently the file you use as the main SCSS entry is index.scss, and you link styles/index.css here. You should have a main.scss as the entry point and link that (Parcel will build the CSS), to comply with the task requirement.

Comment thread src/styles/index.scss
Comment on lines +1 to +18
$day-size: 100px;
$day-color: #eee;
$border-color: black;
$gap: 1px;
$columns: 7;
$padding: 10px;
$days: (
mon: 0,
tue: 1,
wed: 2,
thu: 3,
fri: 4,
sat: 5,
sun: 6,
);
$month-length: (28, 29, 30, 31);
$calendar-font: Arial, sans-serif;
$calendar-font-size: 30px;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You already have _variables.scss with the same calendar variables defined, but this file redefines them and does not import _variables.scss. This makes the shared variables file unused and duplicates definitions, which goes against the requirement to use properly named variables to make calculations clearer. Consider importing _variables.scss and keeping variables in a single place.

@liceduardocastillo1386-commits liceduardocastillo1386-commits closed this by deleting the head repository Jul 14, 2026
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