Develop

[romulo0711]

Test

[mateacademy-ai-mentor]

Review Summary

I am requesting changes because several core requirements from the task are not yet met and they block the solution from being considered complete. The main stylesheet is implemented as src/styles/index.scss and linked via styles/index.css, while the task explicitly requires using src/styles/main.scss as the primary entry and linking that file, so the current setup breaks a key technical constraint. The calendar is not truly centered horizontally and vertically in the viewport: body does not use display: flex, justify-content: center, align-items: center, and height: 100vh (or equivalent) to position the .calendar in the middle of the screen as required. Flex layout is missing the required gap: 1px (using the $gap variable), and the calendar width and start-day offsets rely on hardcoded pixel values and numeric factors instead of consistently using SCSS variables ($columns, $day-size, $gap, $padding), which violates the "Don’t use hardcoded px values" constraint and [CHECKLIST ITEM #1]. Visual tests also show significant layout differences, confirming that the rendered calendar does not match the expected design.

To fix these issues, you should: 1) move your main styles into src/styles/main.scss and update the HTML link/Parcel config to use that file; 2) implement viewport centering with a full-height flex container on body (or a wrapper); 3) add gap: $gap to the .calendar flex container and compute max-width and margin-left for start days using your SCSS variables rather than hardcoded values; and 4) remove unnecessary animation-duration on hover and ensure that changing calendar--month-length-* and calendar--start-day-* modifiers on the calendar element alone correctly adjusts the layout. These changes will align your solution with the task description, the checklist, and the visual expectations, and you already have a solid base with SCSS, BEM, and the core calendar logic, so you are quite close to a fully correct implementation.

---

✨ Please rate this review in the Platform's notification center¹. 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 button² 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 tag³.
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 ↩

[mateacademy-ai-mentor]

The stylesheet link points to styles/index.css, but the description requires writing styles in src/styles/main.scss and (by implication) using that as the main bundled stylesheet. This violates the requirement to use main.scss as the primary styles entry.

[mateacademy-ai-mentor]

The calendar element is not centered vertically and horizontally in the viewport. The task requires: "Display a calendar in the middle of the screen (both horizontally and vertically)", but body only sets width/padding and no centering via flexbox or grid.

[mateacademy-ai-mentor]

Similarly, max-width: 726px is a hardcoded pixel value appearing multiple times, which conflicts with the requirement to avoid reusing hardcoded px values by using SCSS variables and calculations.

[mateacademy-ai-mentor]

All the repeated width: 100px and height: 100px values are hardcoded. The description requires avoiding hardcoded px values when reused and using properly named variables for clarity. In your SCSS you have $day-size, but this compiled CSS still shows raw 100px values.

[mateacademy-ai-mentor]

The calendar block does not configure a flex gap. The task requires using flex with a 1px gap; in SCSS you rely on border width for sizing but do not set a gap or spacing variable, which means the layout does not strictly obey the "1px gap" requirement.

[mateacademy-ai-mentor]

The task requires linking SCSS written in src/styles/main.scss via Parcel; here you link styles/index.css and your main SCSS file is index.scss, which violates: "Write styles in src/styles/main.scss instead of `src/style.css" and the implied build setup.

[mateacademy-ai-mentor]

The calendar container is present and uses the correct modifiers, but the task requires the calendar to be displayed in the middle of the screen horizontally and vertically; there is no centering logic (e.g., flex on body with justify-content/align-items and height: 100vh), so this requirement is not met.

[mateacademy-ai-mentor]

body is given a fixed width and padding, but the task requires the calendar to be centered in the viewport; using a fixed width and only flex-wrap without display:flex or centering properties prevents the calendar from being truly centered.

[mateacademy-ai-mentor]

The container does not define a gap property; requirements say "Use flex with 1px gap and limit its width to exactly 7 columns + 10px paddings", so relying only on borders without an explicit gap: $gap violates that part.

