Skip to content
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 new #5834

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

kondratiukD
Copy link

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job on your submission! 🎉 Your code meets most of the task requirements, and it's clear you've put in a lot of effort. Here are some key points to focus on for improvement:

  • CSS Font Fallback: Ensure you include a fallback font for Roboto to enhance compatibility.
  • CSS Selector Usage: Avoid using tag names like .img for styling; stick to class or ID selectors.
  • HTML Semantic Tags: Incorporate <ul> and <li> tags in your navigation for better semantics.
  • Image Alt Attribute: Make the alt attribute more descriptive than just "logo".
  • Pseudo-element for Active Link: Implement a pseudo-element for the blue line under the active link.
  • Use of gap Property: Replace gap with margin for indents as per the task guidelines.

These adjustments will not only meet the task requirements but also enhance the overall quality 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>
<a
href="#"
class="link hover"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 'hover' class is applied to the 'Laptops & Computers' link. Ensure that this class is defined in your CSS and is intended for this specific link. If it is meant to be a hover effect for all links, consider applying it consistently across all navigation links.

}

a.link.is-active:hover,
a.link.hover:hover {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 'hover' class is used here in conjunction with ':hover'. Ensure that this class is defined in your HTML and is intended for this specific styling. If it is meant to apply to all links, consider using a more general selector or applying the class consistently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants