Skip to content

Develop#5484

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

Develop#5484
liceduardocastillo1386-commits wants to merge 2 commits into
mate-academy:masterfrom
liceduardocastillo1386-commits:develop

Conversation

@liceduardocastillo1386-commits

Copy link
Copy Markdown

-task solution chance solicited 2

@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 of the task is not met: your styles are implemented and linked as src/styles/index.scss instead of the required src/styles/main.scss, and the HTML <link> still points to styles/index.scss. This is a critical, blocking issue because the task description clearly mandates main.scss as the entry point and the checklist explicitly requires that styles live there. To fix this, rename or move your current src/styles/index.scss contents into src/styles/main.scss, then update src/index.html so the <link rel="stylesheet" href="..."> points to styles/main.scss rather than styles/index.scss or index.css. While not strictly blocking, you also have _variables.scss defined but not imported; it would be cleaner to either import _variables.scss into main.scss (for shared variables) or consolidate your variables into a single file to avoid duplication. Functionally, your calendar behavior, SCSS loops, BEM usage, hover effects, and modifier logic all look good and align well with the task’s functional requirements and [CHECKLIST ITEM #1] / [CHECKLIST ITEM #2]. Once you address the stylesheet location/name and the HTML link, your solution should be ready to be accepted. Keep up the good work—your implementation is very close; this is mainly about aligning with the required file structure and naming.


✨ 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/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.

According to the task description, styles must be written in src/styles/main.scss and that SCSS file should be the entry point. Here you link to styles/index.scss, which violates that requirement; consider creating/using main.scss and referencing it here.

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 task requires: "Write styles in src/styles/main.scss instead of src/style.css", but your HTML links to styles/index.scss and there is no main.scss. You should place the main styles in main.scss and update this href accordingly.

Comment thread src/styles/index.scss
@@ -1,3 +1,81 @@
$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.

According to the description, styles should be written in src/styles/main.scss, but you’ve defined them in index.scss. This violates the requirement on the main SCSS file name/location; consider moving this code into main.scss (and, if you keep _variables.scss, importing/using it there).

@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