Skip to content

Develop#5207

Open
JohnyPerfomante wants to merge 7 commits intomate-academy:masterfrom
JohnyPerfomante:develop
Open

Develop#5207
JohnyPerfomante wants to merge 7 commits intomate-academy:masterfrom
JohnyPerfomante:develop

Conversation

@JohnyPerfomante
Copy link

Copy link

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

Choose a reason for hiding this comment

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

Review Summary

I am approving your solution because the calendar implementation fully meets the functional and technical requirements: 31 days are rendered via SCSS, the grid is correctly constrained to 7 columns, start-day and month-length modifiers work, and the hover animation behaves exactly as specified. Your SCSS makes effective use of variables, @for and @each, and BEM naming, and the HTML structure correctly keeps modifiers on the calendar root while keeping each day as a simple calendar__day.

The one notable process gap is that your Pull Request description is missing the required copied checklist with - [x] marks after the demo and test links, which is explicitly requested in the task instructions [CHECKLIST ITEM #X]; make sure to add that in future PRs so your submissions are fully compliant. As non-blocking suggestions, you could replace transition: all with specific properties (e.g. transition: transform 0.5s, background-color 0.5s), add :focus styles mirroring :hover for keyboard users, and optionally set z-index on hover and explicit color/display on the ::before numbers for extra robustness. Overall, this is a clean, well-structured solution—nice work.


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

Footnotes

  1. Rate AI review example

Copy link

@etojeDenys etojeDenys left a comment

Choose a reason for hiding this comment

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

almost done

right now, you have a problem with the day starting. it is always one step ahead. If the user sets the start day to sunday, it shows as monday. If the start day is saturday, it shows as sunday
Image

Copy link

@vadiimvooo vadiimvooo left a comment

Choose a reason for hiding this comment

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

Looks like the issue still persist. please feel free to ask for help in fe_chat.

Copy link

@Anton-Kuchmasov Anton-Kuchmasov left a comment

Choose a reason for hiding this comment

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

Good job!

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.

5 participants