add solution#5468
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving your solution because all core functional requirements for the calendar are correctly implemented: 31 days without numbers in HTML, numbers added via SCSS ::before and @for, proper flex layout with 7 columns, correct start-day and month-length modifiers, and the required hover behavior. The calendar responds correctly when changing modifiers, which satisfies the checklist items about layout being driven by calendar--start-day-* and calendar--month-length-*.
There is one BEM-related issue that you should improve next time: in page.scss you style body instead of the .page block class, even though HTML uses class="page" on <body>; for strict BEM compliance and [CHECKLIST ITEM #3], this should be .page { ... } instead of body { ... }. Also, consider extracting the 20px hover shift into a variable (e.g. $hover-shift: 20px;) to keep all important sizing values centralized and easier to adjust. Overall, your SCSS structure, use of control directives, and adherence to the calendar-related BEM rules are solid—nice work on meeting the functional and layout requirements.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| </head> | ||
| <body> | ||
| <h1>Calendar</h1> | ||
| <body class="page"> |
There was a problem hiding this comment.
The HTML uses class="page" on the body, but the SCSS defines styles for body directly, not for a .page block. Since the task mentions using BEM and a page block file exists, consider applying the centering styles to .page instead of body to align with the intended BEM structure.
| @@ -0,0 +1,55 @@ | |||
| .calendar { | |||
There was a problem hiding this comment.
This violates BEM and checklist item #3 ("All Typical Mistakes from BEM lesson theory are checked"): the page block is used in HTML (<body class="page">) but here you style the body element instead of the .page class. Update this to .page so the block is styled via its class, not a tag selector.
| @@ -0,0 +1,7 @@ | |||
| body { | |||
There was a problem hiding this comment.
The page block is used as a class on <body> in HTML, but here it is styled as the bare body element, which violates BEM and checklist item #3 about following BEM typical mistakes; use .page instead of body for this block.
| @@ -0,0 +1,3 @@ | |||
| @import './variables'; | |||
There was a problem hiding this comment.
This selector should target the BEM block class .page instead of the raw body element to align with BEM and checklist item #3 about avoiding typical BEM mistakes.
| @@ -0,0 +1,5 @@ | |||
| $column: 7; | |||
There was a problem hiding this comment.
The page block is used in HTML (<body class="page">), but the styles are currently attached to the raw body selector in page.scss. This violates the requirement to follow BEM and checklist item #3 about typical BEM mistakes; update page.scss to style .page instead of body so the block is controlled via its class.
2 participants
❗️ Copy this
Checklistto thePull Requestdescription after links, and put- [x]before each point after you checked it.reflects in changing calendar layout
Typical MistakesfromBEMlesson theory are checked.