[mateacademy-ai-mentor]

max-width is hardcoded as 726px; the description requires avoiding repeated hardcoded px values by using variables and calculations based on columns, day size, gap, and paddings, so this conflicts with "Don't use hardcoded px values if they are used several times".

[mateacademy-ai-mentor]

The hover effect implements color, transform, and cursor, but animation-duration is unnecessary; the requirement is to animate changes via transition with 0.5s, which you already do, so this extra property is redundant and not part of the spec.

[mateacademy-ai-mentor]

Although days are correctly numbered in the compiled CSS, the requirement is to use SCSS @for and avoid manually writing each nth-child rule; the presence of 31 separate hardcoded blocks here suggests you’re not relying purely on SCSS generation, which conflicts with the intent of the "Don't use hardcoded px values" and loop-based generation.

[mateacademy-ai-mentor]

The start-day margins are hardcoded pixel values; the description requires using properly named variables and @each in SCSS to generate these, so these literals (101px, 202px, etc.) violate the guideline to avoid repeated hardcoded px values.

[mateacademy-ai-mentor]

body uses $gap in the width calculation but does not apply it as an actual flex gap, and .calendar uses calc(7 * $day-size + 6 * $border-width + 2 * $padding) instead of leveraging $columns and $gap consistently, which goes against the requirement to use variables to make calculations clear and avoid hardcoding values like 7 and 6.

[mateacademy-ai-mentor]

The task requires: "Write styles in src/styles/main.scss instead of src/style.css" and explicitly allows linking *.scss directly. Here you link styles/index.css, and your main stylesheet is index.scss, which violates this constraint; update to use main.scss and link that SCSS file as described.

[mateacademy-ai-mentor]

The requirements say the calendar must be displayed in the middle of the screen horizontally and vertically. Current body styles only set margin, flex-wrap, width, and padding; there’s no centering logic (e.g., flexbox with justify-content/align-items on full viewport height) to place .calendar in the center.

[mateacademy-ai-mentor]

Checklist item #1: "Changing 'month-lengh' and 'start-day' modifier in the code element reflects in changing calendar layout" requires that spacing between days use the $gap variable. In your SCSS, body width uses $gap, but here the container width is a hardcoded 726px and no gap is applied, so changing $gap or columns wouldn’t correctly update layout.

[mateacademy-ai-mentor]

According to the description: "Use flex with 1px gap and limit its width to exactly 7 columns + 10px paddings". In this CSS you don’t set gap: 1px on .calendar; instead, you rely solely on border width and hardcoded width 726px, so the specified 1px flex gap is not implemented.

[mateacademy-ai-mentor]

The task says "Don't use hardcoded px values if they are used several times" and to use variables for clarity. Here, width: 100px, height: 100px, border: 1px ..., font-size: 30px are repeated as raw values in CSS instead of reusable variables from SCSS, which contradicts the requirement when this CSS is treated as source.

[mateacademy-ai-mentor]

Hover behavior should be "All changes should be animated with the duration of 0.5s" using transitions. You already have transition on .calendar__day; adding animation-duration: 0.5s introduces a CSS animation property that isn’t needed and may be confusing in the context of the requirement.

[mateacademy-ai-mentor]

In SCSS you set margin-left: calc($i * ($day-size + $border-width)), but the task also defines a $gap variable and requires flex with 1px gap. This calculation doesn’t include $gap, so the start column offsets will not be accurate if $gap is non-zero, which contradicts the layout requirement that spacing be based on the defined gap.

[mateacademy-ai-mentor]

The calendar must be shown in the middle of the screen (horizontally and vertically). Your .calendar styles set display: flex, flex-wrap, and max-width, but there is no centering relative to the viewport (e.g., with margin: auto or a parent flex container), so the block won’t necessarily appear in the screen center.

[mateacademy-ai-mentor]

