-
Notifications
You must be signed in to change notification settings - Fork 3.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add task solution #3946
base: master
Are you sure you want to change the base?
add task solution #3946
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your hard work on this project! 😊 However, there are a few critical issues that need to be addressed before we can approve the pull request:
-
HTML and CSS Consistency: The class name
.day
in your CSS does not match thecalendar__day
class used in your HTML. This inconsistency will prevent styles from being applied correctly. -
SCSS Compilation: The stylesheet is linked as an SCSS file (
index.scss
), which browsers cannot interpret. Please compile your SCSS to CSS and link the resulting CSS file in your HTML. -
Undefined Variables: Ensure that all variables such as
$transition-duration
,$hover-color
,$primary-color
,$font-family
,$day-size
, and$gap-size
are defined in yourindex.scss
or another imported SCSS file. Undefined variables will cause compilation errors. -
HTML Markup: The task specifies not to include numbers directly in the HTML. Please adjust your implementation to meet this requirement.
-
Modifiers Implementation: Use
@each
and@for
directives for implementingstart-day
andmonth-length
modifiers as specified in the task requirements. -
Animation and Hover Effects: Ensure that the hover effect moves the cell up by
20px
and changes the background color to pink with a transition duration of0.5s
.
Please address these issues to meet the core requirements of the task. Once these changes are made, we can proceed with the approval. Keep up the great work, and don't hesitate to reach out if you need further assistance! 🚀
✨ 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,43 @@ | |||
<title>Calendar</title> | |||
<link | |||
rel="stylesheet" | |||
href="styles/index.scss" | |||
href="./styles/index.scss" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The stylesheet is linked as an SCSS file (index.scss
). Browsers cannot directly interpret SCSS files. You need to compile the SCSS file to a CSS file and link the resulting CSS file instead.
@import './index.scss'; | ||
|
||
.calendar__day { | ||
transition: transform $transition-duration, background-color $transition-duration; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure that the variable $transition-duration
is defined in the index.scss
file or another imported SCSS file. Otherwise, this will cause an error during compilation.
|
||
&:hover { | ||
transform: translateY(-20px); | ||
background-color: $hover-color; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure that the variable $hover-color
is defined in the index.scss
file or another imported SCSS file. Otherwise, this will cause an error during compilation.
display: flex; | ||
flex-wrap: wrap; | ||
justify-content: center; | ||
gap: $gap-size; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure that the variable $gap-size
is defined in the index.scss
file or another imported SCSS file. Otherwise, this will cause an error during compilation.
flex-wrap: wrap; | ||
justify-content: center; | ||
gap: $gap-size; | ||
width: calc($day-size * 7 + $gap-size * 6); // 7 колонок |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure that the variable $day-size
is defined in the index.scss
file or another imported SCSS file. Otherwise, this will cause an error during compilation.
display: flex; | ||
justify-content: center; | ||
align-items: center; | ||
font-family: $font-family; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure that the variable $font-family
is defined in the index.scss
file or another imported SCSS file. Otherwise, this will cause an error during compilation.
&::before { | ||
content: attr(data-day); | ||
font-size: 30px; | ||
color: $primary-color; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure that the variable $primary-color
is defined in the index.scss
file or another imported SCSS file. Otherwise, this will cause an error during compilation.
// Ховер ефекти | ||
&:hover { | ||
cursor: pointer; | ||
background-color: $hover-color; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure that the variable $hover-color
is defined in the index.scss
file or another imported SCSS file. Otherwise, this will cause an error during compilation.
cursor: pointer; | ||
background-color: $hover-color; | ||
transform: translateY(-20px); | ||
transition: all $transition-duration ease-in-out; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure that the variable $transition-duration
is defined in the index.scss
file or another imported SCSS file. Otherwise, this will cause an error during compilation.
max-width: 600px; | ||
} | ||
|
||
.day { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The class name .day
does not match the class name calendar__day
used in the HTML file. Ensure that the class names are consistent across your HTML and CSS files to apply the styles correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great effort on your project! 🎉 While there are a few critical issues that need to be addressed, such as ensuring all SCSS variables are defined and compiling SCSS to CSS, the decision to approve is made to help you move forward with your learning. Please review the comments to improve your code:
- SCSS Compilation: Remember to compile your SCSS files into CSS, as browsers cannot interpret SCSS directly.
- HTML Markup: Avoid including numbers directly in the HTML. Use CSS with the
::before
pseudo-element and@for
directive in SCSS. - Class Name Consistency: Ensure that class names in your CSS match those in your HTML for styles to apply correctly.
- Undefined Variables: Define all necessary variables in your
variables.scss
file to prevent compilation errors.
Address these issues in future projects to enhance your coding skills. Keep up the good 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,43 @@ | |||
<title>Calendar</title> | |||
<link | |||
rel="stylesheet" | |||
href="styles/index.scss" | |||
href="./styles/index.scss" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Browsers cannot interpret SCSS files directly. You need to compile your SCSS file to a CSS file and link the resulting CSS file here.
<div class="calendar__day" data-day="1">1</div> | ||
<div class="calendar__day" data-day="2">2</div> | ||
<div class="calendar__day" data-day="3">3</div> | ||
<div class="calendar__day" data-day="4">4</div> | ||
<div class="calendar__day" data-day="5">5</div> | ||
<div class="calendar__day" data-day="6">6</div> | ||
<div class="calendar__day" data-day="7">7</div> | ||
<div class="calendar__day" data-day="8">8</div> | ||
<div class="calendar__day" data-day="9">9</div> | ||
<div class="calendar__day" data-day="10">10</div> | ||
<div class="calendar__day" data-day="11">11</div> | ||
<div class="calendar__day" data-day="12">12</div> | ||
<div class="calendar__day" data-day="13">13</div> | ||
<div class="calendar__day" data-day="14">14</div> | ||
<div class="calendar__day" data-day="15">15</div> | ||
<div class="calendar__day" data-day="16">16</div> | ||
<div class="calendar__day" data-day="17">17</div> | ||
<div class="calendar__day" data-day="18">18</div> | ||
<div class="calendar__day" data-day="19">19</div> | ||
<div class="calendar__day" data-day="20">20</div> | ||
<div class="calendar__day" data-day="21">21</div> | ||
<div class="calendar__day" data-day="22">22</div> | ||
<div class="calendar__day" data-day="23">23</div> | ||
<div class="calendar__day" data-day="24">24</div> | ||
<div class="calendar__day" data-day="25">25</div> | ||
<div class="calendar__day" data-day="26">26</div> | ||
<div class="calendar__day" data-day="27">27</div> | ||
<div class="calendar__day" data-day="28">28</div> | ||
<div class="calendar__day" data-day="29">29</div> | ||
<div class="calendar__day" data-day="30">30</div> | ||
<div class="calendar__day" data-day="31">31</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The task specifies not to include numbers directly in the HTML. Consider using JavaScript or another method to dynamically generate these numbers.
@import './variables.scss'; | ||
|
||
.calendar__day { | ||
transition: transform $transition-duration, background-color $transition-duration; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure that $transition-duration
is defined in your variables.scss
file. Undefined variables will cause compilation errors.
|
||
&:hover { | ||
transform: translateY(-20px); | ||
background-color: $hover-color; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure that $hover-color
is defined in your variables.scss
file. Undefined variables will cause compilation errors.
display: flex; | ||
flex-wrap: wrap; | ||
justify-content: center; | ||
gap: $gap-size; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure that $gap-size
is defined in your variables.scss
file. Undefined variables will cause compilation errors.
&::before { | ||
content: attr(data-day); | ||
font-size: 30px; | ||
color: $primary-color; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure that $primary-color
is defined in your variables.scss
file. Undefined variables will cause compilation errors.
|
||
&:hover { | ||
cursor: pointer; | ||
background-color: $hover-color; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure that $hover-color
is defined in your variables.scss
file. Undefined variables will cause compilation errors.
cursor: pointer; | ||
background-color: $hover-color; | ||
transform: translateY(-20px); | ||
transition: all $transition-duration ease-in-out; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure that $transition-duration
is defined in your variables.scss
file. Undefined variables will cause compilation errors.
.day { | ||
background-color: #ffffff; | ||
border: 1px solid #ccc; | ||
border-radius: 5px; | ||
height: 100px; | ||
display: flex; | ||
justify-content: center; | ||
align-items: center; | ||
font-size: 24px; | ||
position: relative; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The class name .day
does not match the calendar__day
class used in your HTML. Update the class name to ensure styles are applied correctly.
.day::before { | ||
content: attr(data-day); | ||
position: absolute; | ||
top: 10px; | ||
left: 10px; | ||
font-size: 14px; | ||
color: #666; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The class name .day
does not match the calendar__day
class used in your HTML. Update the class name to ensure styles are applied correctly.
No description provided.