-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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 #6095
base: master
Are you sure you want to change the base?
Develop #6095
Conversation
Mace correct position .header__logo
…ся, тоді елементи меню пісвідцются коли на них наводиш мишкою, али вимикаю він проходить data-qa="hover"
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! There are a few critical issues that need to be addressed before we can approve the pull request.
-
CSS Selector Usage: The use of the universal selector
*
in your CSS impacts performance and goes against the checklist requirements. It's important to apply styles only to specific elements that need them. -
HTML Formatting: Ensure that all lines in your HTML file do not exceed 80 characters to maintain readability and adhere to formatting guidelines.
-
Image
alt
Attribute: Thealt
attribute for the logo image should be more descriptive to improve accessibility. Consider using something like 'Company Logo' or the actual company name. -
CSS Fallback Fonts: Include fallback fonts in your
font-family
property to ensure proper rendering if the primary font is unavailable. -
Logo Link
href
Attribute: Thehref
attribute for the logo link is currently set to#
, which doesn't provide meaningful navigation. Consider linking it to the homepage or a relevant section.
These changes are necessary to meet the core requirements and improve the functionality and accessibility of your code. Once these issues are resolved, we can move forward with approval. Keep up the great work! 😊
✨ 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.
CSS Selector Usage: The use of the universal selector * in your CSS impacts performance and goes against the checklist requirements. It's important to apply styles only to specific elements that need them. HTML Formatting: Ensure that all lines in your HTML file do not exceed 80 characters to maintain readability and adhere to formatting guidelines. Image alt Attribute: The alt attribute for the logo image should be more descriptive to improve accessibility. Consider using something like 'Company Logo' or the actual company name. CSS Fallback Fonts: Include fallback fonts in your font-family property to ensure proper rendering if the primary font is unavailable. Logo Link href Attribute: The href attribute for the logo link is currently set to #, which doesn't provide meaningful navigation. Consider linking it to the homepage or a relevant section.
CSS Selector Usage: The use of the universal selector * in your CSS impacts performance and goes against the checklist requirements. It's important to apply styles only to specific elements that need them. HTML Formatting: Ensure that all lines in your HTML file do not exceed 80 characters to maintain readability and adhere to formatting guidelines. Image alt Attribute: The alt attribute for the logo image should be more descriptive to improve accessibility. Consider using something like 'Company Logo' or the actual company name. CSS Fallback Fonts: Include fallback fonts in your font-family property to ensure proper rendering if the primary font is unavailable. Logo Link href Attribute: The href attribute for the logo link is currently set to #, which doesn't provide meaningful navigation. Consider linking it to the homepage or a relevant section.
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 job on addressing most of the previous feedback! 🎉 Your improvements, especially with the alt
attribute for the logo image and the href
attribute for the logo link, are commendable. However, there are still a few areas that need attention:
-
CSS Selector Usage: The universal selector
*
is still being used, which can impact performance. It's better to apply styles directly to specific elements that require them. -
CSS Fallback Fonts: Ensure that the
font-family
property includes fallback fonts. This is crucial for proper rendering if the primary font is unavailable. For example, you can usefont-family: 'Roboto', Arial, sans-serif;
. -
HTML Formatting: Some lines in the HTML file exceed 80 characters, affecting readability. Consider breaking them into shorter lines.
These issues are important for the overall quality and performance of your code. Please review the comments and make the necessary improvements. Addressing these points will enhance your learning experience and help you avoid similar issues in the future. Keep up the good work and continue to refine your skills! 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
<h1>Moyo header</h1> | ||
<header class="header"> | ||
<a | ||
href="/" |
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 href
attribute for the logo link is set to /
, which may not provide meaningful navigation. Consider linking it to the homepage or a relevant section as per the previous review suggestion.
body { | ||
margin: 0; | ||
font-family: Roboto, sans-serif; |
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 fallback fonts to the font-family
property to ensure proper rendering if 'Roboto' is unavailable. For example, you can use font-family: 'Roboto', Arial, sans-serif;
.
Checklist
❗️ Replace
<your_account>
with your GitHub username and copy the links to thePull Request
description:❗️ Copy this
Checklist
to thePull Request
description after links, and put- [x]
before each point after you checked it.