add changes in README.md, src folder #3874
Conversation
There was a problem hiding this comment.
It looks like there's an issue with the use of the 'font' property in your SCSS. Please adjust it to use the shorthand 'font' property directly or specify 'font-family' and 'font-size' separately to ensure valid CSS. Keep up the great work! 😊
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
There was a problem hiding this comment.
Great job on implementing the SCSS for the calendar component! 🎉 The styles are well-structured, and you've effectively used SCSS features like variables, loops, and mixins. However, please note the incorrect use of nested properties for the 'font' shorthand. Remember to use 'font-family' and 'font-size' separately or apply the 'font' shorthand correctly. Keep up the good work, and make sure to address this in future projects to enhance your coding skills!
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
src/styles/_page.scss
Outdated
| font: { | ||
| family: $font; | ||
| size: $font-size; | ||
| } |
There was a problem hiding this comment.
The use of nested properties for the 'font' shorthand is incorrect. According to CSS syntax, 'font' should be a single shorthand property, not a nested block. You should use 'font-family' and 'font-size' separately or use the 'font' shorthand correctly.
src/index.html
Outdated
| <div class="calendar__day"></div> | ||
|
|
||
| <div class="calendar__day"></div> | ||
|
|
||
| <div class="calendar__day"></div> | ||
|
|
There was a problem hiding this comment.
I think that all the empty lines are redundant)
| <div class="calendar__day"></div> | |
| <div class="calendar__day"></div> | |
| <div class="calendar__day"></div> | |
| <div class="calendar__day"></div> | |
| <div class="calendar__day"></div> | |
| <div class="calendar__day"></div> |
There was a problem hiding this comment.
I made empty lines because AI Buddy recommended
src/styles/_calendar.scss
Outdated
| box-sizing: border-box; | ||
| padding: $padding; | ||
| width: ($padding * 2) + ($gap * 6) + ($size * 7); | ||
| display: flex; | ||
| align-items: center; | ||
| gap: $gap; | ||
| flex-wrap: wrap; | ||
|
|
||
| &__day { | ||
| box-sizing: border-box; |
There was a problem hiding this comment.
maybe this will be a better solution in this case?
this property will work for the entire project
* {
box-sizing: border-box;
}
There was a problem hiding this comment.
the instructions recommend avoiding this selector so as not to decrease performance
src/styles/_calendar.scss
Outdated
| .calendar { | ||
| box-sizing: border-box; | ||
| padding: $padding; | ||
| width: ($padding * 2) + ($gap * 6) + ($size * 7); |
There was a problem hiding this comment.
what do these variables refer to?
I can't figure it out from their name.
Change the variable names
| width: ($padding * 2) + ($gap * 6) + ($size * 7); | |
| width: ($.....* 2) + ($....* 6) + ($.....* 7); |
| } | ||
| } | ||
|
|
||
| @each $day, $margin in (mon: 0, tue: 1, wed: 2, thu: 3, fri: 4, sat: 5, sun: 6) |
There was a problem hiding this comment.
create a variable for it
| @each $day, $margin in (mon: 0, tue: 1, wed: 2, thu: 3, fri: 4, sat: 5, sun: 6) | |
| @each $day, $margin in $weekDays |
src/styles/_variables.scss
Outdated
| $font: Arial, sans-serif; | ||
| $font-size: 30px; | ||
| $hover-offset: 20px; | ||
| $gap: 1px; | ||
| $padding: 10px; | ||
| $border: 1px solid black; |
There was a problem hiding this comment.
u don't need a variables for it. don't create normal variables for fonts. moreover - u use it only once
| $font: Arial, sans-serif; | |
| $font-size: 30px; | |
| $hover-offset: 20px; | |
| $gap: 1px; | |
| $padding: 10px; | |
| $border: 1px solid black; | |
| $gap: 1px; | |
| $padding: 10px; |
reflects in changing calendar layout
Typical MistakesfromBEMlesson theory are checked.