-
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
add task solution #6119
base: master
Are you sure you want to change the base?
add task solution #6119
Conversation
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.
Review Summary
Great job on your submission! 🎉 Your code meets most of the task requirements and constraints, and you've demonstrated a solid understanding of HTML and CSS. Here are some key points and areas for improvement:
-
Roboto Font Embedding: The task requires using the Roboto font with specific settings: roman style, medium (500) weight, and normal width. Currently, the CSS import statement includes a range of weights from 100 to 900, which does not comply with the requirement to select only the medium (500) weight. Additionally, ensure the font-weight range in the import URL is correctly specified as
100,900
instead of100..900
. -
CSS Variable for Blue Color: While you have defined a CSS variable
--blue-color
, it is not consistently used throughout the CSS. The color#00acdc
is hardcoded in multiple places. Make sure to use the defined variable for consistency and maintainability. -
HTML Structure: Your HTML file is well-structured with semantic tags such as
<header>
,<nav>
,<ul>
,<li>
, and<a>
. However, there's a minor issue with an extra space in thehref
attribute valuehref="#gadgets "
. Consider removing the trailing space to avoid potential navigation issues. -
Class
is-active
anddata-qa
Attribute: You've correctly included theis-active
class for the active link and thedata-qa="hover"
attribute for the specified link. -
Avoid Flex
gap
Property: You correctly avoided using the flexgap
property, adhering to the task requirement to usemargin
instead. -
Link Color on Hover: The CSS correctly changes the link color on hover, enhancing user interaction.
-
HTML Formatting and Indentation: Your HTML file follows the 2-space indentation rule and maintains correct formatting, which is great for readability.
-
Image Alt Attribute: The
alt
attribute for the image is descriptive, which is a good practice for accessibility.
While there are a few issues to address, the decision to approve your submission is made to help you move forward with your learning. Please review the comments to improve your code. Remember, addressing these issues will enhance the functionality and maintainability of your code. Keep up the great work and continue learning from these experiences! 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
<a | ||
href="#gadgets " |
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.
There is an extra space at the end of the href attribute value href="#gadgets "
. This could potentially lead to issues with navigation. Consider removing the trailing space.
@@ -1,3 +1,88 @@ | |||
/* stylelint-disable-next-line import-notation */ | |||
@import url('https://fonts.googleapis.com/css2?family=Roboto:ital,wght@0,100..900;1,100..900&display=swap'); |
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 font import URL contains a range 100..900
which is not valid. It should be 100,900
to specify the weights. Consider correcting the font-weight range in the import URL.
No description provided.