Conversation
Bufooon
commented
Feb 15, 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 several core task requirements are not yet met, and they directly affect the main calendar functionality. First, in src/index.html, the calendar root currently uses calendar--start-day-sat; it must default to Sunday, so change this modifier to calendar--start-day-sun, and also reduce the markup to exactly 31 .calendar__day elements (lines 14–55) to match “31 days by default in HTML.” Second, in src/styles/main.scss, the layout uses CSS Grid (display: grid; grid-template-columns: repeat(7, ...)) instead of the required flex layout; you need to switch .calendar to something like display: flex; flex-wrap: wrap; gap: $gap; and size .calendar__day so that 7 items fit per row with the specified 1px gap. Third, day numbers are created via CSS counters, but the requirement is to use a Sass @for loop to generate rules such as .calendar__day:nth-child(#{$i})::before { content: "#{$i}"; }, so please remove the counter logic and replace it with the @for approach. Additionally, update $calendar-max-width to include the horizontal padding (e.g., 7 * $day-size + 6 * $gap + 2 * 10px) so the total width matches “7 columns + 10px paddings,” and ensure your existing --start-day-* modifiers work correctly with the new flex layout (using margin-left on the first visible day with a proper length calculation). Once these changes are made, the modifiers for start-day and month-length will correctly affect the layout, and your otherwise solid use of variables, BEM, @each, and hover effects will fully 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
|
|
||
| <body> | ||
| <h1>Calendar</h1> | ||
| <div class="calendar calendar--start-day-sat calendar--month-length-31"> |
There was a problem hiding this comment.
The root element uses calendar--start-day-sat. This violates the requirement: "Set calendar to start from Sunday by default in your HTML." Change the modifier in HTML to calendar--start-day-sun so the default start day is Sunday.
| <div class="calendar__day"></div> | ||
| <div class="calendar__day"></div> | ||
| <div class="calendar__day"></div> | ||
| <div class="calendar__day"></div> | ||
| <div class="calendar__day"></div> | ||
| <div class="calendar__day"></div> | ||
| <div class="calendar__day"></div> | ||
| <div class="calendar__day"></div> | ||
| <div class="calendar__day"></div> | ||
| <div class="calendar__day"></div> | ||
| <div class="calendar__day"></div> | ||
| <div class="calendar__day"></div> | ||
| <div class="calendar__day"></div> | ||
| <div class="calendar__day"></div> | ||
| <div class="calendar__day"></div> | ||
| <div class="calendar__day"></div> | ||
| <div class="calendar__day"></div> | ||
| <div class="calendar__day"></div> | ||
| <div class="calendar__day"></div> | ||
| <div class="calendar__day"></div> | ||
| <div class="calendar__day"></div> | ||
| <div class="calendar__day"></div> | ||
| <div class="calendar__day"></div> | ||
| <div class="calendar__day"></div> | ||
| <div class="calendar__day"></div> | ||
| <div class="calendar__day"></div> | ||
| <div class="calendar__day"></div> | ||
| <div class="calendar__day"></div> | ||
| <div class="calendar__day"></div> | ||
| <div class="calendar__day"></div> | ||
| <div class="calendar__day"></div> | ||
| <div class="calendar__day"></div> | ||
| <div class="calendar__day"></div> | ||
| <div class="calendar__day"></div> | ||
| <div class="calendar__day"></div> | ||
| <div class="calendar__day"></div> | ||
| <div class="calendar__day"></div> | ||
| <div class="calendar__day"></div> | ||
| <div class="calendar__day"></div> | ||
| <div class="calendar__day"></div> | ||
| <div class="calendar__day"></div> | ||
| <div class="calendar__day"></div> |
There was a problem hiding this comment.
There are 42 .calendar__day elements (lines 14–55). This violates: "Create a markup for the calendar block with 31 days inside" and "Set 31 days by default in your HTML." Keep exactly 31 .calendar__day elements in the HTML (remove extras).
| display: grid; | ||
| grid-template-columns: repeat(7, $day-size); | ||
| grid-auto-rows: $day-size; |
There was a problem hiding this comment.
This uses CSS Grid (display: grid;) and grid-template-columns, which violates the requirement: "Use flex with 1px gap and limit its width to exactly 7 columns + 10px paddings." Switch the calendar layout to a flex approach (e.g. display: flex; flex-wrap: wrap; gap: $gap;) so the calendar rows and column sizing match the task instructions.
| counter-reset: day-counter 0; | ||
| display: grid; | ||
| grid-template-columns: repeat(7, $day-size); | ||
| grid-auto-rows: $day-size; | ||
| gap: $gap; | ||
| max-width: $calendar-max-width; | ||
| width: 100%; | ||
| padding: 10px; | ||
| background: white; | ||
| border: 2px solid #ccc; | ||
| border-radius: 8px; | ||
| box-shadow: 0 10px 30px rgba(0,0,0,0.1); | ||
|
|
||
| @each $day, $offset in ( | ||
| mon: 1, | ||
| tue: 2, | ||
| wed: 3, | ||
| thu: 4, | ||
| fri: 5, | ||
| sat: 6, | ||
| sun: 0 | ||
| ) { | ||
| &--start-#{$day} { | ||
| .calendar__day:first-of-type { | ||
| margin-left: $offset * ($day-size + $gap); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| @for $len from 28 through 31 { | ||
| &--month-length-#{$len} { | ||
| .calendar__day:nth-child(n + #{$len + 1}) { | ||
| visibility: hidden; | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| .calendar__day { | ||
| width: $day-size; | ||
| height: $day-size; | ||
| background: $day-bg; | ||
| border: $day-border; | ||
| display: flex; | ||
| align-items: center; | ||
| justify-content: center; | ||
| cursor: pointer; | ||
| transition: $transition; | ||
|
|
||
| counter-increment: day-counter; | ||
|
|
||
| &::before { | ||
| content: counter(day-counter); | ||
| font-size: 30px; | ||
| font-weight: bold; | ||
| color: #333; | ||
| } |
There was a problem hiding this comment.
The day numbers are generated with CSS counters (counter-reset / counter-increment / content: counter(...)) instead of using a Sass @for loop. This violates: "Use @for (Sass @for at-rule) to add numbers in the center of each day." Replace the counter approach with an @for that emits rules like .calendar__day:nth-child(#{$i})::before { content: "#{$i}"; } so numbers come from Sass.
| @each $day, $offset in ( | ||
| mon: 1, | ||
| tue: 2, | ||
| wed: 3, | ||
| thu: 4, | ||
| fri: 5, | ||
| sat: 6, | ||
| sun: 0 | ||
| ) { | ||
| &--start-#{$day} { | ||
| .calendar__day:first-of-type { | ||
| margin-left: $offset * ($day-size + $gap); |
There was a problem hiding this comment.
While you used @each to generate --start-day- modifiers, the current margin-left approach (margin-left: $offset * ($day-size + $gap);) is applied to a grid child and may not produce the intended column start when combined with Grid. The task requires using flex; therefore this modifier approach should be kept but applied to a flex-based layout. Also ensure the expression yields a length value—use parentheses or ensure Sass unit math produces the correct unit if needed.
| $calendar-max-width: ($day-size * 7) + ($gap * 6); | ||
|
|
||
|
|
||
| * { | ||
| margin: 0; | ||
| padding: 0; | ||
| box-sizing: border-box; | ||
| } | ||
|
|
||
| body { | ||
| min-height: 100vh; | ||
| display: flex; | ||
| justify-content: center; | ||
| align-items: center; | ||
| background: #f5f5f5; | ||
| font-family: Arial, Helvetica, sans-serif; | ||
| } | ||
|
|
||
|
|
||
| .calendar { | ||
| counter-reset: day-counter 0; | ||
| display: grid; | ||
| grid-template-columns: repeat(7, $day-size); | ||
| grid-auto-rows: $day-size; | ||
| gap: $gap; | ||
| max-width: $calendar-max-width; | ||
| width: 100%; | ||
| padding: 10px; |
There was a problem hiding this comment.
The max-width variable is calculated as ($day-size * 7) + ($gap * 6) but the container also uses padding: 10px;. The requirement says to "limit its width to exactly 7 columns + 10px paddings." Include the horizontal padding in the $calendar-max-width calculation so the final width equals 7 day columns plus the required padding.
| @for $len from 28 through 31 { | ||
| &--month-length-#{$len} { | ||
| .calendar__day:nth-child(n + #{$len + 1}) { | ||
| visibility: hidden; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Because the layout uses Grid rather than the required flex approach, CHECKLIST ITEM #1 is not satisfied: "Changing 'month-lengh' and 'start-day' modifier in the code element reflects in changing calendar layout" — switching modifiers may not visually alter the layout as expected. Convert to flex and adjust the first-day offset logic so changing calendar--start-day-* and calendar--month-length-* modifiers visibly change the calendar.