add task solution #5386
add task solution #5386
Conversation
There was a problem hiding this comment.
Your code is on the right track, but there are a few important issues to address before we can approve it. 🚀 Let's focus on these key points:
- Universal Selector: Avoid using the universal selector
*for resetting margins due to performance concerns. Consider using more specific selectors. - Fallback Fonts: Ensure you include fallback fonts in your CSS to maintain font consistency if the main font fails to load.
- HTML Formatting: Check that your lines of code do not exceed 80 characters for better readability.
- Semantic Tags: Use semantic HTML tags to improve SEO and accessibility.
- BEM File Structure: Each BEM block should have its own file. This helps in maintaining a clean and organized codebase.
- Pseudo-Selectors: Ensure yellow stars are styled using container modifiers and pseudo-selectors without extra classes.
These changes will enhance your code's performance, readability, and maintainability. Keep up the great work, and I'm confident you'll nail it with these adjustments! 💪
✨ 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.
There was a problem hiding this comment.
Great job on your submission! 🎉 While there are a few areas for improvement, such as using semantic HTML tags for star ratings, ensuring accessibility with meaningful link text or ARIA labels, and consistently applying the BEM methodology, your code is on the right track. Remember to avoid using the universal selector for resetting margins, include fallback fonts, and consider splitting your CSS into separate files for each BEM block. These changes will enhance your code's performance, maintainability, and accessibility. Keep up the good work and continue learning from these suggestions! 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| @@ -13,6 +13,142 @@ | |||
| /> | |||
| </head> | |||
| <body> | |||
| <h1>Stars</h1> | |||
| <div class="stars stars--0"> | |||
There was a problem hiding this comment.
Consider using a more semantic HTML tag instead of <div> for the star ratings, such as <ul> with <li> elements, to better represent the list of star ratings.
src/index.html
Outdated
| <div class="stars stars--0"> | ||
| <a | ||
| href="#" | ||
| class="stars__star" |
There was a problem hiding this comment.
Each link should have meaningful text or an ARIA label for accessibility. Consider adding aria-label attributes to describe the purpose of each star link.
| ></a> | ||
| </div> | ||
|
|
||
| <div class="stars stars--1"> |
There was a problem hiding this comment.
Similar to the previous comment, consider using semantic HTML tags and ensure accessibility features are in place.
| ></a> | ||
| </div> | ||
|
|
||
| <div class="stars stars--2"> |
There was a problem hiding this comment.
Ensure that the BEM methodology is consistently applied. Check that your CSS file reflects the BEM naming conventions used here.
| @@ -1 +1,36 @@ | |||
| /* add styles here */ | |||
| html { | |||
| margin: 0; | |||
There was a problem hiding this comment.
Consider using a more specific selector for resetting margins instead of applying it to the entire HTML element. This can improve performance.
| display: flex; | ||
| } | ||
|
|
||
| .stars__star { |
There was a problem hiding this comment.
Ensure that you include fallback fonts in your CSS to maintain font consistency if the main font fails to load.
src/style.css
Outdated
| background-image: url(./images/star.svg); | ||
| } | ||
|
|
||
| .stars--1 .stars__star:nth-child(-n + 1) { |
There was a problem hiding this comment.
Consider using pseudo-selectors to style the active stars instead of creating separate classes for each star state. This can simplify your CSS and improve maintainability.
| - [DEMO LINK](https://Julia-Pushko.github.io/layout_stars/) | ||
| - [TEST REPORT LINK](https://Julia-Pushko.github.io/layout_stars/report/html_report/) |
There was a problem hiding this comment.
Add these links to the PR description
src/index.html
Outdated
| <a | ||
| href="#" | ||
| class="stars__star" | ||
| ></a> |
There was a problem hiding this comment.
Fix it everywhere
| <a | |
| href="#" | |
| class="stars__star" | |
| ></a> | |
| <div class="stars__star"></div> |
src/style.css
Outdated
| .stars--1 .stars__star:nth-child(-n + 1) { | ||
| background-image: url('./images/star-active.svg'); | ||
| } | ||
|
|
||
| .stars--2 .stars__star:nth-child(-n + 2) { | ||
| background-image: url('./images/star-active.svg'); | ||
| } | ||
|
|
||
| .stars--3 .stars__star:nth-child(-n + 3) { | ||
| background-image: url('./images/star-active.svg'); | ||
| } | ||
|
|
||
| .stars--4 .stars__star:nth-child(-n + 4) { | ||
| background-image: url('./images/star-active.svg'); | ||
| } | ||
|
|
||
| .stars--5 .stars__star:nth-child(-n + 5) { | ||
| background-image: url('./images/star-active.svg'); | ||
| } |
There was a problem hiding this comment.
You need to combine these selectors with a comma.
DEMO LINK
TEST REPORT LINK
Yellow stars are added with container modifier + pseudo-selector (NO extra classes)
[x ] Each BEM block has its own separate file
[x ] All
Typical MistakesfromBEMlesson theory are checked.[ x] Code follows all the Code Style Rules ❗️