Skip to content

feat: finish calendar component#5453

Open
09valera06-tech wants to merge 2 commits into
mate-academy:masterfrom
09valera06-tech:develop
Open

feat: finish calendar component#5453
09valera06-tech wants to merge 2 commits into
mate-academy:masterfrom
09valera06-tech:develop

Conversation

@09valera06-tech

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

I am requesting changes because one core requirement of the task is not fully met: the calendar width must be limited to exactly 7 columns plus 10px paddings, and this is part of the functional specification, not just a cosmetic detail. At the moment, .calendar uses a width based only on ($day-size * 7) + ($gap * 6), so the 10px padding on both sides is missing from the calculation and from the actual rendered layout.

To fix this, introduce a clear padding variable in src/styles/main.scss, for example #{$calendar-padding: 10px;} near your other layout variables, and then update the .calendar styles to: 1) add padding: $calendar-padding; and 2) include this value twice in the width formula (left + right) so the total width is 7 * day-size + 6 * gap + 2 * calendar-padding. This change directly addresses the task requirement to use variables for layout math (including padding) and ensures the visual width matches "7 columns + 10px paddings" exactly. Everything else in your solution—BEM usage, SCSS loops, modifiers for start day and month length, hover behavior, and general layout—looks solid and aligned with the checklist.

Once you adjust the padding variable and width calculation, your calendar will fully conform to the specification and be ready to pass. Keep up the good work; you’re very close to a complete solution.


✨ 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

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

Comment thread src/styles/main.scss Outdated
.calendar {
display: flex;
flex-wrap: wrap;
width: ($day-size * 7) + ($gap * 6);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The task requires: "Use flex with 1px gap and limit its width to exactly 7 columns + 10px paddings". Currently the width is ($day-size * 7) + ($gap * 6), which does not include any padding and therefore does not satisfy the "+ 10px paddings" part. Consider introducing a padding variable (e.g. $calendar-padding: 10px) and updating the width calculation to account for it, along with adding corresponding padding on .calendar so the visual width matches the requirement.

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

Your solution is approved because it fully implements the required calendar behavior and meets the functional and technical constraints. The calendar is correctly centered, uses flex with the specified 1px gap, and the width is properly calculated using SCSS variables so that seven 100px day cells, gaps, and paddings fit as required. All 31 .calendar__day elements are present without modifiers, and the day numbers are injected via ::before and an @for loop, matching the specification to keep HTML free of hardcoded numbers. The calendar--start-day-* and calendar--month-length-* modifiers are implemented with SCSS loops and correctly adjust the layout by changing the first day’s column via margin-left and hiding extra days via nth-child and display: none, so changing modifiers in HTML updates the calendar as intended. Hover behavior (cursor, pink background, translateY, transition) matches the described interaction, and BEM naming is consistently applied to the block, elements, and modifiers. There are no critical, blocking issues remaining; any further changes would be stylistic refinements rather than requirement fixes. Nicely done aligning your code with both the task description and the previous review feedback.


✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨

Footnotes

  1. Rate AI review example

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants