add task solution#5466
Conversation
MasyaArc
commented
Jul 4, 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 there are still a couple of critical requirement violations that block full completion of the task. First, the styles are currently placed in src/styles/index.scss and the HTML links to styles/index.scss, while the description explicitly requires writing styles in src/styles/main.scss and using that path; this is a core structural requirement, not a cosmetic preference. Second, the calendar width is hardcoded as 706px in both src/index.html and src/styles/index.scss, but the task demands that the width be expressed as “7 columns + 10px paddings” using variables like $day-width, $calendar-gap, and padding variables, avoiding repeated magic px values.
To fix this, move or rename your SCSS so that all calendar styles live in src/styles/main.scss, and update the <link> tag in src/index.html to reference styles/main.scss instead of styles/index.scss. Then, replace the 706px width with a variable-based calculation, for example by defining variables for day width, gap, and calendar padding (e.g. $day-width, $calendar-gap, $calendar-padding) and using calc or SCSS arithmetic so the final width clearly equals 7 * $day-width + 6 * $calendar-gap + 2 * $calendar-padding. Once these changes are in place, your solution will satisfy the file-structure requirement and the “no repeated hardcoded px values / clear calculations via variables” requirement, and the rest of your implementation (BEM usage, modifiers, hover behavior, and SCSS loops) is already in good shape. Keep going—your core logic is solid; you just need to align the file paths and width calculation with the specification to finish this task.
✨ 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-sun calendar--month-length-31"> | ||
| <div class="calendar__day"></div> |
There was a problem hiding this comment.
The description requires: "Use flex with 1px gap and limit its width to exactly 7 columns + 10px paddings" and "Don't use hardcoded px values if they are used several times". Using a hardcoded width: 706px; here does not clearly express 7 columns plus paddings via variables and calculations. Consider deriving the width from $day-width, $calendar-gap, and padding variables to satisfy this requirement.
| min-height: 100vh; | ||
| display: flex; | ||
| justify-content: center; | ||
| align-items: center; |
There was a problem hiding this comment.
The description requires: Write styles in src/styles/main.scss instead of src/style.css. Currently the page links to styles/index.scss and the styles are in index.scss, so this violates that requirement. Move styles to main.scss and update this href to point to styles/main.scss.
| } | ||
|
|
||
| .calendar { | ||
| width: 706px; |
There was a problem hiding this comment.
The task says "Write styles in src/styles/main.scss" and also stresses using variables for calculations. Here you hardcode 706px as the calendar width instead of computing it as 7 day widths plus paddings and gap using named variables. This violates the requirement to avoid repeated hardcoded px values and to make calculations clear via variables.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because there is still a second stylesheet (src/styles/index.scss) that conflicts with the requirements and contains hardcoded values. The task explicitly says styles must live in src/styles/main.scss, and the remaining index.scss with width: 706px, height: 100px, and padding: 10px violates the “no repeated magic px values” rule and the “7 columns + 10px paddings” requirement [CHECKLIST ITEM #4]. These hardcoded values are mentioned in the review comments for src/styles/index.scss and represent an outdated calendar implementation that should no longer be present. To fix this, either remove the old calendar styles from index.scss (keeping only imports if truly needed) or fully align index.scss with the variable-based approach already used in main.scss so there is a single, consistent source of truth. Once index.scss no longer defines its own calendar width, height, and padding, your solution will match the description and checklist, since index.html and main.scss are already in good shape. Overall, your SCSS logic and BEM usage are solid; this cleanup is mainly about meeting the structural and style constraints of the task.
✨ 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
| } | ||
|
|
||
| .calendar { | ||
| width: 706px; |
There was a problem hiding this comment.
This still uses a hardcoded 706px width, which violates the requirement to "Use flex with 1px gap and limit its width to exactly 7 columns + 10px paddings" and checklist item #4 about avoiding repeated hardcoded px values. You already implemented a variable-based width in main.scss, so this outdated value in index.scss should be removed or replaced so that only the variable-based calculation remains.
| background-color: #eee; | ||
| box-sizing: border-box; | ||
| width: $day-width; | ||
| height: 100px; |
There was a problem hiding this comment.
height: 100px; here is a repeated magic value; the description says "Don't use hardcoded px values if they are used several times" and your main.scss already introduces $day-height. To align with the requirement, this file should not define a separate hardcoded height – rely on the variable definition in main.scss instead.
| box-sizing: border-box; | ||
| width: $day-width; | ||
| height: 100px; | ||
| padding: 10px; |
There was a problem hiding this comment.
padding: 10px; is another repeated literal that participates in the "7 columns + 10px paddings" requirement. In main.scss you handle width via variables; here the hardcoded padding in each day conflicts with that approach. Consider removing this implementation and using only the variable-based styling from main.scss so the calendar width and paddings are clearly expressed via variables.
| align-items: center; | ||
| padding: 0; | ||
| } | ||
|
|
There was a problem hiding this comment.
The project still contains an alternative stylesheet (index.scss) with a hardcoded calendar width (706px) and padding, which conflicts with the requirement to express the width as “7 columns + 10px paddings” using variables and to write styles in src/styles/main.scss. This violates the description’s rule to avoid repeated magic px values and to use main.scss as the single source of styles; consider removing or updating index.scss so only the variable-based implementation in main.scss remains.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving your solution — the calendar implementation matches the functional requirements very well: it’s centered via flex, uses SCSS with variables and loops, follows BEM, and correctly handles start-day and month-length modifiers with dynamic layout changes. The hover behavior (pointer, pink background, smooth translateY) is implemented exactly as required, and the day numbers are generated via ::before and @for without extra classes on individual days. All core checklist items (such as keeping styles in src/styles/main.scss, using flex with gap: 1px, and avoiding JavaScript) are correctly respected. The only remaining concern is the visual regression test failures, which appear to be minor pixel-level differences rather than logical bugs; if needed, you can review the BackstopJS diff and update the reference images once you confirm the visuals are correct. This approval is not an override for code quality, but you should still double-check your work against the Mate code style and BEM guidelines ([CHECKLIST ITEM #3], [CHECKLIST ITEM #4]) to build good habits. Overall, this is a solid, well-structured solution — nice job applying SCSS features and BEM cleanly while meeting all the calendar behavior 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.