add stars #4897
add stars #4897
Conversation
yuurex
commented
Oct 4, 2024
yuurex
commented
- DEMO LINK
- TEST REPORT LINK
There was a problem hiding this comment.
Great job on your Pull Request! 🌟 All the feedback provided is about minor improvements and code styling, which do not critically impact the functionality or security of your code. Remember to consider using fallback font-families, be consistent with your margins, and maintain a clean code structure with proper indentation and spacing for better readability. Keep up the good work! 👍
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| @@ -12,7 +12,54 @@ | |||
| href="./style.css" | |||
| /> | |||
There was a problem hiding this comment.
The attributes of the tag should each start on a new line with a consistent 2-space indentation from the tag.
| <body> | ||
| <h1>Stars</h1> | ||
| <div class="stars stars--0"> |
There was a problem hiding this comment.
Add an empty line between sibling div elements to add some 'air' and simplify reading the code.
| <div class="stars__star"></div> | ||
| </div> | ||
|
|
||
| <div class="stars stars--1"> |
There was a problem hiding this comment.
Add an empty line between sibling div elements to add some 'air' and simplify reading the code.
| <div class="stars__star"></div> | ||
| </div> | ||
|
|
||
| <div class="stars stars--2"> |
There was a problem hiding this comment.
Add an empty line between sibling div elements to add some 'air' and simplify reading the code.
| <div class="stars__star"></div> | ||
| </div> | ||
|
|
||
| <div class="stars stars--3"> |
There was a problem hiding this comment.
Add an empty line between sibling div elements to add some 'air' and simplify reading the code.
| <div class="stars__star"></div> | ||
| </div> | ||
|
|
||
| <div class="stars stars--4"> |
There was a problem hiding this comment.
Add an empty line between sibling div elements to add some 'air' and simplify reading the code.
| body { | ||
| margin: 0; | ||
| } |
There was a problem hiding this comment.
It's recommended to avoid styling tags directly (except for html and body). However, since this is the body tag and it's being used to reset default margin, this is acceptable.
| background-image: url(images/star.svg); | ||
| width: 16px; | ||
| height: 16px; | ||
| margin-right: 4px; |
There was a problem hiding this comment.
Be consistent with your margins. It's better to add margin to only one side (top or bottom) to avoid potential margin collapse issues.
| margin-right: 4px; | ||
| background-repeat: no-repeat; | ||
| background-position: center; | ||
| } |
There was a problem hiding this comment.
Consider using a fallback font-family to ensure text is displayed with a reasonable font even if the main font isn't available.
