Calendar add task solution#5476
Conversation
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your solution is approved because all core functional requirements are implemented correctly and the calendar behaves as specified (centered layout, 31 generated days, correct start-day and month-length modifiers, and proper hover effects). The remaining issues are minor code-style details: in src/styles/main.scss and src/styles/index.scss, height: 100px should ideally reuse the $day-width variable so you don’t repeat the raw pixel value, and cursor: pointer would be more precisely aligned with the requirement if it were only applied in the :hover state. These are good improvements to make, but they don’t block functionality or violate any critical parts of the task. If you iterate further, consider changing both day width and height to use the same SCSS variable and referencing main.scss as the primary linked stylesheet to match the wording of the instructions more strictly. Overall, your SCSS structure, BEM usage, and use of loops/maps (@for, @each) are solid and show good understanding of the task’s goals.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
|
|
||
| .visually-hidden { | ||
| position: absolute !important; |
There was a problem hiding this comment.
The description requires: Write styles in src/styles/main.scss instead of src/style.css. and also mentions that SCSS files can be linked directly. Right now you are linking styles/index.scss, which means main.scss is only included via @import. To strictly follow the requirement, consider making main.scss the primary stylesheet (or at least ensure it’s the one directly referenced according to the task wording).
| border: 0 !important; | ||
|
|
There was a problem hiding this comment.
The task says: Each day is a grey (#eee) 100px square (including 1px black border) and also: Don't use hardcoded px values if they are used several times and Use properly named variables to make all the calculations more clear. Here height: 100px; is a hardcoded duplicate of $day-width: 100px;. Consider using $day-width for height as well to keep the square and avoid repeating the raw value.
| white-space: nowrap !important; | ||
|
|
||
| clip: rect(0, 0, 0, 0) !important; | ||
| } |
There was a problem hiding this comment.
The hover behavior should set cursor: pointer on hover specifically: the requirement says On hovering over a cell: cursor should become pointer. You currently set cursor: pointer on the base .calendar__day, so the cursor is always a pointer even when not hovering. Moving this to the &:hover block (or adding cursor: pointer there and removing it from the base state) would align more precisely with the checklist item.
| justify-content: center; | ||
| align-items: center; | ||
| width: $day-width; | ||
| height: 100px; |
There was a problem hiding this comment.
This uses a hardcoded 100px height while you already defined $day-width: 100px;. This violates the requirement "Don't use hardcoded px values if they are used several times"; consider using $day-width (or a shared variable) for both width and height so the square size is controlled by a single variable.
DEMO LINK
TEST REPORT LINK
[ x ] Changing 'month-lengh' and 'start-day' modifier in the code element
reflects in changing calendar layout
[ x ] Each day has no modifiers, only class (eg. calendar__day)
[ x ] All
Typical MistakesfromBEMlesson theory are checked.[ x ] Code follows all the [Code Style Rules ❗️]