Conversation
Alena-88
commented
Feb 19, 2026
- DEMO LINK
- TEST REPORT LINK
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because a few critical requirements that affect the core functionality of the calendar are not yet met. First, in src/styles/calendar.scss the @for loop that generates month-length modifiers only goes from 28 through 30, so no .calendar--month-length-31 rule is created; this breaks the behavior where changing the calendar--month-length-* modifier in HTML should change how many days are visible [CHECKLIST ITEM #1]. You should extend the loop to include 31, for example @for $i from 28 through 31, so the 31-day case is handled correctly. Second, the weekday offset map in src/styles/variables.scss currently uses 'mon': 0, ... 'sun': 6, which is inverted for a Sunday-first grid; this makes .calendar--start-day-* modifiers place the 1st under the wrong weekday, again violating [CHECKLIST ITEM #1]. Update that map to something like:
$start-day-offsets: (
'sun': 0,
'mon': 1,
'tue': 2,
'wed': 3,
'thu': 4,
'fri': 5,
'sat': 6,
);and adjust the usage in calendar.scss accordingly. Finally, in src/index.html the stylesheet link currently points to styles/main.scss; the task requires linking to src/styles/main.scss, otherwise the grader may not pick up your styles. As a minor improvement (not blocking), renaming the map variable from $days to something clearer like $start-day-offsets will better document its purpose. Overall the structure, BEM usage, and SCSS features are well done; once these functional issues are fixed, the solution should satisfy the task requirements.
✨ 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
| @for $i from 28 through 30 { | ||
| &--month-length-#{$i} { | ||
| .calendar__day:nth-child(n + #{$i + 1}) { | ||
| display: none; | ||
| } | ||
| } | ||
| } |
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 @for loop generating month-length modifiers only runs from 28 through 30, so there is no generated .calendar--month-length-31 rule. Your HTML uses calendar--month-length-31, so that modifier currently has no effect. Suggestion: change the loop to include 31 (e.g. @for $i from 28 through 31) so all required month-length modifiers are generated.
| @each $day, $index in $days { | ||
| &--start-day-#{$day} { | ||
| .calendar__day:first-child { | ||
| margin-left: ($cell-size + $gap) * $index; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This also violates checklist item #1: although you used @each $day, $index in $days to create start-day modifiers, the numeric mapping in $days (see variables.scss) is incorrect for a Sunday-first grid. In src/styles/variables.scss you have 'mon': 0, ... 'sun': 6 which will place --start-day-mon at the leftmost column (index 0) rather than Monday being the second column. Because your margin-left uses ($cell-size + $gap) * $index, the mapping should reflect columns where Sunday = 0, Monday = 1, ..., Saturday = 6 (or adjust the index arithmetic). Suggestion: either change $days to 'sun': 0, 'mon': 1, ... 'sat': 6 in variables.scss or adapt the margin calculation accordingly so Monday becomes column 1 and Friday column 5.
| @import './variables'; | ||
| @import './calendar'; |
There was a problem hiding this comment.
The calendar import is present, but the weekday mapping used by the start-day modifiers is incorrect. In src/styles/variables.scss the $days map sets 'mon': 0 … 'sun': 6, and calendar.scss uses that map to compute margin-left for the first day. That mapping places Monday in the very first column (index 0) instead of the Monday column when the grid starts from Sunday. This causes .calendar--start-day-* modifiers to put the 1st on the wrong column and therefore does not satisfy the requirement that the month starts at the correct column (e.g. Monday should place the 1st under Monday). Suggest remapping so 'sun': 0, 'mon': 1, 'tue': 2, … 'sat': 6. See variables.scss and calendar.scss for the current implementation: . This also means changing the mapping is required so that changing the start-day modifier in the HTML (e.g. calendar--start-day-mon) reflects the correct layout, which is part of the checklist item: "Changing 'month-lengh' and 'start-day' modifier in the code element reflects in changing calendar layout".
| @@ -0,0 +1,16 @@ | |||
| @import './variables'; | |||
| @import './calendar'; | |||
There was a problem hiding this comment.
calendar.scss generates month-length modifiers with @for $i from 28 through 30 which creates rules for 28, 29 and 30 only. The task explicitly requires modifiers for 28, 29, 30 and 31 (use @for). The missing 31 value means there is no explicit .calendar--month-length-31 rule generated by the loop (the HTML currently uses calendar--month-length-31 by default). Update the loop to include 31 (for example @for $i from 28 through 31) so the code fulfills the requirement and so that changing the month-length modifier in the HTML changes which days are shown. This violates checklist item: "Changing 'month-lengh' and 'start-day' modifier in the code element reflects in changing calendar layout". See calendar.scss: .
| <link | ||
| rel="stylesheet" | ||
| href="styles/index.scss" | ||
| href="styles/main.scss" |
There was a problem hiding this comment.
This violates the implementation requirement: "Write styles in src/styles/main.scss instead of src/style.css" — the link href points to "styles/main.scss". Update the stylesheet link to the required path, e.g. href="src/styles/main.scss", so the project uses the correct SCSS file.
| $days: ( | ||
| 'mon': 0, | ||
| 'tue': 1, | ||
| 'wed': 2, | ||
| 'thu': 3, | ||
| 'fri': 4, | ||
| 'sat': 5, | ||
| 'sun': 6, | ||
| ); |
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 has 'mon': 0 ... 'sun': 6 which is inverted for a calendar that starts from Sunday. For a Sunday-first calendar the offsets should be: 'sun': 0, 'mon': 1, 'tue': 2, 'wed': 3, 'thu': 4, 'fri': 5, 'sat': 6. Consider replacing the map with e.g.:
$start-day-offsets: (
'sun': 0,
'mon': 1,
'tue': 2,
'wed': 3,
'thu': 4,
'fri': 5,
'sat': 6
);
| $bg-color-hover: #ffbfcb; | ||
| $color-border-day: #000; | ||
| $font-size: 30px; | ||
| $days: ( |
There was a problem hiding this comment.
This does not follow the Code Style requirement to "Use properly named variables to make all the calculations more clear." The name $days is ambiguous — rename it to something descriptive (for example $start-day-offsets or $start-day-map) so the intent is clear when used in the SCSS that generates margin-left for the first day.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because there are a few critical issues that currently break the required functionality of the calendar modifiers. First, in src/styles/calendar.scss, the @for loop that generates .calendar--month-length-* classes only goes up to 30, so .calendar--month-length-31 is never created; update it to loop from 28 through 31 so the calendar--month-length-31 modifier in the HTML actually affects the layout. Second, in src/styles/variables.scss, the $days (start-day offsets) map is inverted for a Sunday-first grid ('mon': 0 ... 'sun': 6), which means .calendar--start-day-* will place the 1st under the wrong weekday; change it to a Sunday-first mapping like:
$start-day-offsets: (
'sun': 0,
'mon': 1,
'tue': 2,
'wed': 3,
'thu': 4,
'fri': 5,
'sat': 6,
);and ensure the @each in calendar.scss uses this map so that Monday is column 1 and Friday is column 5 as required. Third, in src/index.html, the <link> tag currently points to styles/main.scss; it must point to src/styles/main.scss so the correct stylesheet is actually loaded and all your SCSS work is visible in the browser. Aside from these, your layout, numbering via ::before, hover animation, and SCSS structure look solid—once you fix these blockers, the calendar modifiers should fully reflect the HTML settings, and your solution will meet the task requirements.
✨ 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
| @for $i from 28 through 30 { | ||
| &--month-length-#{$i} { | ||
| .calendar__day:nth-child(n + #{$i + 1}) { | ||
| display: none; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This @for loop only generates .calendar--month-length-28, -29, and -30. The task requires month-length modifiers for 28, 29, 30 and 31: "Add a modifier month-length for the calendar with values 28, 29, 30 and 31". Update the loop to include 31, e.g. @for $i from 28 through 31 { ... }, so .calendar--month-length-31 is created and the HTML modifier works.
| @each $day, $index in $days { | ||
| &--start-day-#{$day} { | ||
| .calendar__day:first-child { | ||
| margin-left: ($cell-size + $gap) * $index; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The @each relies on the $days map to compute margin-left offsets for .calendar--start-day-*. The task requires correct placement so that Monday is the 1st and Friday is the 5th: "Implement start-day modifier for the calendar with mon, tue, wed, thu, fri, sat and sun values" and "The month should start at the correct column (Monday is the 1st, Friday is the 5th)". Ensure the $days map (in variables.scss) uses a Sunday-first offset mapping (for example 'sun': 0, 'mon': 1, 'tue': 2, 'wed': 3, 'thu': 4, 'fri': 5, 'sat': 6) so .calendar--start-day-* places the 1st under the correct weekday.
| $days: ( | ||
| 'mon': 0, | ||
| 'tue': 1, | ||
| 'wed': 2, | ||
| 'thu': 3, | ||
| 'fri': 4, | ||
| 'sat': 5, | ||
| 'sun': 6, |
There was a problem hiding this comment.
This violates checklist item: "- [ ] Changing 'month-lengh' and 'start-day' modifier in the code element reflects in changing calendar layout" — the $days map assigns 'mon': 0 ... 'sun': 6, which is inverted for a Sunday-first grid and will place the 1st on the wrong weekday. Update the map so offsets match a Sunday-first grid, for example:
$start-day-offsets: (
'sun': 0,
'mon': 1,
'tue': 2,
'wed': 3,
'thu': 4,
'fri': 5,
'sat': 6,
);
(Your current map in src/styles/variables.scss is the source of the problem: .)
| <link | ||
| rel="stylesheet" | ||
| href="styles/index.scss" | ||
| href="styles/main.scss" |
There was a problem hiding this comment.
This violates the requirement: "Write styles in src/styles/main.scss instead of src/style.css." The currently has href="styles/main.scss". Change it to href="src/styles/main.scss" so the page uses the correct stylesheet file.