Develop #3908
Develop #3908
Conversation
Rostmas-gr
commented
Oct 29, 2024
Rostmas-gr
commented
- DEMO LINK
- TEST REPORT LINK
There was a problem hiding this comment.
Thank you for your hard work on this project! 🌟 There are a couple of critical issues that need to be addressed before we can approve the pull request. Firstly, the HTML file is linking directly to an SCSS file, which browsers cannot interpret. Please compile your SCSS to CSS and update the link accordingly. Secondly, there is a redundant @each loop in your SCSS file that could lead to confusion or maintenance issues. Removing the duplicate loop will help keep your code clean and efficient. Once these issues are resolved, your code will be in great shape! Keep up the great work! 💪
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
🔄 Fix the issues, commit the changes, and then re-request my review using the Re-request review button in the Reviewers tab.
| @@ -9,10 +9,42 @@ | |||
| <title>Calendar</title> | |||
| <link | |||
| rel="stylesheet" | |||
| href="styles/index.scss" | |||
| href="styles/main.scss" | |||
There was a problem hiding this comment.
The href attribute is linking to a SCSS file (styles/main.scss). Browsers cannot directly interpret SCSS files. You need to compile the SCSS file to a CSS file and link to the resulting CSS file instead.
| @each $day, $index in $days-of-week { | ||
| &--start-day-#{$day} .calendar__day:nth-child(1) { | ||
| margin-left: calc(#{$index} * (#{$cell-size} + 1px)); | ||
| } | ||
| } |
There was a problem hiding this comment.
This @each loop is redundant because a similar loop is defined starting at line 75. Consider removing one of these loops to avoid confusion and potential conflicts.
There was a problem hiding this comment.
Review Summary
Great job on implementing the calendar layout using SCSS and BEM methodology! 🎉 Your code meets the task requirements, and you've successfully created a responsive calendar with the specified features. However, there are a couple of areas for improvement:
-
Stylesheet Link Issue: The HTML file currently links to a
.scssfile, which browsers cannot interpret directly. Make sure to compile your SCSS into a.cssfile and update the link in your HTML accordingly. -
Redundant SCSS Code: There's a redundancy in your SCSS code regarding the handling of start days for the calendar. Consider removing one of the duplicate loops to keep your code clean and efficient.
Despite these issues, your work is approved to help you move forward with your learning journey. Please review the comments and address these points to enhance your code quality. Keep up the great work and continue learning from these experiences! 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| @@ -9,10 +9,42 @@ | |||
| <title>Calendar</title> | |||
| <link | |||
| rel="stylesheet" | |||
| href="styles/index.scss" | |||
| href="styles/main.scss" | |||
There was a problem hiding this comment.
The stylesheet link references a .scss file, which is not directly usable in HTML. You should compile the SCSS file to CSS and link the resulting .css file instead.
| @each $day, $index in $days-of-week { | ||
| &--start-day-#{$day} .calendar__day:nth-child(1) { | ||
| margin-left: calc(#{$index} * (#{$cell-size} + 1px)); | ||
| } | ||
| } |
There was a problem hiding this comment.
The @each loop here is redundant because a similar loop is defined later starting at line 75. Consider removing one of these loops to avoid duplication and maintain cleaner code.
