Conversation
frontwebil
commented
Feb 18, 2026
frontwebil
commented
- DEMO LINK
- TEST REPORT LINK
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because there is a critical functional issue with the start-day positioning that violates the modifiers requirement [CHECKLIST ITEM #1]. In src/styles/main.scss you define the $days map as mon: 1, ... , sun: 7 and then compute the first cell offset with margin-left: calc((#{$position - 1}) * (#{$day-size} + #{$gap}));, but your HTML grid visually starts with Sunday as the first column. This means that a modifier like calendar--start-day-mon currently places the first day into the Sunday column instead of the Monday column, so changing the start-day modifier doesn’t actually produce the correct calendar layout. To fix this, adjust either the $days map (e.g. sun: 1, mon: 2, tue: 3, ... , sat: 7) or your margin-left calculation so that position 1 corresponds to the leftmost Sunday column in the rendered grid.
All other core requirements (31 days, generated numbers, flex layout, modifiers for month length, hover animation, SCSS structure) look good and only have minor optional improvements like narrowing the transition properties. Once you correct the start-day mapping so that switching between calendar--start-day-sun, calendar--start-day-mon, etc. correctly repositions the first day, your solution will fully meet the task standards. You’re very close; fixing this mapping will make your modifiers behave exactly as required.
✨ 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
| $days: ( | ||
| mon: 1, | ||
| tue: 2, | ||
| wed: 3, | ||
| thu: 4, | ||
| fri: 5, | ||
| sat: 6, | ||
| sun: 7, | ||
| ); | ||
|
|
||
| @each $name, $position in $days { | ||
| &--start-day-#{$name} { | ||
| .calendar__day:first-child { | ||
| margin-left: calc((#{$position - 1}) * (#{$day-size} + #{$gap})); | ||
| } |
There was a problem hiding this comment.
This violates checklist item #1: "Changing 'month-lengh' and 'start-day' modifier in the code element reflects in changing calendar layout". The $days map currently sets mon: 1, ... , sun: 7 and then applies margin-left: calc((#{$position - 1}) * (#{$day-size} + #{$gap}));. The HTML grid starts from Sunday as the first column, so Sunday must map to position 1 and Monday to position 2, etc. As written, modifiers like calendar--start-day-mon will not place the 1st day in the correct weekday column. Consider changing the map to: mon: 2, tue: 3, wed: 4, thu: 5, fri: 6, sat: 7, sun: 1 (or otherwise adjust the calc) so modifier classes correctly reposition the first day.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because one core functional requirement is still broken: changing the start-day modifier does not correctly shift the first day into the right weekday column. The root cause is in src/styles/main.scss, where the $days map currently treats mon as position 1 and sun as 7, while your visual grid has Sunday as the leftmost column, so position: 1 should correspond to Sunday [CHECKLIST ITEM #1]. To fix this, update the $days map so that sun is 1 and the rest follow in order, for example:
sun: 1, mon: 2, tue: 3, wed: 4, thu: 5, fri: 6, sat: 7
With that change, your existing margin-left calculation ($position - 1) * ($day-size + $gap) will correctly place day 1 in the selected weekday column for each calendar--start-day-* modifier. Everything else in your solution (BEM structure, generated day numbers, month-length modifiers, hover behavior, centering, and sizing) is implemented well, so once this mapping is corrected your calendar should fully meet the task requirements. Keep up the good work—this is a small but important alignment fix to make your modifiers behave exactly as specified.
✨ 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
| $days: ( | ||
| mon: 1, | ||
| tue: 2, | ||
| wed: 3, | ||
| thu: 4, | ||
| fri: 5, | ||
| sat: 6, | ||
| sun: 7, |
There was a problem hiding this comment.
This violates the checklist item: "Changing 'month-lengh' and 'start-day' modifier in the code element reflects in changing calendar layout". The $days map currently sets mon: 1, ..., sun: 7, but your HTML/grid uses Sunday as the leftmost column. Change the map so sun is 1 and the rest follow (for example: sun: 1, mon: 2, tue: 3, wed: 4, thu: 5, fri: 6, sat: 7) so the computed margin-left places the first day into the correct weekday column.
| @each $name, $position in $days { | ||
| &--start-day-#{$name} { | ||
| .calendar__day:first-child { | ||
| margin-left: ($position - 1) * ($day-size + $gap); |
There was a problem hiding this comment.
The margin-left calculation uses ($position - 1) * ($day-size + $gap) which is correct only if the map positions match the visual grid ordering (Sunday as leftmost). After updating the map as suggested, this calculation will produce the expected offsets. If you prefer keeping the current numeric map, invert or adjust this calculation so position 1 corresponds to the leftmost (Sunday) column.
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 as an educational override so you can keep progressing while you refine it. You did a strong job with the HTML structure (index.html), using the correct calendar block, the default modifiers (calendar--start-day-sun and calendar--month-length-31), and exactly 31 .calendar__day elements with numbers generated purely via CSS. Your SCSS also covers most of the requirements well: flex layout for a 7-column calendar, computed width with gaps and padding, ::before + @for for day numbers, hover animation, and month-length modifiers that hide extra days.
The main issue is in src/styles/main.scss with the start-day logic [CHECKLIST ITEM #1]. In the $days map (near the @each $day, $position in $days block), you currently map mon: 1 through sun: 7, which makes Monday the first column; however, the visual grid assumes Sunday is the leftmost column. Because margin-left for the first day is calculated as ($position - 1) * ($day-size + $gap), sun must be mapped to 1 (e.g., sun: 1, mon: 2, ..., sat: 7) so that calendar--start-day-<x> modifiers actually shift the first day into the correct weekday column. Aside from adjusting that mapping and regenerating the corresponding &--start-day-* rules, your implementation is solid and very close to fully correct, so this is a great step in learning how to connect SCSS logic with layout behavior.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| $days: ( | ||
| mon: 1, | ||
| tue: 2, | ||
| wed: 3, | ||
| thu: 4, | ||
| fri: 5, | ||
| sat: 6, | ||
| sun: 7, | ||
| ); | ||
|
|
||
| @each $name, $position in $days { | ||
| &--start-day-#{$name} { | ||
| .calendar__day:first-child { | ||
| margin-left: ($position - 1) * ($day-size + $gap); |
There was a problem hiding this comment.
This violates checklist item #1: 'Changing 'month-lengh' and 'start-day' modifier in the code element reflects in changing calendar layout'
The $days map currently maps mon: 1 … sun: 7 which treats Monday as the leftmost column. For a visual grid where Sunday is the leftmost column you must map sun to position 1 so the current margin-left calculation ($position - 1) * ($day-size + $gap) places the first day in the correct weekday column. Change the map to for example:
$days: (sun: 1, mon: 2, tue: 3, wed: 4, thu: 5, fri: 6, sat: 7);
This change will make the &--start-day-* modifiers behave as required. (Issue located in the $days map and the @each block that applies margin-left.)
2 participants
