add task solution#5479
Conversation
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because there is a critical mismatch with the task’s explicit file/path requirement for the stylesheet, even though your calendar implementation and SCSS logic are otherwise correct. The description states that styles must be written in src/styles/main.scss instead of src/style.css, but you currently use src/styles/index.scss and link it in src/index.html as ./styles/index.scss, which does not satisfy that requirement and blocks completion.
To fix this:
- Rename or move your current
src/styles/index.scsstosrc/styles/main.scss(keep the existing styles and structure as they are). - In
src/index.html, update the<link>tag’shreffrom./styles/index.scssto./styles/main.scssso the HTML references the required file.
Functionally and stylistically, your calendar behaves as specified: the layout, hover behavior, SCSS loops/maps, BEM naming, and modifier classes all look good and meet the checklist requirements. Once you align the stylesheet file name and the link in HTML with the task requirement, your solution will be ready to pass. Keep up the good work—this is a small but important naming fix to bring everything in line with the spec.
✨ 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
| <link | ||
| rel="stylesheet" | ||
| href="styles/index.scss" | ||
| href="./styles/index.scss" |
There was a problem hiding this comment.
The description requires: Write styles in src/styles/main.scss instead of src/style.css. but this file links to ./styles/index.scss. You should have your main SCSS in src/styles/main.scss and reference that file here to match the task specification.
| $calendar-content-width: $calendar-days-width + $calendar-gaps-width; | ||
| $calendar-width: $calendar-content-width + $calendar-padding * 2; | ||
| $hover-shift: 20px; | ||
| $transition-duration: 0.5s; |
There was a problem hiding this comment.
Task description explicitly requires: Write styles in src/styles/main.scss instead of src/style.css. Your stylesheet is named index.scss and linked as ./styles/index.scss, so this violates the requirement; rename the SCSS file to main.scss and update the href accordingly.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving your solution because it fully meets the task requirements and the functional checklist. The calendar is correctly centered, uses flexbox with variables for dimensions and gaps, and contains exactly 31 .calendar__day elements in the HTML without hardcoded numbers. Day numbers are injected purely via SCSS using a ::before pseudo-element and an @for loop, and the calendar--start-day-* and calendar--month-length-* modifiers are generated via @each/@for and correctly control layout using margin-left and nth-child with display: none. Hover behavior is implemented as requested: pointer cursor, pink background #FFBFCB, smooth 0.5s transition, and an upward shift of 20px.
The BackstopJS failure is a minor visual regression difference on the hover screenshot (about 1.4% vs a 0.5% threshold) and appears to be a flaky rendering issue rather than a bug in your implementation, so it should not block this task. BEM naming, SCSS structure, and the absence of JavaScript all align with the constraints, and changing the start-day and month-length modifiers in HTML will correctly reconfigure the calendar as required [CHECKLIST ITEM #1, #2]. Nicely done sticking to the requirements and using SCSS features effectively; you can treat this as a solid reference for similar layout and modifier-based tasks in the future.
✨ 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
Anton-Kuchmasov
left a comment
There was a problem hiding this comment.
All tests are passed. Good job!
DEMO LINK
TEST REPORT LINK
Changing 'month-lengh' and 'start-day' modifier in the code element reflects in changing calendar layout
Each day has no modifiers, only class (eg. calendar__day)
All
Typical MistakesfromBEMlesson theory are checkedCode follows all the [Code Style Rules ❗️]
Fixed:
src/styles/index.scsstosrc/styles/main.scsssrc/index.html