The layout description doesn’t mention an extra heading above the calendar; keeping <h1>Calendar</h1> here may interfere with vertical centering of the calendar block. Consider removing or repositioning this heading so the calendar itself can be centered as required.

[mateacademy-ai-mentor]

The calendar must appear centered horizontally and vertically in the viewport. Currently, the body just has margin, width, and padding, with no centering via flexbox or grid. Add display: flex, justify-content: center, align-items: center, and height: 100vh (or a wrapper with similar styles) to meet this requirement.

[mateacademy-ai-mentor]

Review Summary

I am requesting changes because there are still a couple of critical requirement violations that block full acceptance. First, the calendar is required to be centered in the middle of the screen, but the extra <h1> above the .calendar block shifts the calendar downward and prevents strict vertical centering; you should either remove the heading or wrap only the calendar in a flex-centered container so the calendar itself is centered. Second, the hover rule for .calendar__day in main.scss/main.css still includes animation-duration: 0.5s;, which was explicitly asked to be removed and is unnecessary because the transition already applies the 0.5s animation to background and transform. These two points directly conflict with the task description and prior review feedback, so they must be fixed before the solution can be considered complete.

To address them: (1) update your HTML layout so body (or another flex container) vertically and horizontally centers only the calendar block, with no extra elements affecting the centering; and (2) remove animation-duration from the .calendar__day:hover rule in main.scss, then recompile to main.css so the compiled CSS matches. As minor improvements (not blocking), consider deleting or clearly deactivating src/styles/index.css to avoid confusion with main.scss, and optionally move the repeated 20px hover shift into a SCSS variable for consistency. Overall, you’ve done a solid job with BEM, SCSS variables, and the calendar modifiers—fixing these last issues will bring the solution fully in line with the requirements.

---

✨ Please rate this review in the Platform's notification center¹. 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 button² 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 tag³.
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 ↩

[mateacademy-ai-mentor]

The task requires “Display a calendar in the middle of the screen (both horizontally and vertically).” With the body acting as the flex centering container in your SCSS, this extra <h1> above the calendar will push the calendar down and prevent it from being truly vertically centered in the viewport; consider removing the heading or nesting the calendar in its own flex-centered wrapper so only the calendar block is centered.

[mateacademy-ai-mentor]

The calendar block uses both calendar--start-day-sun and calendar--month-length-31, which is correct, but note that the comment <!-- repita 31 vezes --> suggests more days might be missing; ensure there are exactly 31 .calendar__day elements and remove or update this comment so it doesn’t mislead future changes.

[mateacademy-ai-mentor]

The standalone index.css file duplicates calendar styles and does not use the SCSS variables or gap/padding width formula from main.scss, which can conflict with the requirement to use src/styles/main.scss as the main styling source; consider removing this file or ensuring it is never referenced.

[mateacademy-ai-mentor]

You still use animation-duration: 0.5s; in the hover rule; this was previously requested to be removed and is not needed because your transition already defines a 0.5s animation.

[mateacademy-ai-mentor]

Here you still rely on hardcoded numerical values in the compiled CSS (gap: 1px, padding: 10px, max-width: 726px), which diverges from the variable-based width and gap logic in main.scss; make sure your SCSS (main.scss) is the only source you maintain and that it doesn’t contradict the requirements (no repeated hardcoded px values in source SCSS).

[mateacademy-ai-mentor]

The hover rule in SCSS still includes animation-duration: 0.5s;; this contradicts the previous correction (“remove unnecessary animation-duration on hover”) and isn’t required by the task, since the transition already controls the 0.5s duration.

[mateacademy-ai-mentor]

animation-duration: 0.5s; is not required by the task and was called out in the previous review; the hover effect should rely only on the transition you already have, so this line should be removed.

[mateacademy-ai-mentor]

Here you again set animation-duration: 0.5s; on hover. This contradicts the requirement to just animate background and transform over 0.5s via transition; remove this property so only the transition controls the timing.