-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Develop #4909
base: master
Are you sure you want to change the base?
Develop #4909
Conversation
OkMoroz
commented
Oct 7, 2024
- DEMO LINK
- TEST REPORT LINK
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 work on your CSS! 🌟 It's fantastic to see that you're following best practices and keeping your code well-structured. Remember, those small issues mentioned are just minor tweaks and don't significantly impact the functionality of your code. Keep up the good coding habits, and always aim for improvement! 👍
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
@@ -9,10 +9,58 @@ | |||
<title>Stars</title> | |||
<link | |||
rel="stylesheet" | |||
href="./style.css" | |||
href="./styles/style.css" | |||
/> |
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 attributes of the link tag should also start on a new line, each with a 2-space indentation related to the tag, to keep the attributes correctly formatted.
<div class="stars__container"> | ||
<div class="stars stars--0"> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
</div> | ||
|
||
<div class="stars stars--1"> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
</div> | ||
|
||
<div class="stars stars--2"> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
</div> | ||
|
||
<div class="stars stars--3"> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
</div> | ||
|
||
<div class="stars stars--4"> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
</div> | ||
|
||
<div class="stars stars--5"> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
</div> | ||
</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.
Add empty lines between sibling blocks of HTML to add some 'air' and simplify reading. However, do not add empty lines between parent and child elements.
<div class="stars stars--0"> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
</div> | ||
|
||
<div class="stars stars--1"> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
</div> | ||
|
||
<div class="stars stars--2"> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
</div> | ||
|
||
<div class="stars stars--3"> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
</div> | ||
|
||
<div class="stars stars--4"> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
</div> | ||
|
||
<div class="stars stars--5"> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
</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.
Consider using semantic tags for these blocks to improve the page's SEO and accessibility. For example, each '.stars' div could be an 'article' or 'section' if it represents a distinct section of content.
background-image: url('../../images/star.svg'); | ||
background-repeat: no-repeat; | ||
background-position: center; | ||
margin-right: 4px; |
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.
Be consistent with your margins (Add only top or bottom, but not both). Since .stars__star
elements are laid out horizontally, consider applying margin only to the right or left to maintain consistency and to avoid potential margin collapse issues.
@import './blocks/stars.css'; | ||
|
||
body { | ||
margin: 0; |
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.
Consider adding a fallback font family to enhance cross-browser compatibility. For example, if you're using a sans-serif font, your CSS could look like this:
body {
margin: 0;
font-family: 'YourFont', sans-serif;
}
This ensures that if the primary font fails to load for any reason, the browser will use a default sans-serif font instead.