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 #5979

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

add task solution #5979

wants to merge 1 commit into from

Conversation

Alya-ya
Copy link

@Alya-ya Alya-ya commented Jan 21, 2025

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.

Review Summary

Great job on your submission! 🎉 Your code meets most of the task requirements and constraints, and you've effectively used semantic HTML tags and CSS styling. However, there are a few areas for improvement:

  1. HTML & Navigation:

    • The href attribute for the "Laptops & Computers" link contains spaces. This could cause navigation issues. Consider replacing spaces with %20, hyphens, or underscores.
  2. CSS Styling:

    • The align-items property is used on the img selector, which is not applicable. Removing this line would be beneficial as it doesn't affect image styling.
    • Ensure that the header stretches the full page width without using horizontal margins, as required.
    • The height should be set for nav links, not the header, to align with task requirements.

Despite these minor issues, your work is solid and demonstrates a good understanding of HTML and CSS. The decision to approve is made to help you move forward with your learning. Please review the comments to improve your code, and address these issues in future projects to avoid similar problems. Keep up the great work and continue learning from your experiences! 🚀

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨

</li>
<li class="nav_item">
<a
href="#Laptops & Computers"

Choose a reason for hiding this comment

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

The href attribute value 'Laptops & Computers' contains a space, which is not valid in URLs. Consider replacing the space with '%20' or using a hyphen or underscore.

width: 40px;
height: 40px;
margin-bottom: 0;
align-items: center;

Choose a reason for hiding this comment

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

The align-items property is not applicable to the img element. Consider removing this line as it does not affect image styling.

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